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 <s...@denx.de> > > > > > Cc: Tudor Ambarus <tudor.amba...@microchip.com> > > > > > Cc: Eugen Hristev <eugen.hris...@microchip.com> > > > > > Cc: Sergiu Moga <sergiu.m...@microchip.com> > > > > > Cc: Pali Rohár <p...@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: s...@denx.de