[PATCH] imx8mp_evk: Make SPL binary size smaller

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
board to fail to boot.

Reduce the SPL size by the same amount so that it can boot again.

Further SPL reduction work is needed, such as removing driver model support
in SPL.

Just to provide a comparison: NXP U-Boot tree has a SPL binary size of 64kB
versus 96KB in U-Boot mainline.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Hi,

I plan to reduce SPL size even further by removing SPL_DM=y, but this
needs more time to accomplish, so I prefer to give a small SPL reduction
at this time, just to allow the board to boot again.

Also, will try to come up with a SPL size detection in build time, as it
is hard to debug such issues in run-time.

 configs/imx8mp_evk_defconfig | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
index 44b2935f69..00f4ccbe0d 100644
--- a/configs/imx8mp_evk_defconfig
+++ b/configs/imx8mp_evk_defconfig
@@ -33,7 +33,6 @@ CONFIG_SPL_BOOTROM_SUPPORT=y
 CONFIG_SPL_SEPARATE_BSS=y
 CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_SPL_POWER_SUPPORT=y
-CONFIG_SPL_WATCHDOG_SUPPORT=y
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="u-boot=> "
 # CONFIG_CMD_EXPORTENV is not set
@@ -79,8 +78,5 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_MXC_UART=y
 CONFIG_SYSRESET=y
-CONFIG_SPL_SYSRESET=y
 CONFIG_SYSRESET_PSCI=y
-CONFIG_SYSRESET_WATCHDOG=y
 # CONFIG_WATCHDOG is not set
-CONFIG_IMX_WATCHDOG=y
-- 
2.17.1

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/7/20 2:00 PM, Fabio Estevam wrote:
> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
> board to fail to boot.
> 
> Reduce the SPL size by the same amount so that it can boot again.
> 
> Further SPL reduction work is needed, such as removing driver model support
> in SPL.
> 
> Just to provide a comparison: NXP U-Boot tree has a SPL binary size of 64kB
> versus 96KB in U-Boot mainline.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Hi,
> 
> I plan to reduce SPL size even further by removing SPL_DM=y, but this
> needs more time to accomplish, so I prefer to give a small SPL reduction
> at this time, just to allow the board to boot again.
> 
> Also, will try to come up with a SPL size detection in build time, as it
> is hard to debug such issues in run-time.
> 
>  configs/imx8mp_evk_defconfig | 4 ----
>  1 file changed, 4 deletions(-)

So the obvious question is, if you call hang(), does the SPL still reset
or does it fail?

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Harald Seiler]

Hello Fabio,

On Thu, 2020-05-07 at 09:00 -0300, Fabio Estevam wrote:
> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
> board to fail to boot.

Just to check, the addition of the clock driver probably lead to the same
issue?  So the clock driver is potentially working, it is just too big for
SPL?

> Reduce the SPL size by the same amount so that it can boot again.
> 
> Further SPL reduction work is needed, such as removing driver model support
> in SPL.
> 
> Just to provide a comparison: NXP U-Boot tree has a SPL binary size of 64kB
> versus 96KB in U-Boot mainline.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Hi,
> 
> I plan to reduce SPL size even further by removing SPL_DM=y, but this
> needs more time to accomplish, so I prefer to give a small SPL reduction
> at this time, just to allow the board to boot again.
> 
> Also, will try to come up with a SPL size detection in build time, as it
> is hard to debug such issues in run-time.

The SPL linker script already has checks for SPL size. See
arch/arm/cpu/u-boot-spl.lds line 81 and following.  Maybe you just need to
properly set those config-options for your board?

>  configs/imx8mp_evk_defconfig | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
> index 44b2935f69..00f4ccbe0d 100644
> --- a/configs/imx8mp_evk_defconfig
> +++ b/configs/imx8mp_evk_defconfig
> @@ -33,7 +33,6 @@ CONFIG_SPL_BOOTROM_SUPPORT=y
>  CONFIG_SPL_SEPARATE_BSS=y
>  CONFIG_SPL_I2C_SUPPORT=y
>  CONFIG_SPL_POWER_SUPPORT=y
> -CONFIG_SPL_WATCHDOG_SUPPORT=y
>  CONFIG_HUSH_PARSER=y
>  CONFIG_SYS_PROMPT="u-boot=> "
>  # CONFIG_CMD_EXPORTENV is not set
> @@ -79,8 +78,5 @@ CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
>  CONFIG_MXC_UART=y
>  CONFIG_SYSRESET=y
> -CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_PSCI=y
> -CONFIG_SYSRESET_WATCHDOG=y
>  # CONFIG_WATCHDOG is not set
> -CONFIG_IMX_WATCHDOG=y
-- 
Harald

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Harald

On Thu, May 7, 2020 at 9:08 AM Harald Seiler <hws@denx.de> wrote:

> Just to check, the addition of the clock driver probably lead to the same
> issue?  So the clock driver is potentially working, it is just too big for
> SPL?

That's correct.

> The SPL linker script already has checks for SPL size. See
> arch/arm/cpu/u-boot-spl.lds line 81 and following.  Maybe you just need to
> properly set those config-options for your board?

Yes, we do this for imx6. Will implement the same for imx8m.

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

On Thu, May 7, 2020 at 8:59 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
> board to fail to boot.

The new SPL size is 97kB, which is below the limit define in imx8mp_evk.h:
#define CONFIG_SPL_MAX_SIZE (152 * 1024)

It seems the problem is not related to SPL size.

Peng, do you have any ideas?

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/8/20 3:07 AM, Fabio Estevam wrote:
> On Thu, May 7, 2020 at 8:59 AM Fabio Estevam <festevam@gmail.com> wrote:
>>
>> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
>> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
>> board to fail to boot.
> 
> The new SPL size is 97kB, which is below the limit define in imx8mp_evk.h:
> #define CONFIG_SPL_MAX_SIZE (152 * 1024)
> 
> It seems the problem is not related to SPL size.

Disable tiny printf in SPL, because that's really broken with
dm_dump_all(), run dm_dump_all() just after power_init_board() in
board/freescale/imx8m{p,q}_evk/spl.c  and compare the results. There is
likely gonna be a missing driver, my bet would still be on the clock.

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Thu, May 7, 2020 at 10:40 PM Marek Vasut <marex@denx.de> wrote:

> Disable tiny printf in SPL, because that's really broken with
> dm_dump_all(), run dm_dump_all() just after power_init_board() in
> board/freescale/imx8m{p,q}_evk/spl.c  and compare the results. There is
> likely gonna be a missing driver, my bet would still be on the clock.

That's a good suggestion. Yes, the clock drivers are missing for i.MX8MP.

Here is the comparison against a i.MX8MM EVK:
https://pastebin.com/raw/XVc6AAvr

I will activate them on SPL tomorrow.

Also, I noticed that i.MX8MM EVK has the following watchdog issues on master:

U-Boot SPL 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)
Normal Boot
WDT:   Started without servicing (60s timeout)
Trying to boot from MMC1


U-Boot 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)

CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
Reset cause: POR
Model: FSL i.MX8MM EVK board
DRAM:  2 GiB
WDT:   Started without servicing (60s timeout)
MMC:   FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... OK
In:    serial
Out:   serial
Err:   serial
Net:
Warning: ethernet at 30be0000 using MAC address from ROM
eth0: ethernet at 30be0000
Hit any key to stop autoboot:  0
u-boot=>

It reboots after staying 60s in the U-Boot prompt.

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/8/20 4:56 AM, Fabio Estevam wrote:
> Hi Marek,
> 
> On Thu, May 7, 2020 at 10:40 PM Marek Vasut <marex@denx.de> wrote:
> 
>> Disable tiny printf in SPL, because that's really broken with
>> dm_dump_all(), run dm_dump_all() just after power_init_board() in
>> board/freescale/imx8m{p,q}_evk/spl.c  and compare the results. There is
>> likely gonna be a missing driver, my bet would still be on the clock.
> 
> That's a good suggestion. Yes, the clock drivers are missing for i.MX8MP.
> 
> Here is the comparison against a i.MX8MM EVK:
> https://pastebin.com/raw/XVc6AAvr
> 
> I will activate them on SPL tomorrow.

Great, thanks.

> Also, I noticed that i.MX8MM EVK has the following watchdog issues on master:
> 
> U-Boot SPL 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)
> Normal Boot
> WDT:   Started without servicing (60s timeout)
> Trying to boot from MMC1
> 
> 
> U-Boot 2020.07-rc1-00387-g67887903af (May 07 2020 - 23:49:27 -0300)
> 
> CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> Reset cause: POR
> Model: FSL i.MX8MM EVK board
> DRAM:  2 GiB
> WDT:   Started without servicing (60s timeout)
> MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> Loading Environment from MMC... OK
> In:    serial
> Out:   serial
> Err:   serial
> Net:
> Warning: ethernet at 30be0000 using MAC address from ROM
> eth0: ethernet at 30be0000
> Hit any key to stop autoboot:  0
> u-boot=>
> 
> It reboots after staying 60s in the U-Boot prompt.

Enable CONFIG_WATCHDOG in your U-Boot config, that will enable WDT
servicing.

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Thu, May 7, 2020 at 11:59 PM Marek Vasut <marex@denx.de> wrote:

> Enable CONFIG_WATCHDOG in your U-Boot config, that will enable WDT
> servicing.

Thanks. I changed the defconfig like this:

--- a/configs/imx8mm_evk_defconfig
+++ b/configs/imx8mm_evk_defconfig
@@ -85,5 +85,4 @@ CONFIG_SPL_SYSRESET=y
 CONFIG_SYSRESET_PSCI=y
 CONFIG_SYSRESET_WATCHDOG=y
 CONFIG_DM_THERMAL=y
-# CONFIG_WATCHDOG is not set
 CONFIG_IMX_WATCHDOG=y

I will submit this patch and for other boards tomorrow.

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Thu, May 7, 2020 at 11:56 PM Fabio Estevam <festevam@gmail.com> wrote:

> I will activate them on SPL tomorrow.

After activating the SPL clocks I see:

 clk          69  [   ]   imx_clk_gate2         |   |
`-- dram1_root_clk

The dram1_root_clk is not getting turned on.

This clock is marked as CLK_IS_CRITICAL, but it seems that U-Boot core
clock driver does not handle it yet.

I have also tried to enable it manually by doing:

clk_get_by_id(IMX8MP_CLK_DRAM_CORE, &clk);
clk_enable(clk);
clk_get_by_id(IMX8MP_CLK_DRAM1_ROOT, &clk);
clk_enable(clk);

but it did not work.

The complete clock dump is here:
https://pastebin.com/raw/40U6hQny

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

On Fri, May 8, 2020 at 1:53 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Marek,
>
> On Thu, May 7, 2020 at 11:56 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > I will activate them on SPL tomorrow.
>
> After activating the SPL clocks I see:
>
>  clk          69  [   ]   imx_clk_gate2         |   |
> `-- dram1_root_clk
>
> The dram1_root_clk is not getting turned on.

Looks at the iMX8MM EVK log:
https://pastebin.com/raw/XVc6AAvr

It also has:

[   ]   clk_gate              |   |       `-- dram_pll_out

So it seems the problem is somewhere else.

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/8/20 1:49 PM, Fabio Estevam wrote:
> On Fri, May 8, 2020 at 1:53 AM Fabio Estevam <festevam@gmail.com> wrote:
>>
>> Hi Marek,
>>
>> On Thu, May 7, 2020 at 11:56 PM Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> I will activate them on SPL tomorrow.
>>
>> After activating the SPL clocks I see:
>>
>>  clk          69  [   ]   imx_clk_gate2         |   |
>> `-- dram1_root_clk
>>
>> The dram1_root_clk is not getting turned on.
> 
> Looks at the iMX8MM EVK log:
> https://pastebin.com/raw/XVc6AAvr
> 
> It also has:
> 
> [   ]   clk_gate              |   |       `-- dram_pll_out
> 
> So it seems the problem is somewhere else.

Maybe you also need entries like
126dcc925d ("ARM: imx: imx8mm: Add missing clock entries for FEC clock")
for MX8MP ?

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

Hi Marek,

On Fri, May 8, 2020 at 8:57 AM Marek Vasut <marex@denx.de> wrote:

> Maybe you also need entries like
> 126dcc925d ("ARM: imx: imx8mm: Add missing clock entries for FEC clock")
> for MX8MP ?

That only affects U-Boot proper, not SPL.

imx8mm clock driver also does not register Ethernet clocks in SPL:

/* clks not needed in SPL stage */
#ifndef CONFIG_SPL_BUILD
clk_dm(IMX8MM_CLK_ENET_REF,    imx8m_clk_composite("enet_ref",
imx8mm_enet_ref_sels,    base + 0xa980));
.....

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/8/20 2:04 PM, Fabio Estevam wrote:
> Hi Marek,
> 
> On Fri, May 8, 2020 at 8:57 AM Marek Vasut <marex@denx.de> wrote:
> 
>> Maybe you also need entries like
>> 126dcc925d ("ARM: imx: imx8mm: Add missing clock entries for FEC clock")
>> for MX8MP ?
> 
> That only affects U-Boot proper, not SPL.
> 
> imx8mm clock driver also does not register Ethernet clocks in SPL:
> 
> /* clks not needed in SPL stage */
> #ifndef CONFIG_SPL_BUILD
> clk_dm(IMX8MM_CLK_ENET_REF,    imx8m_clk_composite("enet_ref",
> imx8mm_enet_ref_sels,    base + 0xa980));
> .....

Don't you need similar entry for WDT ?

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Fabio Estevam]

On Fri, May 8, 2020 at 9:04 AM Marek Vasut <marex@denx.de> wrote:

> Don't you need similar entry for WDT ?

arch/arm/mach-imx/imx8m/clock_slice.c already contains the wdog entries.

Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Peng Fan]

> Subject: Re: [PATCH] imx8mp_evk: Make SPL binary size smaller
> 
> On Thu, May 7, 2020 at 8:59 AM Fabio Estevam <festevam@gmail.com>
> wrote:
> >
> > Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP
> EVK")
> > caused the u-boot-spl.bin binary size to grow by around 2000 bytes,
> > which makes the board to fail to boot.
> 
> The new SPL size is 97kB, which is below the limit define in imx8mp_evk.h:
> #define CONFIG_SPL_MAX_SIZE (152 * 1024)
> 
> It seems the problem is not related to SPL size.
> 
> Peng, do you have any ideas?

The following patch will make it boot again, not perfect, I need think
more out an elegant fix. Disable DM SPL? Or recalculate the address.
Enable DM consumes too much
on chip ocram malloc area. Need to think about it how we could
use less ocram for SPL malloc area.

diff --git a/include/configs/imx8mp_evk.h b/include/configs/imx8mp_evk.h
index 80e5738961..b90a4f6932 100644
--- a/include/configs/imx8mp_evk.h
+++ b/include/configs/imx8mp_evk.h
@@ -23,15 +23,15 @@
 #ifdef CONFIG_SPL_BUILD
 /*#define CONFIG_ENABLE_DDR_TRAINING_DEBUG*/
 #define CONFIG_SPL_LDSCRIPT            "arch/arm/cpu/armv8/u-boot-spl.lds"
-#define CONFIG_SPL_STACK               0x990000
-#define CONFIG_SPL_BSS_START_ADDR      0x0095e000
-#define CONFIG_SPL_BSS_MAX_SIZE        0x2000  /* 8 KB */
+#define CONFIG_SPL_STACK               0x98fc00
+#define CONFIG_SPL_BSS_START_ADDR      0x0098fc00
+#define CONFIG_SPL_BSS_MAX_SIZE        0x400   /* 1 KB */
 #define CONFIG_SYS_SPL_MALLOC_START    0x42200000
 #define CONFIG_SYS_SPL_MALLOC_SIZE     SZ_512K /* 512 KB */
 #define CONFIG_SYS_ICACHE_OFF
 #define CONFIG_SYS_DCACHE_OFF

-#define CONFIG_MALLOC_F_ADDR           0x940000
+#define CONFIG_MALLOC_F_ADDR           0x950000

 #define CONFIG_SPL_ABORT_ON_RAW_IMAGE

Regards,
Peng.

> 
> Thanks

[PATCH] imx8mp_evk: Make SPL binary size smaller

[Marek Vasut]

On 5/10/20 4:32 PM, Peng Fan wrote:
>> Subject: Re: [PATCH] imx8mp_evk: Make SPL binary size smaller
>>
>> On Thu, May 7, 2020 at 8:59 AM Fabio Estevam <festevam@gmail.com>
>> wrote:
>>>
>>> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP
>> EVK")
>>> caused the u-boot-spl.bin binary size to grow by around 2000 bytes,
>>> which makes the board to fail to boot.
>>
>> The new SPL size is 97kB, which is below the limit define in imx8mp_evk.h:
>> #define CONFIG_SPL_MAX_SIZE (152 * 1024)
>>
>> It seems the problem is not related to SPL size.
>>
>> Peng, do you have any ideas?
> 
> The following patch will make it boot again, not perfect, I need think
> more out an elegant fix. Disable DM SPL? Or recalculate the address.
> Enable DM consumes too much
> on chip ocram malloc area. Need to think about it how we could
> use less ocram for SPL malloc area.

Can't you put the malloc area and stack into DRAM ?
Look at SPL_STACK_R and MALLOC_SIMPLE