diff mbox series

[V2,2/2] imx8mp_evk: simplify board spl code

Message ID 20200511071830.10493-2-peng.fan@nxp.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series [V2,1/2] imx: imx8mp_evk: fix boot issue | expand

Commit Message

Peng Fan May 11, 2020, 7:18 a.m. UTC 
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@nxp.com>
---

V2:
 Fix build warning

 board/freescale/imx8mp_evk/spl.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

Comments

Fabio Estevam May 11, 2020, 12:14 p.m. UTC | #1 
Hi Peng,

On Mon, May 11, 2020 at 3:55 AM Peng Fan <peng.fan@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@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@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
Peng Fan May 11, 2020, 12:25 p.m. UTC | #2 
> 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@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@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@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 mbox series

Patch

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@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);
 }