[PATCH] configs: at91: sama5d2_icp_mmc: Enable CONFIG_LTO

[Stefan Roese]

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.

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