| Message ID | 20200511071830.10493-2-peng.fan@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V2,1/2] imx: imx8mp_evk: fix boot issue | expand |
Hi Peng, On Mon, May 11, 2020 at 3:55 AM Peng Fan <peng.fan at nxp.com> wrote: > > Simplify board SPL code > - BSS area will be cleared by crt0_64.S > - No need to get ccm device in spl_board_init > - Use spl_early_init, not spl_init > - timer_init has been invoked in arch_cpu_init These are several changes in the same patch. It would be better to split them. > > Signed-off-by: Peng Fan <peng.fan at nxp.com> > --- > > V2: > Fix build warning > > board/freescale/imx8mp_evk/spl.c | 30 ++---------------------------- > 1 file changed, 2 insertions(+), 28 deletions(-) > > diff --git a/board/freescale/imx8mp_evk/spl.c b/board/freescale/imx8mp_evk/spl.c > index c5f640dc7b..7b02f43d95 100644 > --- a/board/freescale/imx8mp_evk/spl.c > +++ b/board/freescale/imx8mp_evk/spl.c > @@ -25,11 +25,6 @@ > #include <mmc.h> > #include <asm/arch/ddr.h> > > -#include <dm/uclass.h> > -#include <dm/device.h> > -#include <dm/uclass-internal.h> > -#include <dm/device-internal.h> > - > DECLARE_GLOBAL_DATA_PTR; > > int spl_board_boot_device(enum boot_device boot_dev_spl) > @@ -44,16 +39,7 @@ void spl_dram_init(void) > > void spl_board_init(void) > { > - struct udevice *dev; > - int ret; > - > puts("Normal Boot\n"); > - > - ret = uclass_get_device_by_name(UCLASS_CLK, > - "clock-controller at 30380000", > - &dev); We need this for the SPL watchdog driver, no? I tried selecting CONFIG_SPL_CLK_IMX8MP=y and then the board does not boot anymore, so it looks like we need a better solution. > - if (ret < 0) > - printf("Failed to find clock node. Check device tree\n"); > } > > #define I2C_PAD_CTRL (PAD_CTL_DSE6 | PAD_CTL_HYS | PAD_CTL_PUE | PAD_CTL_PE) > @@ -114,29 +100,19 @@ int board_fit_config_name_match(const char *name) > } > #endif > > +/* Do not use BSS area in this phase */ > void board_init_f(ulong dummy) > { > - int ret; > - > arch_cpu_init(); > > init_uart_clk(1); > > board_early_init_f(); > > - timer_init(); > + spl_early_init(); You should check the returned value from spl_early_init(). If you send a v2, could you please: Thanks
> Subject: Re: [PATCH V2 2/2] imx8mp_evk: simplify board spl code > > Hi Peng, > > On Mon, May 11, 2020 at 3:55 AM Peng Fan <peng.fan at nxp.com> wrote: > > > > Simplify board SPL code > > - BSS area will be cleared by crt0_64.S > > - No need to get ccm device in spl_board_init > > - Use spl_early_init, not spl_init > > - timer_init has been invoked in arch_cpu_init > > These are several changes in the same patch. It would be better to split them.. ok > > > > > Signed-off-by: Peng Fan <peng.fan at nxp.com> > > --- > > > > V2: > > Fix build warning > > > > board/freescale/imx8mp_evk/spl.c | 30 ++---------------------------- > > 1 file changed, 2 insertions(+), 28 deletions(-) > > > > diff --git a/board/freescale/imx8mp_evk/spl.c > > b/board/freescale/imx8mp_evk/spl.c > > index c5f640dc7b..7b02f43d95 100644 > > --- a/board/freescale/imx8mp_evk/spl.c > > +++ b/board/freescale/imx8mp_evk/spl.c > > @@ -25,11 +25,6 @@ > > #include <mmc.h> > > #include <asm/arch/ddr.h> > > > > -#include <dm/uclass.h> > > -#include <dm/device.h> > > -#include <dm/uclass-internal.h> > > -#include <dm/device-internal.h> > > - > > DECLARE_GLOBAL_DATA_PTR; > > > > int spl_board_boot_device(enum boot_device boot_dev_spl) @@ -44,16 > > +39,7 @@ void spl_dram_init(void) > > > > void spl_board_init(void) > > { > > - struct udevice *dev; > > - int ret; > > - > > puts("Normal Boot\n"); > > - > > - ret = uclass_get_device_by_name(UCLASS_CLK, > > - > "clock-controller at 30380000", > > - &dev); > > We need this for the SPL watchdog driver, no? No. clk is enabled in clock_imx8mm.c. > > I tried selecting CONFIG_SPL_CLK_IMX8MP=y and then the board does not > boot anymore, so it looks like we need a better solution. Let's not enable this for SPL. We have limited ram, I plan to disable CCF in SPL. > > > - if (ret < 0) > > - printf("Failed to find clock node. Check device tree\n"); > > } > > > > #define I2C_PAD_CTRL (PAD_CTL_DSE6 | PAD_CTL_HYS | PAD_CTL_PUE > | > > PAD_CTL_PE) @@ -114,29 +100,19 @@ int > > board_fit_config_name_match(const char *name) } #endif > > > > +/* Do not use BSS area in this phase */ > > void board_init_f(ulong dummy) > > { > > - int ret; > > - > > arch_cpu_init(); > > > > init_uart_clk(1); > > > > board_early_init_f(); > > > > - timer_init(); > > + spl_early_init(); > > You should check the returned value from spl_early_init(). ok. Thanks, Peng. > > If you send a v2, could you please: > > Thanks
diff --git a/board/freescale/imx8mp_evk/spl.c b/board/freescale/imx8mp_evk/spl.c index c5f640dc7b..7b02f43d95 100644 --- a/board/freescale/imx8mp_evk/spl.c +++ b/board/freescale/imx8mp_evk/spl.c @@ -25,11 +25,6 @@ #include <mmc.h> #include <asm/arch/ddr.h> -#include <dm/uclass.h> -#include <dm/device.h> -#include <dm/uclass-internal.h> -#include <dm/device-internal.h> - DECLARE_GLOBAL_DATA_PTR; int spl_board_boot_device(enum boot_device boot_dev_spl) @@ -44,16 +39,7 @@ void spl_dram_init(void) void spl_board_init(void) { - struct udevice *dev; - int ret; - puts("Normal Boot\n"); - - ret = uclass_get_device_by_name(UCLASS_CLK, - "clock-controller at 30380000", - &dev); - if (ret < 0) - printf("Failed to find clock node. Check device tree\n"); } #define I2C_PAD_CTRL (PAD_CTL_DSE6 | PAD_CTL_HYS | PAD_CTL_PUE | PAD_CTL_PE) @@ -114,29 +100,19 @@ int board_fit_config_name_match(const char *name) } #endif +/* Do not use BSS area in this phase */ void board_init_f(ulong dummy) { - int ret; - arch_cpu_init(); init_uart_clk(1); board_early_init_f(); - timer_init(); + spl_early_init(); preloader_console_init(); - /* Clear the BSS. */ - memset(__bss_start, 0, __bss_end - __bss_start); - - ret = spl_init(); - if (ret) { - debug("spl_init() failed: %d\n", ret); - hang(); - } - enable_tzc380(); setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1); @@ -145,6 +121,4 @@ void board_init_f(ulong dummy) /* DDR initialization */ spl_dram_init(); - - board_init_r(NULL, 0); }
Simplify board SPL code - BSS area will be cleared by crt0_64.S - No need to get ccm device in spl_board_init - Use spl_early_init, not spl_init - timer_init has been invoked in arch_cpu_init Signed-off-by: Peng Fan <peng.fan at nxp.com> --- V2: Fix build warning board/freescale/imx8mp_evk/spl.c | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-)