[PATCH] configs: at91: sama5d2_icp_mmc: Enable CONFIG_LTO

[Pali Rohár]

On Thursday 27 April 2023 12:30:43 Stefan Roese wrote:
> On 4/27/23 11:51, Eugen Hristev wrote:
> > On 4/27/23 12:26, Stefan Roese wrote:
> > > Hi Eugen,
> > > 
> > > On 4/27/23 11:19, Eugen Hristev wrote:
> > > > Hi Stefan,
> > > > 
> > > > Thank you for the patch.
> > > > 
> > > > This I guess is a workaround such that you can add a bit more of code.
> > > > In the end, it's not scalable, and we have to find a better way,
> > > > probably by removing some of the code to make the SPL smaller.
> > > 
> > > U-Boot image size increase resulting in overflowing some limits is
> > > a common problem, especially in SPL. Enabling LTO gives quite some good
> > > improvements in image size decrease. So I don't think it's an
> > > workaround.
> > 
> > If this was not needed until today, and not adding any new
> > functionality, I would call this a workaround to avoid shrinking the
> > size to fit in the SRAM.
> > When we are adding more and more, and eventually hit this problem again,
> > LTO already enabled, what we will do ?
> > That's why I call this a workaround because we are not solving the
> > problem, just postponing so we can add more things today.
> 
> This is what's happening since many years. But okay, let's call it a
> workaround.
> 
> > > 
> > > > How does this impact the size? How much we are gaining ?
> > > 
> > > I did not measure this. I just checked that this target compiles clean
> > > again with LTO enabled and the MMC related patches applied.
> > > 
> > > Could you (or some college?) please investigate here, how the results
> > > are in image size?
> > 
> > No, you are submitting the patch, I assume you could give some numbers
> > to support your patch.
> 
> Sorry, my time is limited and frankly, I don't feel very much motivated
> (any more) to do additional work here. Even if it's not that much of
> effort.
> 
> > > 
> > > > We can perhaps have a look to see which code is removed and
> > > > guard it by #ifndef SPL_BUILD and that might lower the size. (if
> > > > ofcourse, this code should really be removed)
> > > 
> > > Sure, other improvements in image size decrease are of course always a
> > > good idea.
> > > 
> > > > Also, I don't have a board at hand to test this, so it has to be
> > > > tested first to make sure the board doesn't break.
> > > 
> > > Agreed. I assume/hope that one of your colleges will be able to test
> > > this?
> > 
> > Someone from Microchip can, or other people using the board from the
> > community
> > 
> > I no longer work for Microchip, but I am still maintaining the AT91
> > custodian tree
> 
> Okay. Let's see, where this goes.

Well, if nobody wants to care about this board, go ahead with this
change and if it is not enough that drop support for this board.

> Thanks,
> Stefan
> 
> > Eugen
> > > 
> > > Thanks,
> > > Stefan
> > > 
> > > > Eugen
> > > > 
> > > > 
> > > > On 4/27/23 11:59, Stefan Roese wrote:
> > > > > Adding just a tiny bit more code for sama5d2_icp_mmc leads to a SRAM
> > > > > image overflow. Fix this by enabling LTO for this board, so that such
> > > > > changes still can be made to the common U-Boot code.
> > > > > 
> > > > > Signed-off-by: Stefan Roese <sr at denx.de>
> > > > > Cc: Tudor Ambarus <tudor.ambarus at microchip.com>
> > > > > Cc: Eugen Hristev <eugen.hristev at microchip.com>
> > > > > Cc: Sergiu Moga <sergiu.moga at microchip.com>
> > > > > Cc: Pali Rohár <pali at kernel.org>
> > > > > ---
> > > > >   configs/sama5d2_icp_mmc_defconfig | 5 +++--
> > > > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/configs/sama5d2_icp_mmc_defconfig
> > > > > b/configs/sama5d2_icp_mmc_defconfig
> > > > > index e1b602d8e5ec..a3c57a3f1250 100644
> > > > > --- a/configs/sama5d2_icp_mmc_defconfig
> > > > > +++ b/configs/sama5d2_icp_mmc_defconfig
> > > > > @@ -9,9 +9,11 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
> > > > >   CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > > > >   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> > > > >   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x20003ef0
> > > > > +CONFIG_SF_DEFAULT_SPEED=66000000
> > > > >   CONFIG_ENV_SIZE=0x4000
> > > > >   CONFIG_DM_GPIO=y
> > > > >   CONFIG_DEFAULT_DEVICE_TREE="at91-sama5d2_icp"
> > > > > +CONFIG_OF_LIBFDT_OVERLAY=y
> > > > >   CONFIG_SPL_MMC=y
> > > > >   CONFIG_SPL_SERIAL=y
> > > > >   CONFIG_SPL_DRIVERS_MISC=y
> > > > > @@ -24,6 +26,7 @@ CONFIG_SPL_FS_FAT=y
> > > > >   CONFIG_SPL_LIBDISK_SUPPORT=y
> > > > >   CONFIG_SYS_LOAD_ADDR=0x22000000
> > > > >   CONFIG_DEBUG_UART=y
> > > > > +CONFIG_LTO=y
> > > > >   CONFIG_ENV_VARS_UBOOT_CONFIG=y
> > > > >   CONFIG_SYS_MONITOR_LEN=524288
> > > > >   CONFIG_FIT=y
> > > > > @@ -86,7 +89,6 @@ CONFIG_MMC_SDHCI=y
> > > > >   CONFIG_MMC_SDHCI_ATMEL=y
> > > > >   CONFIG_DM_SPI_FLASH=y
> > > > >   CONFIG_SF_DEFAULT_BUS=2
> > > > > -CONFIG_SF_DEFAULT_SPEED=66000000
> > > > >   CONFIG_SPI_FLASH_SFDP_SUPPORT=y
> > > > >   CONFIG_SPI_FLASH_ATMEL=y
> > > > >   CONFIG_SPI_FLASH_MACRONIX=y
> > > > > @@ -110,5 +112,4 @@ CONFIG_TIMER=y
> > > > >   CONFIG_SPL_TIMER=y
> > > > >   CONFIG_ATMEL_TCB_TIMER=y
> > > > >   CONFIG_SPL_ATMEL_TCB_TIMER=y
> > > > > -CONFIG_OF_LIBFDT_OVERLAY=y
> > > > >   # CONFIG_EFI_LOADER_HII is not set
> > > > 
> > > 
> > > Viele Grüße,
> > > Stefan Roese
> > > 
> > 
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de