From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peng Fan <peng.fan@nxp.com>
Date: Mon, 11 May 2020 12:25:17 +0000
Subject: [PATCH V2 2/2] imx8mp_evk: simplify board spl code
In-Reply-To: <CAOMZO5DH+NeJBbCWh88yGLb0fa-vvmniK=6a810nxAE6LbF4-Q@mail.gmail.com>
References: <20200511071830.10493-1-peng.fan@nxp.com>
 <20200511071830.10493-2-peng.fan@nxp.com>
 <CAOMZO5DH+NeJBbCWh88yGLb0fa-vvmniK=6a810nxAE6LbF4-Q@mail.gmail.com>
Message-ID: <DB6PR0402MB27602E5A0A1F62C61BA7AFB588A10@DB6PR0402MB2760.eurprd04.prod.outlook.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

> 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 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