Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup
Hi Peng, Peter, See comments below Regards -Original Message- From: Peng Fan [mailto:b51...@freescale.com] Sent: Thursday, August 27, 2015 3:16 AM To: Peter Robinson pbrobin...@gmail.com Cc: Alonso Lazcano Adrian-B38018 aalo...@freescale.com; Estevam Fabio- R49496 fabio.este...@freescale.com; u-boot@lists.denx.de; ota...@ossystems.com.br Subject: Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup On Thu, Aug 27, 2015 at 10:00:57AM +0100, Peter Robinson wrote: On Thu, Aug 27, 2015 at 2:39 AM, Peng Fan b51...@freescale.com wrote: Hi Adrian, Since this is only for mx6, why move the code to imx-common? ifeq ($(SOC),$(filter $(SOC),mx6)) -obj-y += cache.o +obj-y += cache.o init.o Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part. I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff. Personally I think it's better off being done from the outset else it never gets done and we end up in the situation we were for imx6. I agree that we'd better remove duplicated code and consolidate code for common usage to make it cleaner and easy to maintian. But from the current patch, clear_ldo_ramp set_ldo_voltage set_ahb_rate clear_mmdc_ch_mask init_bandgap set_preclk_from_osc are all imx6 specific, imx7 do not have the implementation for these functions for now. The goal of arch_cpu_init is for different SoC usage from my understanding, I think it should be there for different SoCs. [Adrian] Ok I agree with that all imx6 specific function can be kept on mx6/soc.c And that each soc will implement their corresponding arch_cpu_init function to Address their particular initialization. The following functions can be consolidated for common usage. boot_mode_apply init_src init_aips [Adrian] Ok I can rework the patch set for it :) The Makefile part shows only for imx6 ifeq ($(SOC),$(filter $(SOC),mx6)), from this patch. Then why move most code mainly imx6 specific to imx- common? [Adrian] For future patch to just add mx7 in soc filter. If we do need such a patch for now, init_aips and boot_mode_apply and init_src may can be consolidated for common usage for imx6/7. But anyway I prefer a small cleanup patch to do this after basic imx7 upstreamed. [Adrian] iMX7 upstream is been kept on hold due to feedback to try to reuse as much of Common code as possible, some drivers (thermal, usb) had already reworked to Support both imx6 and imx7; I think we need to push for: arm: imx: common rework cache settings for imx6 and this patch (version 2 will be sent ASAP) So basic imx7 support ca be accepted. Regards, Peng. Peter -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup
On Thu, Aug 27, 2015 at 1:31 PM, Alonso Adrian aalo...@freescale.com wrote: If we do need such a patch for now, init_aips and boot_mode_apply and init_src may can be consolidated for common usage for imx6/7. But anyway I prefer a small cleanup patch to do this after basic imx7 upstreamed. [Adrian] iMX7 upstream is been kept on hold due to feedback to try to reuse as much of Common code as possible, some drivers (thermal, usb) had already reworked to Support both imx6 and imx7; I think we need to push for: arm: imx: common rework cache settings for imx6 and this patch (version 2 will be sent ASAP) So basic imx7 support ca be accepted. Fully agree. Before adding new code we need to rework the code to be common and add the new one. Add code duplication for later rework does open the door for it never to be reworked and this is not acceptable. -- Otavio Salvador O.S. Systems http://www.ossystems.com.brhttp://code.ossystems.com.br Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup
On Thu, Aug 27, 2015 at 10:00:57AM +0100, Peter Robinson wrote: On Thu, Aug 27, 2015 at 2:39 AM, Peng Fan b51...@freescale.com wrote: Hi Adrian, Since this is only for mx6, why move the code to imx-common? ifeq ($(SOC),$(filter $(SOC),mx6)) -obj-y += cache.o +obj-y += cache.o init.o Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part. I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff. Personally I think it's better off being done from the outset else it never gets done and we end up in the situation we were for imx6. I agree that we'd better remove duplicated code and consolidate code for common usage to make it cleaner and easy to maintian. But from the current patch, clear_ldo_ramp set_ldo_voltage set_ahb_rate clear_mmdc_ch_mask init_bandgap set_preclk_from_osc are all imx6 specific, imx7 do not have the implementation for these functions for now. The goal of arch_cpu_init is for different SoC usage from my understanding, I think it should be there for different SoCs. The following functions can be consolidated for common usage. boot_mode_apply init_src init_aips The Makefile part shows only for imx6 ifeq ($(SOC),$(filter $(SOC),mx6)), from this patch. Then why move most code mainly imx6 specific to imx-common? If we do need such a patch for now, init_aips and boot_mode_apply and init_src may can be consolidated for common usage for imx6/7. But anyway I prefer a small cleanup patch to do this after basic imx7 upstreamed. Regards, Peng. Peter -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup
On Thu, Aug 27, 2015 at 2:39 AM, Peng Fan b51...@freescale.com wrote: Hi Adrian, Since this is only for mx6, why move the code to imx-common? ifeq ($(SOC),$(filter $(SOC),mx6)) -obj-y += cache.o +obj-y += cache.o init.o Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part. I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff. Personally I think it's better off being done from the outset else it never gets done and we end up in the situation we were for imx6. Peter ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: imx: imx-common: init: move arch init common setup
Hi Adrian, Since this is only for mx6, why move the code to imx-common? ifeq ($(SOC),$(filter $(SOC),mx6)) -obj-y += cache.o +obj-y += cache.o init.o Also many pieces of code are only for imx6, imx7 do not need them for now, such as the ldo ramp part. I think basic imx7 support do not need such a big change, if we do need to consolidate code for common usage, we can do this when we need to add more stuff. [.] Regards, Peng. -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
