diff mbox series

imx8mp_evk: Make SPL binary size smaller

Message ID 20200507120029.3172-1-festevam@gmail.com
State New
Headers show
Series imx8mp_evk: Make SPL binary size smaller | expand

Commit Message

Fabio Estevam May 7, 2020, noon UTC
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 at 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(-)

Comments

Marek Vasut May 7, 2020, 12:02 p.m. UTC | #1
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 at 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?
Harald Seiler May 7, 2020, 12:08 p.m. UTC | #2
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 at 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
Fabio Estevam May 7, 2020, 12:37 p.m. UTC | #3
Hi Harald

On Thu, May 7, 2020 at 9:08 AM Harald Seiler <hws at 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
Fabio Estevam May 8, 2020, 1:07 a.m. UTC | #4
On Thu, May 7, 2020 at 8:59 AM Fabio Estevam <festevam at 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
Marek Vasut May 8, 2020, 1:40 a.m. UTC | #5
On 5/8/20 3:07 AM, Fabio Estevam wrote:
> On Thu, May 7, 2020 at 8:59 AM Fabio Estevam <festevam at 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.
Fabio Estevam May 8, 2020, 2:56 a.m. UTC | #6
Hi Marek,

On Thu, May 7, 2020 at 10:40 PM Marek Vasut <marex at 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
Marek Vasut May 8, 2020, 2:59 a.m. UTC | #7
On 5/8/20 4:56 AM, Fabio Estevam wrote:
> Hi Marek,
> 
> On Thu, May 7, 2020 at 10:40 PM Marek Vasut <marex at 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.
Fabio Estevam May 8, 2020, 4:53 a.m. UTC | #8
Hi Marek,

On Thu, May 7, 2020 at 11:56 PM Fabio Estevam <festevam at 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
Fabio Estevam May 8, 2020, 11:49 a.m. UTC | #9
On Fri, May 8, 2020 at 1:53 AM Fabio Estevam <festevam at gmail.com> wrote:
>
> Hi Marek,
>
> On Thu, May 7, 2020 at 11:56 PM Fabio Estevam <festevam at 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
Marek Vasut May 8, 2020, 11:57 a.m. UTC | #10
On 5/8/20 1:49 PM, Fabio Estevam wrote:
> On Fri, May 8, 2020 at 1:53 AM Fabio Estevam <festevam at gmail.com> wrote:
>>
>> Hi Marek,
>>
>> On Thu, May 7, 2020 at 11:56 PM Fabio Estevam <festevam at 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 ?
Marek Vasut May 8, 2020, 12:04 p.m. UTC | #11
On 5/8/20 2:04 PM, Fabio Estevam wrote:
> Hi Marek,
> 
> On Fri, May 8, 2020 at 8:57 AM Marek Vasut <marex at 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 ?
Fabio Estevam May 8, 2020, 12:04 p.m. UTC | #12
Hi Marek,

On Fri, May 8, 2020 at 8:57 AM Marek Vasut <marex at 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
Fabio Estevam May 8, 2020, 3:54 p.m. UTC | #13
On Fri, May 8, 2020 at 9:04 AM Marek Vasut <marex at 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
diff mbox series

Patch

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