[PATCH] configs: at91: sama5d2_icp_mmc: Enable CONFIG_LTO

[Eugen Hristev]

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.

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

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

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
>