[PATCH] configs: at91: sama5d2_icp_mmc: Enable CONFIG_LTO
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
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.
How does this impact the size? How much we are gaining ? 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)
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.
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
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.
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?
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?
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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@denx.de
On 4/27/23 19:55, Pali Rohár wrote:
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.
Hi Pali,
I kind of dislike this attitude. If a patchset breaks a board, a patchset should be changed. Or rejected. I don't agree with removing boards just because in a few days nobody tested one patch. And applying untested patches is again something which I disagree upon.
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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@denx.de
On 03/05/2023 at 11:51, Eugen Hristev wrote:
[You don't often get email from eugen.hristev@collabora.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
On 4/27/23 19:55, Pali Rohár wrote:
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.
I'm monitoring that, with help from Eugen.
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.
Well, give us some time, please!
Hi Pali,
I kind of dislike this attitude. If a patchset breaks a board, a patchset should be changed. Or rejected. I don't agree with removing boards just because in a few days nobody tested one patch. And applying untested patches is again something which I disagree upon.
Thanks Eugen for your support.
We didn't ask for this situation, so Pali, understand that we need to get organized before giving an answer / testing.
Best regards, Nicolas
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@denx.de > Cc: Tudor Ambarus tudor.ambarus@microchip.com > Cc: Eugen Hristev eugen.hristev@microchip.com > Cc: Sergiu Moga sergiu.moga@microchip.com > Cc: Pali Rohár pali@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@denx.de
On Thu, Apr 27, 2023 at 11:26:46AM +0200, 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.
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.
Note that buildman already has flags to tell you how much the size changed and in what functions between commits.
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
Mihai (added to the thread) tested this patch on sama5d2_icp.
Mihai, can you add your Tested-by tag then? This is the original LTO patch for sama5d2_icp that we discussed about.
Thanks, Eugen
Hello Eugen,
I have successfully tested the binary on sama5d2_icp board. Board is booting OK the zImage from fat (SD-card).
Tested-by: Mihai Sain mihai.sain@microchip.com
Best regards, Mihai Sain
-----Original Message----- From: Eugen Hristev eugen.hristev@collabora.com Sent: Monday, July 24, 2023 2:08 PM To: u-boot@lists.denx.de; Mihai Sain - M19926 Mihai.Sain@microchip.com Cc: Pali Rohár pali@kernel.org; Stefan Roese sr@denx.de Subject: Re: [PATCH] configs: at91: sama5d2_icp_mmc: Enable CONFIG_LTO
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
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@denx.de Cc: Tudor Ambarus tudor.ambarus@microchip.com Cc: Eugen Hristev eugen.hristev@microchip.com Cc: Sergiu Moga sergiu.moga@microchip.com Cc: Pali Rohár pali@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
Mihai (added to the thread) tested this patch on sama5d2_icp.
Mihai, can you add your Tested-by tag then? This is the original LTO patch for sama5d2_icp that we discussed about.
Thanks, Eugen
participants (6)
-
Eugen Hristev -
Mihai.Sain＠microchip.com -
Nicolas.Ferre＠microchip.com -
Pali Rohár -
Stefan Roese -
Tom Rini