[U-Boot] [PATCH 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[U-Boot] [PATCH 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
semantics across all of U-boot. This patch series aims at fixing this by
splitting the maximum size into separate image (code + data + rodata +
linker list) size on the one hand, and BSS size on the other hand.

Albert ARIBAUD (4):
  cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics
  da850evm, da840_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics
  smdk5250,snow: fix CONFIG_SPL_MAX_SIZE semantics
  ARM: fix CONFIG_SPL_MAX_SIZE semantics

 README                                         |   17 +++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds                    |   10 ++++++++--
 arch/arm/cpu/u-boot.lds                        |    4 ----
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
 include/configs/cam_enc_4xx.h                  |    4 +++-
 include/configs/da850evm.h                     |    4 +++-
 include/configs/exynos5250-dt.h                |    3 ++-
 9 files changed, 34 insertions(+), 14 deletions(-)

-- 
1.7.10.4

[U-Boot] [PATCH 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
max size between image and BSS based on sizes reported
for current build.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
 include/configs/cam_enc_4xx.h        |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
index dd9d52d..25625dc 100644
--- a/board/ait/cam_enc_4xx/u-boot-spl.lds
+++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
index 56528dd..df3682b 100644
--- a/include/configs/cam_enc_4xx.h
+++ b/include/configs/cam_enc_4xx.h
@@ -230,7 +230,9 @@
 #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
 
 #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
-#define CONFIG_SPL_MAX_SIZE		12320
+/* SPL max size is 12K -- but --pad-to requires a single decimal number */
+#define CONFIG_SPL_MAX_SIZE		12288
+#define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)
 
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SYS_TEXT_BASE		0x81080000
-- 
1.7.10.4

[U-Boot] [PATCH 2/4] da850evm, da840_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
max size between image and BSS based on sizes reported
for current build.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 include/configs/da850evm.h                     |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
index bc34fb5..d7edeb2 100644
--- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
+++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
index 99b4de7..e15c86e 100644
--- a/include/configs/da850evm.h
+++ b/include/configs/da850evm.h
@@ -399,7 +399,9 @@
 #define CONFIG_SPL_LDSCRIPT	"board/$(BOARDDIR)/u-boot-spl-da850evm.lds"
 #define CONFIG_SPL_STACK	0x8001ff00
 #define CONFIG_SPL_TEXT_BASE	0x80000000
-#define CONFIG_SPL_MAX_SIZE	32768
+/* SPL max size is 24K -- but --pad-to requires a single decimal number */
+#define CONFIG_SPL_MAX_SIZE	24576
+#define CONFIG_SPL_BSS_MAX_SIZE	(8*1024)
 #endif
 
 /* Load U-Boot Image From MMC */
-- 
1.7.10.4

[U-Boot] [PATCH 3/4] smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
max size between image and BSS based on sizes reported
for current build.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 board/samsung/smdk5250/smdk5250-uboot-spl.lds |    2 +-
 include/configs/exynos5250-dt.h               |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
index 4c8baaa..208e626 100644
--- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds
+++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
@@ -26,7 +26,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
index 496a194..372b0b4 100644
--- a/include/configs/exynos5250-dt.h
+++ b/include/configs/exynos5250-dt.h
@@ -141,7 +141,8 @@
 /* specific .lds file */
 #define CONFIG_SPL_LDSCRIPT	"board/samsung/smdk5250/smdk5250-uboot-spl.lds"
 #define CONFIG_SPL_TEXT_BASE	0x02023400
-#define CONFIG_SPL_MAX_SIZE	(14 * 1024)
+#define CONFIG_SPL_MAX_SIZE	(10 * 1024)
+#define CONFIG_SPL_BSS_MAX_SIZE	(4 * 1024)
 
 #define CONFIG_BOOTCOMMAND	"mmc read 40007000 451 2000; bootm 40007000"
 
-- 
1.7.10.4

[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

The ASSERT() in arch/arm/cpu/u-boot.lds is unneeded as
this linker file is not used for SPL builds. Remove it.

The ASSERT() in arch/arm/cpu/u-boot-spl.lds is wrong
as it compares image+BSS size to max image size only.
Split it in two distinct ASSERT()s, one for image size,
one for BSS size.

Finally, update README regarding the (now homogeneous)
semantics of the CONFIG_SPL_MAX_SIZE and
CONFIG_SPL_BSS_MAX_SIZE macros.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 README                      |   17 +++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds |   10 ++++++++--
 arch/arm/cpu/u-boot.lds     |    4 ----
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/README b/README
index a35ef31..c902421 100644
--- a/README
+++ b/README
@@ -2784,7 +2784,14 @@ FIT uImage format:
 		LDSCRIPT for linking the SPL binary.
 
 		CONFIG_SPL_MAX_SIZE
-		Maximum binary size (text, data and rodata) of the SPL binary.
+		Maximum size for the image (sum of the text, data, rodata
+		and linker lists sections) of the SPL executable.
+		When defined, linker checks that the actual image size does
+		not exceed it.
+		The total amount of memory used by the SPL when running is
+		equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
 
 		CONFIG_SPL_TEXT_BASE
 		TEXT_BASE for linking the SPL binary.
@@ -2797,7 +2804,13 @@ FIT uImage format:
 		Link address for the BSS within the SPL binary.
 
 		CONFIG_SPL_BSS_MAX_SIZE
-		Maximum binary size of the BSS section of the SPL binary.
+		Maximum size of the BSS section of the SPL executable.
+		When defined, linker checks that the actual BSS size does
+		not exceed it.
+		The total amount of memory used by the SPL when running is
+		equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
 
 		CONFIG_SPL_STACK
 		Adress of the start of the stack SPL will use
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index 3c0d99c..89ef9ce 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -88,6 +88,12 @@ SECTIONS
 	/DISCARD/ : { *(.gnu*) }
 }
 
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
+#if defined(CONFIG_SPL_MAX_SIZE)
+ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
+	"SPL image too big");
+#endif
+
+#if defined(CONFIG_SPL_BSS_MAX_SIZE)
+ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS MAX_SIZE), \
+	"SPL image BSS too big");
 #endif
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 3a1083d..7bbc4f5 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -101,7 +101,3 @@ SECTIONS
 	/DISCARD/ : { *(.interp*) }
 	/DISCARD/ : { *(.gnu*) }
 }
-
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
-#endif
-- 
1.7.10.4

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Tom Rini]

On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:

> CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
> max size between image and BSS based on sizes reported
> for current build.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> 
> ---
> board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
>  include/configs/cam_enc_4xx.h        |    4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
> index dd9d52d..25625dc 100644
> --- a/board/ait/cam_enc_4xx/u-boot-spl.lds
> +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
> @@ -25,7 +25,7 @@
>   */
>  
>  MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
> -		LENGTH = CONFIG_SPL_MAX_SIZE }
> +		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
>  
>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>  OUTPUT_ARCH(arm)
> diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
> index 56528dd..df3682b 100644
> --- a/include/configs/cam_enc_4xx.h
> +++ b/include/configs/cam_enc_4xx.h
> @@ -230,7 +230,9 @@
>  #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
>  
>  #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
> -#define CONFIG_SPL_MAX_SIZE		12320
> +/* SPL max size is 12K -- but --pad-to requires a single decimal number */
> +#define CONFIG_SPL_MAX_SIZE		12288
> +#define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)

This is wrong, you've just increased the overall limit to 16K.  I know
there's a reason that current limit is so exact, Heiko?  And also, this
shows the conceptual problem I have (and 2/2 has the same, along with
tegra).  The important limit is the combined size.  It doesn't matter if
it's 11K text/data/rodata and 1K BSS, or 8+4.  When using custom linker
scripts, we avoid this and can just comment overall (which would need
adding here) that we only care about the combined size.  But then tegra
would be wrong since it uses the generic arm spl linker script?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130408/3c3a9f45/attachment.pgp>

[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Benoît Thébaudeau]

Hi Albert,

On Monday, April 8, 2013 9:58:29 PM, Albert ARIBAUD wrote:
> The ASSERT() in arch/arm/cpu/u-boot.lds is unneeded as
> this linker file is not used for SPL builds. Remove it.
> 
> The ASSERT() in arch/arm/cpu/u-boot-spl.lds is wrong
> as it compares image+BSS size to max image size only.
> Split it in two distinct ASSERT()s, one for image size,
> one for BSS size.
> 
> Finally, update README regarding the (now homogeneous)
> semantics of the CONFIG_SPL_MAX_SIZE and
> CONFIG_SPL_BSS_MAX_SIZE macros.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>  README                      |   17 +++++++++++++++--
>  arch/arm/cpu/u-boot-spl.lds |   10 ++++++++--
>  arch/arm/cpu/u-boot.lds     |    4 ----
>  3 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/README b/README
> index a35ef31..c902421 100644
> --- a/README
> +++ b/README
> @@ -2784,7 +2784,14 @@ FIT uImage format:
>  		LDSCRIPT for linking the SPL binary.
>  
>  		CONFIG_SPL_MAX_SIZE
> -		Maximum binary size (text, data and rodata) of the SPL binary.
> +		Maximum size for the image (sum of the text, data, rodata
> +		and linker lists sections) of the SPL executable.
> +		When defined, linker checks that the actual image size does
> +		not exceed it.
> +		The total amount of memory used by the SPL when running is
> +		equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
> +		it exists.
> +		Note: image and BSS are disjoint for some targets.
>  
>  		CONFIG_SPL_TEXT_BASE
>  		TEXT_BASE for linking the SPL binary.
> @@ -2797,7 +2804,13 @@ FIT uImage format:
>  		Link address for the BSS within the SPL binary.
>  
>  		CONFIG_SPL_BSS_MAX_SIZE
> -		Maximum binary size of the BSS section of the SPL binary.
> +		Maximum size of the BSS section of the SPL executable.
> +		When defined, linker checks that the actual BSS size does
> +		not exceed it.
> +		The total amount of memory used by the SPL when running is
> +		equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
> +		it exists.
> +		Note: image and BSS are disjoint for some targets.
>  
>  		CONFIG_SPL_STACK
>  		Adress of the start of the stack SPL will use
> diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> index 3c0d99c..89ef9ce 100644
> --- a/arch/arm/cpu/u-boot-spl.lds
> +++ b/arch/arm/cpu/u-boot-spl.lds
> @@ -88,6 +88,12 @@ SECTIONS
>  	/DISCARD/ : { *(.gnu*) }
>  }
>  
> -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image
> too big");
> +#if defined(CONFIG_SPL_MAX_SIZE)
> +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \

The possible relocation and MMU data is also part of the binary image file, so
that would be __bss_start rather than __image_copy_end above, and README should
be updated to reflect this.

> +	"SPL image too big");
> +#endif
> +
> +#if defined(CONFIG_SPL_BSS_MAX_SIZE)
> +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS MAX_SIZE), \
> +	"SPL image BSS too big");
>  #endif
> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> index 3a1083d..7bbc4f5 100644
> --- a/arch/arm/cpu/u-boot.lds
> +++ b/arch/arm/cpu/u-boot.lds
> @@ -101,7 +101,3 @@ SECTIONS
>  	/DISCARD/ : { *(.interp*) }
>  	/DISCARD/ : { *(.gnu*) }
>  }
> -
> -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image
> too big");
> -#endif
> --
> 1.7.10.4
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

Best regards,
Beno?t

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Heiko Schocher]

Hello Tom,

Am 08.04.2013 22:43, schrieb Tom Rini:
> On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
> 
>> CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
>> max size between image and BSS based on sizes reported
>> for current build.
>>
>> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>
>> ---
>> board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
>>  include/configs/cam_enc_4xx.h        |    4 +++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
>> index dd9d52d..25625dc 100644
>> --- a/board/ait/cam_enc_4xx/u-boot-spl.lds
>> +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
>> @@ -25,7 +25,7 @@
>>   */
>>  
>>  MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
>> -		LENGTH = CONFIG_SPL_MAX_SIZE }
>> +		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
>>  
>>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>>  OUTPUT_ARCH(arm)
>> diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
>> index 56528dd..df3682b 100644
>> --- a/include/configs/cam_enc_4xx.h
>> +++ b/include/configs/cam_enc_4xx.h
>> @@ -230,7 +230,9 @@
>>  #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
>>  
>>  #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
>> -#define CONFIG_SPL_MAX_SIZE		12320
>> +/* SPL max size is 12K -- but --pad-to requires a single decimal number */
>> +#define CONFIG_SPL_MAX_SIZE		12288
>> +#define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)
> 
> This is wrong, you've just increased the overall limit to 16K.  I know
> there's a reason that current limit is so exact, Heiko?  And also, this

The cam_enc_4xx use only 12k for the SPL code. This is defined in the
UBL header, see u-boot:doc/README.davinci.nand_spl, but can be adapted
for this board. The SoC has an IRam of 32K - ~2k for RBL stack, see:

http://www.ti.com/lit/gpn/tms320dm368

I have no access anymore to this HW to do some tests :-( so I looked
into the hexdump of the current u-boot code with your patch applied, and
the code on the interesting borders (0x0, 0x800 and 0x3800) looks good
to me ...

> shows the conceptual problem I have (and 2/2 has the same, along with
> tegra).  The important limit is the combined size.  It doesn't matter if
> it's 11K text/data/rodata and 1K BSS, or 8+4.  When using custom linker
> scripts, we avoid this and can just comment overall (which would need
> adding here) that we only care about the combined size.  But then tegra
> would be wrong since it uses the generic arm spl linker script?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Heiko,

On Tue, 09 Apr 2013 08:50:26 +0200, Heiko Schocher <hs@denx.de> wrote:

> Hello Tom,
> 
> Am 08.04.2013 22:43, schrieb Tom Rini:
> > On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
> > 
> >> CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
> >> max size between image and BSS based on sizes reported
> >> for current build.
> >>
> >> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> >>
> >> ---
> >> board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
> >>  include/configs/cam_enc_4xx.h        |    4 +++-
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
> >> index dd9d52d..25625dc 100644
> >> --- a/board/ait/cam_enc_4xx/u-boot-spl.lds
> >> +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
> >> @@ -25,7 +25,7 @@
> >>   */
> >>  
> >>  MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
> >> -		LENGTH = CONFIG_SPL_MAX_SIZE }
> >> +		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
> >>  
> >>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> >>  OUTPUT_ARCH(arm)
> >> diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
> >> index 56528dd..df3682b 100644
> >> --- a/include/configs/cam_enc_4xx.h
> >> +++ b/include/configs/cam_enc_4xx.h
> >> @@ -230,7 +230,9 @@
> >>  #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
> >>  
> >>  #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
> >> -#define CONFIG_SPL_MAX_SIZE		12320
> >> +/* SPL max size is 12K -- but --pad-to requires a single decimal number */
> >> +#define CONFIG_SPL_MAX_SIZE		12288
> >> +#define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)
> > 
> > This is wrong, you've just increased the overall limit to 16K.  I know
> > there's a reason that current limit is so exact, Heiko?  And also, this
> 
> The cam_enc_4xx use only 12k for the SPL code. This is defined in the
> UBL header, see u-boot:doc/README.davinci.nand_spl, but can be adapted
> for this board. The SoC has an IRam of 32K - ~2k for RBL stack, see:
> 
> http://www.ti.com/lit/gpn/tms320dm368
> 
> I have no access anymore to this HW to do some tests :-( so I looked
> into the hexdump of the current u-boot code with your patch applied, and
> the code on the interesting borders (0x0, 0x800 and 0x3800) looks good
> to me ...
> 
> > shows the conceptual problem I have (and 2/2 has the same, along with
> > tegra).  The important limit is the combined size.  It doesn't matter if
> > it's 11K text/data/rodata and 1K BSS, or 8+4.  When using custom linker
> > scripts, we avoid this and can just comment overall (which would need
> > adding here) that we only care about the combined size.  But then tegra
> > would be wrong since it uses the generic arm spl linker script?

Thanks Heiko.

I'd read about the SoC IRAM, and had chosen 16K indeed arbitrarily but
taking care not to use most of it -- half felt like safe enough.
However, I'd missed the UBL thing, thanks for pointing this out. So
either I keep 12K, split for instance 10K and 2K (5 pages and 1 page),
or I reaise the number of pages in board/ait/cam_enc_4xx/ublimage.cfg,
correct?

Let us assume I keep 12K. Here is a current build of cam_enc_4xx:

text  data   bss   dec  hex filename
439526  13148  311092  763766  ba776  ./u-boot
  9073	  840     500	10413   28ad  ./spl/u-boot-spl

And the map file gives __start = 0x20, __bss_start = 0x26e0, and
__bss_end = __image_copy_end = _end = 0x28d4, which makes the
size of the non-BSS part of the image equal to 9952 bytes (thus below
10K) and the BSS part size is 500 bytes, below 2K.

So, it seems I could just replace

#define CONFIG_SPL_MAX_SIZE		12288
#define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)

with

#define CONFIG_SPL_MAX_SIZE		10240
#define CONFIG_SPL_BSS_MAX_SIZE		(2*1024)

and keep the UBL cfg file untouched -- any future size issue with
image or BSS size would imply changing these values and uptating the
UBL cfg file.

Would that be ok?

> bye,
> Heiko

Amicalement,
-- 
Albert.

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Heiko Schocher]

Hello Albert,

On 09.04.2013 11:08, Albert ARIBAUD wrote:
> Hi Heiko,
> 
> On Tue, 09 Apr 2013 08:50:26 +0200, Heiko Schocher <hs@denx.de> wrote:
> 
>> Hello Tom,
>>
>> Am 08.04.2013 22:43, schrieb Tom Rini:
>>> On Mon, Apr 08, 2013 at 09:58:26AM -0000, Albert ARIBAUD wrote:
>>>
>>>> CONFIG_SPL_MAX_SIZE wrongly included BSS size. Split
>>>> max size between image and BSS based on sizes reported
>>>> for current build.
>>>>
>>>> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>>>
>>>> ---
>>>> board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
>>>>  include/configs/cam_enc_4xx.h        |    4 +++-
>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
>>>> index dd9d52d..25625dc 100644
>>>> --- a/board/ait/cam_enc_4xx/u-boot-spl.lds
>>>> +++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
>>>> @@ -25,7 +25,7 @@
>>>>   */
>>>>  
>>>>  MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
>>>> -		LENGTH = CONFIG_SPL_MAX_SIZE }
>>>> +		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
>>>>  
>>>>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>>>>  OUTPUT_ARCH(arm)
>>>> diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
>>>> index 56528dd..df3682b 100644
>>>> --- a/include/configs/cam_enc_4xx.h
>>>> +++ b/include/configs/cam_enc_4xx.h
>>>> @@ -230,7 +230,9 @@
>>>>  #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
>>>>  
>>>>  #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
>>>> -#define CONFIG_SPL_MAX_SIZE		12320
>>>> +/* SPL max size is 12K -- but --pad-to requires a single decimal number */
>>>> +#define CONFIG_SPL_MAX_SIZE		12288
>>>> +#define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)
>>>
>>> This is wrong, you've just increased the overall limit to 16K.  I know
>>> there's a reason that current limit is so exact, Heiko?  And also, this
>>
>> The cam_enc_4xx use only 12k for the SPL code. This is defined in the
>> UBL header, see u-boot:doc/README.davinci.nand_spl, but can be adapted
>> for this board. The SoC has an IRam of 32K - ~2k for RBL stack, see:
>>
>> http://www.ti.com/lit/gpn/tms320dm368
>>
>> I have no access anymore to this HW to do some tests :-( so I looked
>> into the hexdump of the current u-boot code with your patch applied, and
>> the code on the interesting borders (0x0, 0x800 and 0x3800) looks good
>> to me ...
>>
>>> shows the conceptual problem I have (and 2/2 has the same, along with
>>> tegra).  The important limit is the combined size.  It doesn't matter if
>>> it's 11K text/data/rodata and 1K BSS, or 8+4.  When using custom linker
>>> scripts, we avoid this and can just comment overall (which would need
>>> adding here) that we only care about the combined size.  But then tegra
>>> would be wrong since it uses the generic arm spl linker script?
> 
> Thanks Heiko.
> 
> I'd read about the SoC IRAM, and had chosen 16K indeed arbitrarily but
> taking care not to use most of it -- half felt like safe enough.
> However, I'd missed the UBL thing, thanks for pointing this out. So
> either I keep 12K, split for instance 10K and 2K (5 pages and 1 page),
> or I reaise the number of pages in board/ait/cam_enc_4xx/ublimage.cfg,
> correct?

Yes, but I would prefer not to change the number of pages.

> Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
> 
> text  data   bss   dec  hex filename
> 439526  13148  311092  763766  ba776  ./u-boot
>   9073	  840     500	10413   28ad  ./spl/u-boot-spl
> 
> And the map file gives __start = 0x20, __bss_start = 0x26e0, and
> __bss_end = __image_copy_end = _end = 0x28d4, which makes the
> size of the non-BSS part of the image equal to 9952 bytes (thus below
> 10K) and the BSS part size is 500 bytes, below 2K.
> 
> So, it seems I could just replace
> 
> #define CONFIG_SPL_MAX_SIZE		12288
> #define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)
> 
> with
> 
> #define CONFIG_SPL_MAX_SIZE		10240
> #define CONFIG_SPL_BSS_MAX_SIZE		(2*1024)
> 
> and keep the UBL cfg file untouched -- any future size issue with
> image or BSS size would imply changing these values and uptating the
> UBL cfg file.
> 
> Would that be ok?

Yes, that seems good to me, but I could not test it ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Heiko,

On Tue, 09 Apr 2013 14:11:38 +0200, Heiko Schocher <hs@denx.de> wrote:

> > Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
> > 
> > text  data   bss   dec  hex filename
> > 439526  13148  311092  763766  ba776  ./u-boot
> >   9073	  840     500	10413   28ad  ./spl/u-boot-spl
> > 
> > And the map file gives __start = 0x20, __bss_start = 0x26e0, and
> > __bss_end = __image_copy_end = _end = 0x28d4, which makes the
> > size of the non-BSS part of the image equal to 9952 bytes (thus below
> > 10K) and the BSS part size is 500 bytes, below 2K.
> > 
> > So, it seems I could just replace
> > 
> > #define CONFIG_SPL_MAX_SIZE		12288
> > #define CONFIG_SPL_BSS_MAX_SIZE		(4*1024)
> > 
> > with
> > 
> > #define CONFIG_SPL_MAX_SIZE		10240
> > #define CONFIG_SPL_BSS_MAX_SIZE		(2*1024)
> > 
> > and keep the UBL cfg file untouched -- any future size issue with
> > image or BSS size would imply changing these values and uptating the
> > UBL cfg file.
> > 
> > Would that be ok?
> 
> Yes, that seems good to me, but I could not test it ...

I can do a diff of the binaries with and without the patch --
normally, they should be the same except for the U-Boot version
identification. Would that do?

> bye,
> Heiko

Amicalement,
-- 
Albert.

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Heiko Schocher]

Hello Albert,

on 09.04.2013 14:42, wrote Albert ARIBAUD:
> Hi Heiko,
> 
> On Tue, 09 Apr 2013 14:11:38 +0200, Heiko Schocher <hs@denx.de> wrote:
> 
>>> Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
>>>
>>> text  data   bss   dec  hex filename
>>> 439526  13148  311092  763766  ba776  ./u-boot
>>>   9073	  840     500	10413   28ad  ./spl/u-boot-spl
[...]
>>> Would that be ok?
>>
>> Yes, that seems good to me, but I could not test it ...
> 
> I can do a diff of the binaries with and without the patch --
> normally, they should be the same except for the U-Boot version
> identification. Would that do?

Yes. I did this too, and see only a diff for the version string.
(But not forget to commit your change first, else the "-dirty"
in the version string will insert an offset ;-)

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [U-Boot, 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Heiko,

On Tue, 09 Apr 2013 15:17:02 +0200, Heiko Schocher <hs@denx.de> wrote:

> Hello Albert,
> 
> on 09.04.2013 14:42, wrote Albert ARIBAUD:
> > Hi Heiko,
> > 
> > On Tue, 09 Apr 2013 14:11:38 +0200, Heiko Schocher <hs@denx.de> wrote:
> > 
> >>> Let us assume I keep 12K. Here is a current build of cam_enc_4xx:
> >>>
> >>> text  data   bss   dec  hex filename
> >>> 439526  13148  311092  763766  ba776  ./u-boot
> >>>   9073	  840     500	10413   28ad  ./spl/u-boot-spl
> [...]
> >>> Would that be ok?
> >>
> >> Yes, that seems good to me, but I could not test it ...
> > 
> > I can do a diff of the binaries with and without the patch --
> > normally, they should be the same except for the U-Boot version
> > identification. Would that do?
> 
> Yes. I did this too, and see only a diff for the version string.

Thanks -- I've staged the 10K+2K change for V2.

> (But not forget to commit your change first, else the "-dirty"
> in the version string will insert an offset ;-)

I actually keep a 'fake_build' branch with a single nifty commit on
it that makes almost all version information sources constant (only
mkimage resists as it collects the current date and time
programmatically, but that's ok, I don't aim for 100% faking.

So, whenever I need to do bulk comparisons, I add this commit above the
two branches to be compared, and two batch builds later, I can compare
the .bins, even if different commit ID and/or local changes are
involved. :)

> bye,
> Heiko

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Beno?t,

On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> Hi Albert,

> > diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> > index 3c0d99c..89ef9ce 100644
> > --- a/arch/arm/cpu/u-boot-spl.lds
> > +++ b/arch/arm/cpu/u-boot-spl.lds
> > @@ -88,6 +88,12 @@ SECTIONS
> >  	/DISCARD/ : { *(.gnu*) }
> >  }
> >  
> > -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> > -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image
> > too big");
> > +#if defined(CONFIG_SPL_MAX_SIZE)
> > +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
> 
> The possible relocation and MMU data is also part of the binary image file, so
> that would be __bss_start rather than __image_copy_end above, and README should
> be updated to reflect this.

Actually, mmutable is not used in any SPL; it is used only in targets
h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL.
I have just confirmed this with a MAKEALL -a arm and a grep on all map
files.

This presence of mmutable in u-boot-spl.lds is in fact an overlook
that I missed when I created this file from u-boot.lds. I have just
finished verifying that removing the mmutable section altogether does
not change a single bit to any of the 309 ARM platforms currently built
under MAKEALL -a arm.

I'll remove mmutable entries from u-boot-spl.lds in V2.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Benoît Thébaudeau]

Hi Albert,

On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
> Hi Beno?t,
> 
> On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Beno?t Th?baudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > Hi Albert,
> 
> > > diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> > > index 3c0d99c..89ef9ce 100644
> > > --- a/arch/arm/cpu/u-boot-spl.lds
> > > +++ b/arch/arm/cpu/u-boot-spl.lds
> > > @@ -88,6 +88,12 @@ SECTIONS
> > >  	/DISCARD/ : { *(.gnu*) }
> > >  }
> > >  
> > > -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> > > -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL
> > > image
> > > too big");
> > > +#if defined(CONFIG_SPL_MAX_SIZE)
> > > +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
> > 
> > The possible relocation and MMU data is also part of the binary image file,
> > so
> > that would be __bss_start rather than __image_copy_end above, and README
> > should
> > be updated to reflect this.
> 
> Actually, mmutable is not used in any SPL; it is used only in targets
> h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL.
> I have just confirmed this with a MAKEALL -a arm and a grep on all map
> files.
> 
> This presence of mmutable in u-boot-spl.lds is in fact an overlook
> that I missed when I created this file from u-boot.lds. I have just
> finished verifying that removing the mmutable section altogether does
> not change a single bit to any of the 309 ARM platforms currently built
> under MAKEALL -a arm.
> 
> I'll remove mmutable entries from u-boot-spl.lds in V2.

OK, that's perfect for MMU data, but what about relocation data?

Best regards,
Beno?t

[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Benoît Thébaudeau]

Hi Albert,

On Tuesday, April 9, 2013 7:43:06 PM, Albert ARIBAUD wrote:
> Hi Beno?t,
> 
> On Tue, 9 Apr 2013 16:24:36 +0200 (CEST), Beno?t Th?baudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > Hi Albert,
> > 
> > On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
> > > Hi Beno?t,
> > > 
> > > On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Beno?t Th?baudeau
> > > <benoit.thebaudeau@advansee.com> wrote:
> > > 
> > > > Hi Albert,
> > > 
> > > > > diff --git a/arch/arm/cpu/u-boot-spl.lds
> > > > > b/arch/arm/cpu/u-boot-spl.lds
> > > > > index 3c0d99c..89ef9ce 100644
> > > > > --- a/arch/arm/cpu/u-boot-spl.lds
> > > > > +++ b/arch/arm/cpu/u-boot-spl.lds
> > > > > @@ -88,6 +88,12 @@ SECTIONS
> > > > >  	/DISCARD/ : { *(.gnu*) }
> > > > >  }
> > > > >  
> > > > > -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> > > > > -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE),
> > > > > "SPL
> > > > > image
> > > > > too big");
> > > > > +#if defined(CONFIG_SPL_MAX_SIZE)
> > > > > +ASSERT(__image_copy_end - __image_copy_start <
> > > > > (CONFIG_SPL_MAX_SIZE), \
> > > > 
> > > > The possible relocation and MMU data is also part of the binary image
> > > > file,
> > > > so
> > > > that would be __bss_start rather than __image_copy_end above, and
> > > > README
> > > > should
> > > > be updated to reflect this.
> > > 
> > > Actually, mmutable is not used in any SPL; it is used only in targets
> > > h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL.
> > > I have just confirmed this with a MAKEALL -a arm and a grep on all map
> > > files.
> > > 
> > > This presence of mmutable in u-boot-spl.lds is in fact an overlook
> > > that I missed when I created this file from u-boot.lds. I have just
> > > finished verifying that removing the mmutable section altogether does
> > > not change a single bit to any of the 309 ARM platforms currently built
> > > under MAKEALL -a arm.
> > > 
> > > I'll remove mmutable entries from u-boot-spl.lds in V2.
> > 
> > OK, that's perfect for MMU data, but what about relocation data?
> 
> Relocation data should not exist for SPLs, which do not relocate.
> 
> Unfortunately, most tegra and some exynos have start.S code going
> through the relocation loop even for their SPL; that's cardhu,
> colibri_t20_iris, dalmore, harmony, medcom-wide, origen, paz00, plutux,
> seaboard, smdkv310, tec, trimslice, ventana, and whistler.
> 
> Tegras do it for their arm720t; Exynos' probably do something similar.
> I am not going to try and change some start.S so close in time to
> release. :)
> 
> Fortunately, for all the SPLs that fail building if I remove .rel.dyn
> and .dynsym, these sections are actually empty, i.e. their __end is
> equal to their __image_copy_end. I have manually verified both reloc
> section emptiness and equality of _end and __image_copy_end.
> 
> Therefore I'll leave the ASSERT() as written in V2, and will provide a
> separate patch for fixing the Tegra / Exynos unneeded relocation data
> issue.

That's perfect.

Best regards,
Beno?t

[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Beno?t,

On Tue, 9 Apr 2013 16:24:36 +0200 (CEST), Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> Hi Albert,
> 
> On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
> > Hi Beno?t,
> > 
> > On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Beno?t Th?baudeau
> > <benoit.thebaudeau@advansee.com> wrote:
> > 
> > > Hi Albert,
> > 
> > > > diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> > > > index 3c0d99c..89ef9ce 100644
> > > > --- a/arch/arm/cpu/u-boot-spl.lds
> > > > +++ b/arch/arm/cpu/u-boot-spl.lds
> > > > @@ -88,6 +88,12 @@ SECTIONS
> > > >  	/DISCARD/ : { *(.gnu*) }
> > > >  }
> > > >  
> > > > -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> > > > -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL
> > > > image
> > > > too big");
> > > > +#if defined(CONFIG_SPL_MAX_SIZE)
> > > > +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
> > > 
> > > The possible relocation and MMU data is also part of the binary image file,
> > > so
> > > that would be __bss_start rather than __image_copy_end above, and README
> > > should
> > > be updated to reflect this.
> > 
> > Actually, mmutable is not used in any SPL; it is used only in targets
> > h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL.
> > I have just confirmed this with a MAKEALL -a arm and a grep on all map
> > files.
> > 
> > This presence of mmutable in u-boot-spl.lds is in fact an overlook
> > that I missed when I created this file from u-boot.lds. I have just
> > finished verifying that removing the mmutable section altogether does
> > not change a single bit to any of the 309 ARM platforms currently built
> > under MAKEALL -a arm.
> > 
> > I'll remove mmutable entries from u-boot-spl.lds in V2.
> 
> OK, that's perfect for MMU data, but what about relocation data?

Relocation data should not exist for SPLs, which do not relocate.

Unfortunately, most tegra and some exynos have start.S code going
through the relocation loop even for their SPL; that's cardhu,
colibri_t20_iris, dalmore, harmony, medcom-wide, origen, paz00, plutux,
seaboard, smdkv310, tec, trimslice, ventana, and whistler.

Tegras do it for their arm720t; Exynos' probably do something similar.
I am not going to try and change some start.S so close in time to
release. :)

Fortunately, for all the SPLs that fail building if I remove .rel.dyn
and .dynsym, these sections are actually empty, i.e. their __end is
equal to their __image_copy_end. I have manually verified both reloc
section emptiness and equality of _end and __image_copy_end.

Therefore I'll leave the ASSERT() as written in V2, and will provide a
separate patch for fixing the Tegra / Exynos unneeded relocation data
issue.

> Best regards,
> Beno?t

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
semantics across all of U-boot. This patch series aims at fixing this by
splitting the maximum size into separate image (code + data + rodata +
linker list) size on the one hand, and BSS size on the other hand.

Changes in v2:
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
- removed mmutable in SPL linker file

Albert ARIBAUD (4):
  cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics
  da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics
  smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics
  ARM: fix CONFIG_SPL_MAX_SIZE semantics

 README                                         |   17 +++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds                    |   19 ++++++++-----------
 arch/arm/cpu/u-boot.lds                        |    4 ----
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
 include/configs/cam_enc_4xx.h                  |    4 +++-
 include/configs/da850evm.h                     |    4 +++-
 include/configs/exynos5250-dt.h                |    3 ++-
 9 files changed, 34 insertions(+), 23 deletions(-)

-- 
1.7.10.4

[U-Boot] [PATCH v2 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size.
Split 12K max size between 10K image (text,rodata,data)
and 2K BSS based on sizes reported for current build:

   text	   data	    bss
   9073	    840	    500

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v2:
- brought back total size to 12K

 board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
 include/configs/cam_enc_4xx.h        |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
index dd9d52d..25625dc 100644
--- a/board/ait/cam_enc_4xx/u-boot-spl.lds
+++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
index 56528dd..b73b72c 100644
--- a/include/configs/cam_enc_4xx.h
+++ b/include/configs/cam_enc_4xx.h
@@ -230,7 +230,9 @@
 #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
 
 #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
-#define CONFIG_SPL_MAX_SIZE		12320
+/* SPL total size is 12K -- but --pad-to requires a single decimal number */
+#define CONFIG_SPL_MAX_SIZE		10240
+#define CONFIG_SPL_BSS_MAX_SIZE		(2*1024)
 
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SYS_TEXT_BASE		0x81080000
-- 
1.7.10.4

[U-Boot] [PATCH v2 2/4] da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size.
Split 32K max size between 24K image (text,rodata,data)
and 8K BSS based on sizes reported for current build:

   text	   data	    bss
  15676	   1316	    108

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v2:
- fixed commit summary typoes

 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 include/configs/da850evm.h                     |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
index bc34fb5..d7edeb2 100644
--- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
+++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
index 99b4de7..e15c86e 100644
--- a/include/configs/da850evm.h
+++ b/include/configs/da850evm.h
@@ -399,7 +399,9 @@
 #define CONFIG_SPL_LDSCRIPT	"board/$(BOARDDIR)/u-boot-spl-da850evm.lds"
 #define CONFIG_SPL_STACK	0x8001ff00
 #define CONFIG_SPL_TEXT_BASE	0x80000000
-#define CONFIG_SPL_MAX_SIZE	32768
+/* SPL max size is 24K -- but --pad-to requires a single decimal number */
+#define CONFIG_SPL_MAX_SIZE	24576
+#define CONFIG_SPL_BSS_MAX_SIZE	(8*1024)
 #endif
 
 /* Load U-Boot Image From MMC */
-- 
1.7.10.4

[U-Boot] [PATCH v2 3/4] smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size.
Split 14K max size between 10K image (text,rodata,data)
and 4K BSS based on sizes reported for current build:

   text	   data	    bss
   4136	    904	      0

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v2:
- fixed spacing in commit summary

 board/samsung/smdk5250/smdk5250-uboot-spl.lds |    2 +-
 include/configs/exynos5250-dt.h               |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
index 4c8baaa..208e626 100644
--- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds
+++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
@@ -26,7 +26,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
index 496a194..372b0b4 100644
--- a/include/configs/exynos5250-dt.h
+++ b/include/configs/exynos5250-dt.h
@@ -141,7 +141,8 @@
 /* specific .lds file */
 #define CONFIG_SPL_LDSCRIPT	"board/samsung/smdk5250/smdk5250-uboot-spl.lds"
 #define CONFIG_SPL_TEXT_BASE	0x02023400
-#define CONFIG_SPL_MAX_SIZE	(14 * 1024)
+#define CONFIG_SPL_MAX_SIZE	(10 * 1024)
+#define CONFIG_SPL_BSS_MAX_SIZE	(4 * 1024)
 
 #define CONFIG_BOOTCOMMAND	"mmc read 40007000 451 2000; bootm 40007000"
 
-- 
1.7.10.4

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds
as this file is never used for SPL builds.

Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds
to separately test image (text,data,rodata...) size
and BSS size each against its own max.

Also, output section mmutable is not used in SPL builds.
Remove it.

Finally, update README regarding the (now homogeneous)
semantics of the CONFIG_SPL_MAX_SIZE and
CONFIG_SPL_BSS_MAX_SIZE macros.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
Reported-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
---
Changes in v2:
- removed mmutable in SPL linker file

 README                      |   17 +++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds |   19 ++++++++-----------
 arch/arm/cpu/u-boot.lds     |    4 ----
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/README b/README
index 5c5cd18..f31ded0 100644
--- a/README
+++ b/README
@@ -2776,7 +2776,14 @@ FIT uImage format:
 		LDSCRIPT for linking the SPL binary.
 
 		CONFIG_SPL_MAX_SIZE
-		Maximum binary size (text, data and rodata) of the SPL binary.
+		Maximum size for the image (sum of the text, data, rodata
+		and linker lists sections) of the SPL executable.
+		When defined, linker checks that the actual image size does
+		not exceed it.
+		The total amount of memory used by the SPL when running is
+		equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
 
 		CONFIG_SPL_TEXT_BASE
 		TEXT_BASE for linking the SPL binary.
@@ -2789,7 +2796,13 @@ FIT uImage format:
 		Link address for the BSS within the SPL binary.
 
 		CONFIG_SPL_BSS_MAX_SIZE
-		Maximum binary size of the BSS section of the SPL binary.
+		Maximum size of the BSS section of the SPL executable.
+		When defined, linker checks that the actual BSS size does
+		not exceed it.
+		The total amount of memory used by the SPL when running is
+		equal CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
 
 		CONFIG_SPL_STACK
 		Adress of the start of the stack SPL will use
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index 3c0d99c..2e86de4 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -65,15 +65,6 @@ SECTIONS
 
 	_end = .;
 
-	/*
-	 * Deprecated: this MMU section is used by pxa at present but
-	 * should not be used by new boards/CPUs.
-	 */
-	. = ALIGN(4096);
-	.mmutable : {
-		*(.mmutable)
-	}
-
 	.bss __rel_dyn_start (OVERLAY) : {
 		__bss_start = .;
 		*(.bss*)
@@ -88,6 +79,12 @@ SECTIONS
 	/DISCARD/ : { *(.gnu*) }
 }
 
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
+#if defined(CONFIG_SPL_MAX_SIZE)
+ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
+	"SPL image too big");
+#endif
+
+#if defined(CONFIG_SPL_BSS_MAX_SIZE)
+ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS MAX_SIZE), \
+	"SPL image BSS too big");
 #endif
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 3a1083d..7bbc4f5 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -101,7 +101,3 @@ SECTIONS
 	/DISCARD/ : { *(.interp*) }
 	/DISCARD/ : { *(.gnu*) }
 }
-
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
-#endif
-- 
1.7.10.4

[U-Boot] [PATCH v2 3/4] smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics

[Minkyu Kang]

On 10/04/13 08:14, Albert ARIBAUD wrote:
> CONFIG_SPL_MAX_SIZE wrongly included BSS size.
> Split 14K max size between 10K image (text,rodata,data)
> and 4K BSS based on sizes reported for current build:
> 
>    text	   data	    bss
>    4136	    904	      0
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
> Changes in v2:
> - fixed spacing in commit summary
> 
>  board/samsung/smdk5250/smdk5250-uboot-spl.lds |    2 +-
>  include/configs/exynos5250-dt.h               |    3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
> index 4c8baaa..208e626 100644
> --- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds
> +++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
> @@ -26,7 +26,7 @@
>   */
>  
>  MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
> -		LENGTH = CONFIG_SPL_MAX_SIZE }
> +		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
>  
>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>  OUTPUT_ARCH(arm)
> diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
> index 496a194..372b0b4 100644
> --- a/include/configs/exynos5250-dt.h
> +++ b/include/configs/exynos5250-dt.h
> @@ -141,7 +141,8 @@
>  /* specific .lds file */
>  #define CONFIG_SPL_LDSCRIPT	"board/samsung/smdk5250/smdk5250-uboot-spl.lds"
>  #define CONFIG_SPL_TEXT_BASE	0x02023400
> -#define CONFIG_SPL_MAX_SIZE	(14 * 1024)
> +#define CONFIG_SPL_MAX_SIZE	(10 * 1024)
> +#define CONFIG_SPL_BSS_MAX_SIZE	(4 * 1024)
>  
>  #define CONFIG_BOOTCOMMAND	"mmc read 40007000 451 2000; bootm 40007000"
>  
> 

Acked-by: Minkyu Kang <mk7.kang@samsung.com>

Thanks,
Minkyu Kang.

[U-Boot] [PATCH v2 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Heiko Schocher]

Hello Albert,

On 10.04.2013 01:14, Albert ARIBAUD wrote:
> CONFIG_SPL_MAX_SIZE wrongly included BSS size.
> Split 12K max size between 10K image (text,rodata,data)
> and 2K BSS based on sizes reported for current build:
> 
>    text	   data	    bss
>    9073	    840	    500
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>

Thanks!

Acked-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Stephen Warren]

On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
> Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds
> as this file is never used for SPL builds.
> 
> Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds
> to separately test image (text,data,rodata...) size
> and BSS size each against its own max.
> 
> Also, output section mmutable is not used in SPL builds.
> Remove it.
> 
> Finally, update README regarding the (now homogeneous)
> semantics of the CONFIG_SPL_MAX_SIZE and
> CONFIG_SPL_BSS_MAX_SIZE macros.

This still seems to have separate defines for SPL text/data/rodata size
and BSS size. If I want instead to limit the total text/data/rodata/bss
size, but place no specific limit on the bss size individually, can I
not do that?

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Stephen,

On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:

> On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
> > Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds
> > as this file is never used for SPL builds.
> > 
> > Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds
> > to separately test image (text,data,rodata...) size
> > and BSS size each against its own max.
> > 
> > Also, output section mmutable is not used in SPL builds.
> > Remove it.
> > 
> > Finally, update README regarding the (now homogeneous)
> > semantics of the CONFIG_SPL_MAX_SIZE and
> > CONFIG_SPL_BSS_MAX_SIZE macros.
> 
> This still seems to have separate defines for SPL text/data/rodata size
> and BSS size. If I want instead to limit the total text/data/rodata/bss
> size, but place no specific limit on the bss size individually, can I
> not do that?

This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant
meaning -- one of the issues which prompted this patch series is that
in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime
excluded, and this inconsistency had to be resolved. As in the rest of
U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the
semantics that was decided.

What we could do, though, is subdivide testing based on the existence or
non-existence of CONFIG_SPL_BSS_START_ADDR:

- if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
  BSS are disjoint and we test each one against its max size, as this
  patch series does;

- if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
  and BSS are contiguous and we test the whole of SPL against the sum
  of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.

I guess this will be considered useless complication -- after all,
once you have artificially partitioned your SPL space into image+BSS --
and you know from the build command how much should be allotted to each
of them -- the worst that can happen is that a later build fails with
an explicit error message forcing you to look at current image and BSS
size and adjust one or both of the max values accordingly.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Stephen Warren]

On 04/10/2013 04:50 PM, Albert ARIBAUD wrote:
> On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren wrote:
>> On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
...
>> This still seems to have separate defines for SPL text/data/rodata size
>> and BSS size. If I want instead to limit the total text/data/rodata/bss
>> size, but place no specific limit on the bss size individually, can I
>> not do that?
> 
> This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant
> meaning -- one of the issues which prompted this patch series is that
> in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime
> excluded, and this inconsistency had to be resolved. As in the rest of
> U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the
> semantics that was decided.
> 
> What we could do, though, is subdivide testing based on the existence or
> non-existence of CONFIG_SPL_BSS_START_ADDR:
> 
> - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
>   BSS are disjoint and we test each one against its max size, as this
>   patch series does;
> 
> - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
>   and BSS are contiguous and we test the whole of SPL against the sum
>   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.

Why not either:

a) define CONFIG_SPL_MAX_SIZE to include the BSS size if
CONFIG_SPL_BSS_START_ADDR is not set, but exclude it if it is.

or:

b) use 3 defines instead of 2, so that CONFIG_SPL_MAX_SIZE (if defined)
always limits text+rodata+data, CONFIG_SPL_MAX_FOOTPRINT (if defined)
always limits text+rodata+data+bss, and CONFIG_SPL_BSS_MAX_SIZE (if
defined) always limits bss size.

Tegra would define only CONFIG_SPL_MAX_FOOTPRINT in this scheme. Other
boards would presumably define other combinations of those.

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> What we could do, though, is subdivide testing based on the existence or
> non-existence of CONFIG_SPL_BSS_START_ADDR:
> 
> - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
>   BSS are disjoint and we test each one against its max size, as this
>   patch series does;
> 
> - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
>   and BSS are contiguous and we test the whole of SPL against the sum
>   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
> 
> I guess this will be considered useless complication -- after all,
> once you have artificially partitioned your SPL space into image+BSS --
> and you know from the build command how much should be allotted to each
> of them -- the worst that can happen is that a later build fails with
> an explicit error message forcing you to look at current image and BSS
> size and adjust one or both of the max values accordingly.

P.S. In any case, the proposal above will go in, if at all, as a
separate patch; the current patch series is going in right now as it is.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

On Wed, 10 Apr 2013 01:14:51 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
> semantics across all of U-boot. This patch series aims at fixing this by
> splitting the maximum size into separate image (code + data + rodata +
> linker list) size on the one hand, and BSS size on the other hand.
> 
> Changes in v2:
> - brought back total size to 12K
> - fixed commit summary typoes
> - fixed spacing in commit summary
> - removed mmutable in SPL linker file
> 
> Albert ARIBAUD (4):
>   cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics
>   da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics
>   smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics
>   ARM: fix CONFIG_SPL_MAX_SIZE semantics
> 
>  README                                         |   17 +++++++++++++++--
>  arch/arm/cpu/u-boot-spl.lds                    |   19 ++++++++-----------
>  arch/arm/cpu/u-boot.lds                        |    4 ----
>  board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
>  board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
>  board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
>  include/configs/cam_enc_4xx.h                  |    4 +++-
>  include/configs/da850evm.h                     |    4 +++-
>  include/configs/exynos5250-dt.h                |    3 ++-
>  9 files changed, 34 insertions(+), 23 deletions(-)
> 

Applied to u-boot-arm/master.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Stephen Warren]

On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
> On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> 
>> What we could do, though, is subdivide testing based on the existence or
>> non-existence of CONFIG_SPL_BSS_START_ADDR:
>>
>> - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
>>   BSS are disjoint and we test each one against its max size, as this
>>   patch series does;
>>
>> - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
>>   and BSS are contiguous and we test the whole of SPL against the sum
>>   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
>>
>> I guess this will be considered useless complication -- after all,
>> once you have artificially partitioned your SPL space into image+BSS --
>> and you know from the build command how much should be allotted to each
>> of them -- the worst that can happen is that a later build fails with
>> an explicit error message forcing you to look at current image and BSS
>> size and adjust one or both of the max values accordingly.
> 
> P.S. In any case, the proposal above will go in, if at all, as a
> separate patch; the current patch series is going in right now as it is.

I wonder what the point of code-review is if you're just going to ignore it.

What's really odd here is that by my reading of the relevant threads,
TomR already pointed out this exact issue earlier on, and you had agreed
that you'd resolve it in a way that didn't have this issue, yet the
patch has this issue???

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Stephen,

On Wed, 10 Apr 2013 17:09:45 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:

> On 04/10/2013 04:50 PM, Albert ARIBAUD wrote:
> > On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren wrote:
> >> On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
> ...
> >> This still seems to have separate defines for SPL text/data/rodata size
> >> and BSS size. If I want instead to limit the total text/data/rodata/bss
> >> size, but place no specific limit on the bss size individually, can I
> >> not do that?
> > 
> > This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant
> > meaning -- one of the issues which prompted this patch series is that
> > in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime
> > excluded, and this inconsistency had to be resolved. As in the rest of
> > U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the
> > semantics that was decided.
> > 
> > What we could do, though, is subdivide testing based on the existence or
> > non-existence of CONFIG_SPL_BSS_START_ADDR:
> > 
> > - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
> >   BSS are disjoint and we test each one against its max size, as this
> >   patch series does;
> > 
> > - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
> >   and BSS are contiguous and we test the whole of SPL against the sum
> >   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
> 
> Why not either:
> 
> a) define CONFIG_SPL_MAX_SIZE to include the BSS size if
> CONFIG_SPL_BSS_START_ADDR is not set, but exclude it if it is.

That was in my original proposals, and it was spoken against:
<http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073>
<http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094>

> or:
> 
> b) use 3 defines instead of 2, so that CONFIG_SPL_MAX_SIZE (if defined)
> always limits text+rodata+data, CONFIG_SPL_MAX_FOOTPRINT (if defined)
> always limits text+rodata+data+bss, and CONFIG_SPL_BSS_MAX_SIZE (if
> defined) always limits bss size.
> 
> Tegra would define only CONFIG_SPL_MAX_FOOTPRINT in this scheme.
> Other boards would presumably define other combinations of those.

That is a possibility, with the minor nitpick that we could possibly
end up having conflicting values for the three symbols, whereas with
the CONFIG_SPL_BSS_START_ADDR solution, there can be no value conflict.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Stephen,

On Wed, 10 Apr 2013 17:16:49 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:

> On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
> > On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD
> > <albert.u.boot@aribaud.net> wrote:
> > 
> >> What we could do, though, is subdivide testing based on the existence or
> >> non-existence of CONFIG_SPL_BSS_START_ADDR:
> >>
> >> - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
> >>   BSS are disjoint and we test each one against its max size, as this
> >>   patch series does;
> >>
> >> - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
> >>   and BSS are contiguous and we test the whole of SPL against the sum
> >>   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
> >>
> >> I guess this will be considered useless complication -- after all,
> >> once you have artificially partitioned your SPL space into image+BSS --
> >> and you know from the build command how much should be allotted to each
> >> of them -- the worst that can happen is that a later build fails with
> >> an explicit error message forcing you to look at current image and BSS
> >> size and adjust one or both of the max values accordingly.
> > 
> > P.S. In any case, the proposal above will go in, if at all, as a
> > separate patch; the current patch series is going in right now as it is.
> 
> I wonder what the point of code-review is if you're just going to ignore it.

Can we please avoid this kind of talk? It has no argumentative value
and can only lead to conflicts. Your account below is sufficient to
convey your argumentation without any of the potential ill-effects of
the above.

> What's really odd here is that by my reading of the relevant threads,
> TomR already pointed out this exact issue earlier on, and you had agreed
> that you'd resolve it in a way that didn't have this issue, yet the
> patch has this issue???

Then our reading of the thread do not agree. Here is mine:

- in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158046>,
  Tom clearly asks for separate text+data+rodata size on one hand and
  BSS size on the other hand (his #2 case, which he wants applied
  uniformally and a solution found for Tegra.

- in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073> I
  replied to Tom with a proposal in two parts, the first implementing
  his #2 case strictly, the second implementing case #1 at the cost of
  some minor added complexity and of muddying the symbol's semantics; I
  suggested that if Tom really did not want the second part of my
  proposal, it could be dropped and only the first part implemented.

- in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094>,
  Tom asked that I keep part 1 and drop part 2 -- which I did.

Additionally, I did ask Tom on IRC if V2 was ok with him, and had his
agreement.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Tom Rini]

On Thu, Apr 11, 2013 at 04:32:53PM +0200, Albert ARIBAUD wrote:
> Hi Stephen,
> 
> On Wed, 10 Apr 2013 17:16:49 -0600, Stephen Warren
> <swarren@wwwdotorg.org> wrote:
> 
> > On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
> > > On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD
> > > <albert.u.boot@aribaud.net> wrote:
> > > 
> > >> What we could do, though, is subdivide testing based on the existence or
> > >> non-existence of CONFIG_SPL_BSS_START_ADDR:
> > >>
> > >> - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
> > >>   BSS are disjoint and we test each one against its max size, as this
> > >>   patch series does;
> > >>
> > >> - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
> > >>   and BSS are contiguous and we test the whole of SPL against the sum
> > >>   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
> > >>
> > >> I guess this will be considered useless complication -- after all,
> > >> once you have artificially partitioned your SPL space into image+BSS --
> > >> and you know from the build command how much should be allotted to each
> > >> of them -- the worst that can happen is that a later build fails with
> > >> an explicit error message forcing you to look at current image and BSS
> > >> size and adjust one or both of the max values accordingly.
> > > 
> > > P.S. In any case, the proposal above will go in, if at all, as a
> > > separate patch; the current patch series is going in right now as it is.
> > 
> > I wonder what the point of code-review is if you're just going to ignore it.
> 
> Can we please avoid this kind of talk? It has no argumentative value
> and can only lead to conflicts. Your account below is sufficient to
> convey your argumentation without any of the potential ill-effects of
> the above.
> 
> > What's really odd here is that by my reading of the relevant threads,
> > TomR already pointed out this exact issue earlier on, and you had agreed
> > that you'd resolve it in a way that didn't have this issue, yet the
> > patch has this issue???
> 
> Then our reading of the thread do not agree. Here is mine:
> 
> - in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158046>,
>   Tom clearly asks for separate text+data+rodata size on one hand and
>   BSS size on the other hand (his #2 case, which he wants applied
>   uniformally and a solution found for Tegra.
> 
> - in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073> I
>   replied to Tom with a proposal in two parts, the first implementing
>   his #2 case strictly, the second implementing case #1 at the cost of
>   some minor added complexity and of muddying the symbol's semantics; I
>   suggested that if Tom really did not want the second part of my
>   proposal, it could be dropped and only the first part implemented.
> 
> - in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094>,
>   Tom asked that I keep part 1 and drop part 2 -- which I did.
> 
> Additionally, I did ask Tom on IRC if V2 was ok with him, and had his
> agreement.

To be clear, I think the fake partitioning scheme we use when the case
is "we care about text/data/rodata/bss fitting in $X" is not optimal,
but it covers all of the cases without adding more complexity / another
way to achieve the same ends.  If it turns out that platforms end up
needing to re-tweak this value we can come back and say it's time to add
a different mechanism for this hard-limit case.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130411/2a1970ed/attachment.pgp>

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Stephen Warren]

On 04/11/2013 10:08 AM, Tom Rini wrote:
...
> To be clear, I think the fake partitioning scheme we use when the
> case is "we care about text/data/rodata/bss fitting in $X" is not
> optimal, but it covers all of the cases without adding more
> complexity / another way to achieve the same ends.

But it does add more complexity; somebody has to pick, and potentially
keep adjusting, the split between text/rodata/data and bss, even
though they don't care about that split, but it's an implementation
wart. It's also trivial to fix it properly as I described.

But, if it absolutely has to be that way, then OK. However, I would
ask that at least the Tegra board configuration files be set up so
that the previous overall restriction (bss doesn't overlap the main
U-Boot) be left in tact, so we don't have to debug that problem again.

[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Stephen,

On Wed, 10 Apr 2013 17:09:45 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:

> On 04/10/2013 04:50 PM, Albert ARIBAUD wrote:
> > On Wed, 10 Apr 2013 16:21:54 -0600, Stephen Warren wrote:
> >> On 04/09/2013 05:14 PM, Albert ARIBAUD wrote:
> ...
> >> This still seems to have separate defines for SPL text/data/rodata size
> >> and BSS size. If I want instead to limit the total text/data/rodata/bss
> >> size, but place no specific limit on the bss size individually, can I
> >> not do that?
> > 
> > This would defeat the purpose of giving CONFIG_SPL_MAX_SIZE a constant
> > meaning -- one of the issues which prompted this patch series is that
> > in ARM sometime CONFIG_SPL_MAX_SIZE meant BSS included, and sometime
> > excluded, and this inconsistency had to be resolved. As in the rest of
> > U-boot, CONFIG_SPL_MAX_SIZE was meant BSS excluded, this is the
> > semantics that was decided.
> > 
> > What we could do, though, is subdivide testing based on the existence or
> > non-existence of CONFIG_SPL_BSS_START_ADDR:
> > 
> > - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
> >   BSS are disjoint and we test each one against its max size, as this
> >   patch series does;
> > 
> > - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
> >   and BSS are contiguous and we test the whole of SPL against the sum
> >   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
> 
> Why not either:
> 
> a) define CONFIG_SPL_MAX_SIZE to include the BSS size if
> CONFIG_SPL_BSS_START_ADDR is not set, but exclude it if it is.
> 
> or:
> 
> b) use 3 defines instead of 2, so that CONFIG_SPL_MAX_SIZE (if defined)
> always limits text+rodata+data, CONFIG_SPL_MAX_FOOTPRINT (if defined)
> always limits text+rodata+data+bss, and CONFIG_SPL_BSS_MAX_SIZE (if
> defined) always limits bss size.
> 
> Tegra would define only CONFIG_SPL_MAX_FOOTPRINT in this scheme. Other
> boards would presumably define other combinations of those.

Tom R on IRC showed preference for CONFIG_SPL_MAX_FOOTPRINT -- I'll
send out a V3 with this solution.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

On Thu, 11 Apr 2013 01:10:17 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> On Wed, 10 Apr 2013 01:14:51 +0200, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> 
> > CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
> > semantics across all of U-boot. This patch series aims at fixing this by
> > splitting the maximum size into separate image (code + data + rodata +
> > linker list) size on the one hand, and BSS size on the other hand.
> > 
> > Changes in v2:
> > - brought back total size to 12K
> > - fixed commit summary typoes
> > - fixed spacing in commit summary
> > - removed mmutable in SPL linker file
> > 
> > Albert ARIBAUD (4):
> >   cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics
> >   da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics
> >   smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics
> >   ARM: fix CONFIG_SPL_MAX_SIZE semantics
> > 
> >  README                                         |   17 +++++++++++++++--
> >  arch/arm/cpu/u-boot-spl.lds                    |   19 ++++++++-----------
> >  arch/arm/cpu/u-boot.lds                        |    4 ----
> >  board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
> >  board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
> >  board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
> >  include/configs/cam_enc_4xx.h                  |    4 +++-
> >  include/configs/da850evm.h                     |    4 +++-
> >  include/configs/exynos5250-dt.h                |    3 ++-
> >  9 files changed, 34 insertions(+), 23 deletions(-)
> > 
> 
> Applied to u-boot-arm/master.

... and then rolled back.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v3 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
semantics across all of U-boot. This patch series aims at fixing this by
splitting the maximum size into separate image (code + data + rodata +
linker list) size on the one hand, and BSS size on the other hand.

Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test

Changes in v2:
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary
- removed mmutable in SPL linker file

Albert ARIBAUD (4):
  cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics
  da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics
  smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics
  ARM: fix CONFIG_SPL_MAX_SIZE semantics

 README                                         |   26 ++++++++++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds                    |   24 ++++++++++++----------
 arch/arm/cpu/u-boot.lds                        |    4 ----
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
 include/configs/cam_enc_4xx.h                  |    4 +++-
 include/configs/da850evm.h                     |    4 +++-
 include/configs/exynos5250-dt.h                |    3 ++-
 include/configs/tegra-common.h                 |    2 +-
 10 files changed, 49 insertions(+), 24 deletions(-)

-- 
1.7.10.4

[U-Boot] [PATCH v3 1/4] cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size.
Split 12K max size between 10K image (text,rodata,data)
and 2K BSS based on sizes reported for current build:

   text	   data	    bss
   9073	    840	    500

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v3: None
Changes in v2:
- brought back total size to 12K

 board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
 include/configs/cam_enc_4xx.h        |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
index be1027d..18a152d 100644
--- a/board/ait/cam_enc_4xx/u-boot-spl.lds
+++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
index 56528dd..b73b72c 100644
--- a/include/configs/cam_enc_4xx.h
+++ b/include/configs/cam_enc_4xx.h
@@ -230,7 +230,9 @@
 #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
 
 #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
-#define CONFIG_SPL_MAX_SIZE		12320
+/* SPL total size is 12K -- but --pad-to requires a single decimal number */
+#define CONFIG_SPL_MAX_SIZE		10240
+#define CONFIG_SPL_BSS_MAX_SIZE		(2*1024)
 
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SYS_TEXT_BASE		0x81080000
-- 
1.7.10.4

[U-Boot] [PATCH v3 2/4] da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size.
Split 32K max size between 24K image (text,rodata,data)
and 8K BSS based on sizes reported for current build:

   text	   data	    bss
  15676	   1316	    108

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v3: None
Changes in v2:
- fixed commit summary typoes

 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 include/configs/da850evm.h                     |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
index 2ae5a2c..25d6728 100644
--- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
+++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
index 99b4de7..e15c86e 100644
--- a/include/configs/da850evm.h
+++ b/include/configs/da850evm.h
@@ -399,7 +399,9 @@
 #define CONFIG_SPL_LDSCRIPT	"board/$(BOARDDIR)/u-boot-spl-da850evm.lds"
 #define CONFIG_SPL_STACK	0x8001ff00
 #define CONFIG_SPL_TEXT_BASE	0x80000000
-#define CONFIG_SPL_MAX_SIZE	32768
+/* SPL max size is 24K -- but --pad-to requires a single decimal number */
+#define CONFIG_SPL_MAX_SIZE	24576
+#define CONFIG_SPL_BSS_MAX_SIZE	(8*1024)
 #endif
 
 /* Load U-Boot Image From MMC */
-- 
1.7.10.4

[U-Boot] [PATCH v3 3/4] smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE wrongly included BSS size.
Split 14K max size between 10K image (text,rodata,data)
and 4K BSS based on sizes reported for current build:

   text	   data	    bss
   4136	    904	      0

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v3: None
Changes in v2:
- fixed spacing in commit summary

 board/samsung/smdk5250/smdk5250-uboot-spl.lds |    2 +-
 include/configs/exynos5250-dt.h               |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
index 7df0a1d..c45cc37 100644
--- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds
+++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
@@ -26,7 +26,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = (CONFIG_SPL_MAX_SIZE + CONFIG_SPL_BSS_MAX_SIZE) }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
index 496a194..372b0b4 100644
--- a/include/configs/exynos5250-dt.h
+++ b/include/configs/exynos5250-dt.h
@@ -141,7 +141,8 @@
 /* specific .lds file */
 #define CONFIG_SPL_LDSCRIPT	"board/samsung/smdk5250/smdk5250-uboot-spl.lds"
 #define CONFIG_SPL_TEXT_BASE	0x02023400
-#define CONFIG_SPL_MAX_SIZE	(14 * 1024)
+#define CONFIG_SPL_MAX_SIZE	(10 * 1024)
+#define CONFIG_SPL_BSS_MAX_SIZE	(4 * 1024)
 
 #define CONFIG_BOOTCOMMAND	"mmc read 40007000 451 2000; bootm 40007000"
 
-- 
1.7.10.4

[U-Boot] [PATCH v3 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds
as this file is never used for SPL builds.

Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds
to separately test image (text,data,rodata...) size,
BSS size, and full footprint each against its own max,
and make Tegra boards check full footprint.

Also, output section mmutable is not used in SPL builds.
Remove it.

Finally, update README regarding the (now homogeneous)
semantics of CONFIG_SPL_[BSS_]MAX_SIZE and add the new
CONFIG_SPL_MAX_FOOTPRINT macro.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
Reported-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
---
Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test

Changes in v2:
- removed mmutable in SPL linker file

 README                         |   26 ++++++++++++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds    |   24 +++++++++++++-----------
 arch/arm/cpu/u-boot.lds        |    4 ----
 include/configs/tegra-common.h |    2 +-
 4 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/README b/README
index 6272853..c470489 100644
--- a/README
+++ b/README
@@ -2775,8 +2775,24 @@ FIT uImage format:
 		CONFIG_SPL_LDSCRIPT
 		LDSCRIPT for linking the SPL binary.
 
+		CONFIG_SPL_MAX_FOOTPRINT
+		Maximum size in memory allocated to the SPL.
+		When defined, linker checks that the actual memory used
+		by SPL size does not exceed it.
+		CONFIG_MAX_FOOTPRINT should always be equal to or greater
+		than CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
+
 		CONFIG_SPL_MAX_SIZE
-		Maximum binary size (text, data and rodata) of the SPL binary.
+		Maximum size for the image (sum of the text, data, rodata
+		and linker lists sections) of the SPL executable.
+		When defined, linker checks that the actual image size does
+		not exceed it.
+		CONFIG_MAX_FOOTPRINT should always be equal to or greater
+		than CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
 
 		CONFIG_SPL_TEXT_BASE
 		TEXT_BASE for linking the SPL binary.
@@ -2789,7 +2805,13 @@ FIT uImage format:
 		Link address for the BSS within the SPL binary.
 
 		CONFIG_SPL_BSS_MAX_SIZE
-		Maximum binary size of the BSS section of the SPL binary.
+		Maximum size for the BSS section of the SPL executable.
+		When defined, linker checks that the actual BSS size does
+		not exceed it.
+		CONFIG_MAX_FOOTPRINT should always be equal to or greater
+		than CONFIG_SPL_MAX_SIZE, plus CONFIG_SPL_BSS_MAX_SIZE if
+		it exists.
+		Note: image and BSS are disjoint for some targets.
 
 		CONFIG_SPL_STACK
 		Adress of the start of the stack SPL will use
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index 3c0d99c..1408f03 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -65,15 +65,6 @@ SECTIONS
 
 	_end = .;
 
-	/*
-	 * Deprecated: this MMU section is used by pxa at present but
-	 * should not be used by new boards/CPUs.
-	 */
-	. = ALIGN(4096);
-	.mmutable : {
-		*(.mmutable)
-	}
-
 	.bss __rel_dyn_start (OVERLAY) : {
 		__bss_start = .;
 		*(.bss*)
@@ -88,6 +79,17 @@ SECTIONS
 	/DISCARD/ : { *(.gnu*) }
 }
 
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
+#if defined(CONFIG_SPL_MAX_SIZE)
+ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
+	"SPL image too big");
+#endif
+
+#if defined(CONFIG_SPL_BSS_MAX_SIZE)
+ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
+	"SPL image BSS too big");
+#endif
+
+#if defined(CONFIG_SPL_MAX_FOOTPRINT)
+ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
+	"SPL image plus BSS too big");
 #endif
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 3a1083d..7bbc4f5 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -101,7 +101,3 @@ SECTIONS
 	/DISCARD/ : { *(.interp*) }
 	/DISCARD/ : { *(.gnu*) }
 }
-
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
-#endif
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index 036ded0..e9241b8 100644
--- a/include/configs/tegra-common.h
+++ b/include/configs/tegra-common.h
@@ -158,7 +158,7 @@
 #define CONFIG_SPL_RAM_DEVICE
 #define CONFIG_SPL_BOARD_INIT
 #define CONFIG_SPL_NAND_SIMPLE
-#define CONFIG_SPL_MAX_SIZE		(CONFIG_SYS_TEXT_BASE - \
+#define CONFIG_SPL_MAX_FOOTPRINT	(CONFIG_SYS_TEXT_BASE - \
 						CONFIG_SPL_TEXT_BASE)
 #define CONFIG_SYS_SPL_MALLOC_SIZE	0x00010000
 
-- 
1.7.10.4

[U-Boot] [PATCH v3 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Tom Rini]

On Fri, Apr 12, 2013 at 01:55:39PM +0200, Albert ARIBAUD wrote:

> CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
> semantics across all of U-boot. This patch series aims at fixing this by
> splitting the maximum size into separate image (code + data + rodata +
> linker list) size on the one hand, and BSS size on the other hand.
> 
> Changes in v3:
> - introduced CONFIG_SPL_MAX_FOOTPRINT
> - fixed typo in BSS size test
> 
> Changes in v2:
> - brought back total size to 12K
> - fixed commit summary typoes
> - fixed spacing in commit summary
> - removed mmutable in SPL linker file
> 
> Albert ARIBAUD (4):
>   cam_enc_4xx: fix CONFIG_SPL_MAX_SIZE semantics
>   da850evm, da850_am18xxevm: fix CONFIG_SPL_MAX_SIZE semantics
>   smdk5250, snow: fix CONFIG_SPL_MAX_SIZE semantics
>   ARM: fix CONFIG_SPL_MAX_SIZE semantics

The problem is that cam_enc_4xx, da850* and quite likely smdk5250 all
want the new CONFIG_SPL_MAX_FOOTPRINT and not an arbitrary split between
BSS and everything else.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130412/b6d8a6a4/attachment.pgp>

[U-Boot] [PATCH v4 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
semantics across all of U-boot. This patch series aims at fixing this by
splitting the maximum size into separate image (code + data + rodata +
linker list) size on the one hand, and BSS size on the other hand.

Changes in v4:
- reordered patches so that FOOTPRINT is introduced first,
  then individual boards are switched to using it
- rewrote README entries
- converted to CONFIG_SPL_MAX_FOOTPRINT
- limited SPL size to exactly 6 2K pages
- converted to CONFIG_SPL_MAX_FOOTPRINT
- converted to CONFIG_SPL_MAX_FOOTPRINT

Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test

Changes in v2:
- removed mmutable in SPL linker file
- brought back total size to 12K
- fixed commit summary typoes
- fixed spacing in commit summary

Albert ARIBAUD (4):
  ARM: fix CONFIG_SPL_MAX_SIZE semantics
  cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT
  da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT
  smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT

 README                                         |   18 ++++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds                    |   24 +++++++++++++-----------
 arch/arm/cpu/u-boot.lds                        |    4 ----
 board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
 include/configs/cam_enc_4xx.h                  |    2 +-
 include/configs/da850evm.h                     |    2 +-
 include/configs/exynos5250-dt.h                |    2 +-
 include/configs/tegra-common.h                 |    2 +-
 10 files changed, 36 insertions(+), 24 deletions(-)

-- 
1.7.10.4

[U-Boot] [PATCH v4 1/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Remove SPL-related ASSERT() in arch/arm/cpu/u-boot.lds
as this file is never used for SPL builds.

Rewrite the ASSERT() in arch/arm/cpu/u-boot-spl.lds
to separately test image (text,data,rodata...) size,
BSS size, and full footprint each against its own max,
and make Tegra boards check full footprint.

Also, output section mmutable is not used in SPL builds.
Remove it.

Finally, update README regarding the (now homogeneous)
semantics of CONFIG_SPL_[BSS_]MAX_SIZE and add the new
CONFIG_SPL_MAX_FOOTPRINT macro.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
Reported-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
---
Changes in v4:
- rewrote README entries

Changes in v3:
- introduced CONFIG_SPL_MAX_FOOTPRINT
- fixed typo in BSS size test

Changes in v2:
- removed mmutable in SPL linker file

 README                         |   18 ++++++++++++++++--
 arch/arm/cpu/u-boot-spl.lds    |   24 +++++++++++++-----------
 arch/arm/cpu/u-boot.lds        |    4 ----
 include/configs/tegra-common.h |    2 +-
 4 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/README b/README
index 6272853..f544c88 100644
--- a/README
+++ b/README
@@ -2775,8 +2775,18 @@ FIT uImage format:
 		CONFIG_SPL_LDSCRIPT
 		LDSCRIPT for linking the SPL binary.
 
+		CONFIG_SPL_MAX_FOOTPRINT
+		Maximum size in memory allocated to the SPL, BSS included.
+		When defined, the linker checks that the actual memory
+		used by SPL from _start to __bss_end does not exceed it.
+		CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not
+		be both defined at the same time.
+
 		CONFIG_SPL_MAX_SIZE
-		Maximum binary size (text, data and rodata) of the SPL binary.
+		Maximum size of the SPL image (text, data, rodata, and
+		linker lists sections), BSS excluded.
+		When defined, the linker checks that the actual size does
+		not exceed it.
 
 		CONFIG_SPL_TEXT_BASE
 		TEXT_BASE for linking the SPL binary.
@@ -2789,7 +2799,11 @@ FIT uImage format:
 		Link address for the BSS within the SPL binary.
 
 		CONFIG_SPL_BSS_MAX_SIZE
-		Maximum binary size of the BSS section of the SPL binary.
+		Maximum size in memory allocated to the SPL BSS.
+		When defined, the linker checks that the actual memory used
+		by SPL from __bss_start to __bss_end does not exceed it.
+		CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not
+		be both defined at the same time.
 
 		CONFIG_SPL_STACK
 		Adress of the start of the stack SPL will use
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index 3c0d99c..1408f03 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -65,15 +65,6 @@ SECTIONS
 
 	_end = .;
 
-	/*
-	 * Deprecated: this MMU section is used by pxa at present but
-	 * should not be used by new boards/CPUs.
-	 */
-	. = ALIGN(4096);
-	.mmutable : {
-		*(.mmutable)
-	}
-
 	.bss __rel_dyn_start (OVERLAY) : {
 		__bss_start = .;
 		*(.bss*)
@@ -88,6 +79,17 @@ SECTIONS
 	/DISCARD/ : { *(.gnu*) }
 }
 
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
+#if defined(CONFIG_SPL_MAX_SIZE)
+ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
+	"SPL image too big");
+#endif
+
+#if defined(CONFIG_SPL_BSS_MAX_SIZE)
+ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
+	"SPL image BSS too big");
+#endif
+
+#if defined(CONFIG_SPL_MAX_FOOTPRINT)
+ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
+	"SPL image plus BSS too big");
 #endif
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 3a1083d..7bbc4f5 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -101,7 +101,3 @@ SECTIONS
 	/DISCARD/ : { *(.interp*) }
 	/DISCARD/ : { *(.gnu*) }
 }
-
-#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
-ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL image too big");
-#endif
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index 036ded0..e9241b8 100644
--- a/include/configs/tegra-common.h
+++ b/include/configs/tegra-common.h
@@ -158,7 +158,7 @@
 #define CONFIG_SPL_RAM_DEVICE
 #define CONFIG_SPL_BOARD_INIT
 #define CONFIG_SPL_NAND_SIMPLE
-#define CONFIG_SPL_MAX_SIZE		(CONFIG_SYS_TEXT_BASE - \
+#define CONFIG_SPL_MAX_FOOTPRINT	(CONFIG_SYS_TEXT_BASE - \
 						CONFIG_SPL_TEXT_BASE)
 #define CONFIG_SYS_SPL_MALLOC_SIZE	0x00010000
 
-- 
1.7.10.4

[U-Boot] [PATCH v4 2/4] cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT

[Albert ARIBAUD]

This target wants to check full SPL size, BSS included.
Remove CONFIG_SPL_MAX_SIZE definition and instead define
CONFIG_SPL_MAX_FOOTPRINT.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v4:
- converted to CONFIG_SPL_MAX_FOOTPRINT
- limited SPL size to exactly 6 2K pages

Changes in v3: None
Changes in v2:
- brought back total size to 12K

 board/ait/cam_enc_4xx/u-boot-spl.lds |    2 +-
 include/configs/cam_enc_4xx.h        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/ait/cam_enc_4xx/u-boot-spl.lds b/board/ait/cam_enc_4xx/u-boot-spl.lds
index be1027d..1daa1b3 100644
--- a/board/ait/cam_enc_4xx/u-boot-spl.lds
+++ b/board/ait/cam_enc_4xx/u-boot-spl.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = CONFIG_SPL_MAX_FOOTPRINT }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h
index 56528dd..b27551d 100644
--- a/include/configs/cam_enc_4xx.h
+++ b/include/configs/cam_enc_4xx.h
@@ -230,7 +230,7 @@
 #define CONFIG_SPL_STACK		(0x00010000 + 0x7f00)
 
 #define CONFIG_SPL_TEXT_BASE		0x00000020 /*CONFIG_SYS_SRAM_START*/
-#define CONFIG_SPL_MAX_SIZE		12320
+#define CONFIG_SPL_MAX_FOOTPRINT	12288
 
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SYS_TEXT_BASE		0x81080000
-- 
1.7.10.4

[U-Boot] [PATCH v4 3/4] da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT

[Albert ARIBAUD]

This target wants to check full SPL size, BSS included.
Remove CONFIG_SPL_MAX_SIZE definition and instead define
CONFIG_SPL_MAX_FOOTPRINT.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v4:
- converted to CONFIG_SPL_MAX_FOOTPRINT

Changes in v3: None
Changes in v2:
- fixed commit summary typoes

 board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
 include/configs/da850evm.h                     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
index 2ae5a2c..b1b8701 100644
--- a/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
+++ b/board/davinci/da8xxevm/u-boot-spl-da850evm.lds
@@ -25,7 +25,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = CONFIG_SPL_MAX_FOOTPRINT }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
index 99b4de7..583568d 100644
--- a/include/configs/da850evm.h
+++ b/include/configs/da850evm.h
@@ -399,7 +399,7 @@
 #define CONFIG_SPL_LDSCRIPT	"board/$(BOARDDIR)/u-boot-spl-da850evm.lds"
 #define CONFIG_SPL_STACK	0x8001ff00
 #define CONFIG_SPL_TEXT_BASE	0x80000000
-#define CONFIG_SPL_MAX_SIZE	32768
+#define CONFIG_SPL_MAX_FOOTPRINT	32768
 #endif
 
 /* Load U-Boot Image From MMC */
-- 
1.7.10.4

[U-Boot] [PATCH v4 4/4] smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT

[Albert ARIBAUD]

This target wants to check full SPL size, BSS included.
Remove CONFIG_SPL_MAX_SIZE definition and instead define
CONFIG_SPL_MAX_FOOTPRINT.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
Changes in v4:
- converted to CONFIG_SPL_MAX_FOOTPRINT

Changes in v3: None
Changes in v2:
- fixed spacing in commit summary

 board/samsung/smdk5250/smdk5250-uboot-spl.lds |    2 +-
 include/configs/exynos5250-dt.h               |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250-uboot-spl.lds b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
index 7df0a1d..c0a7602 100644
--- a/board/samsung/smdk5250/smdk5250-uboot-spl.lds
+++ b/board/samsung/smdk5250/smdk5250-uboot-spl.lds
@@ -26,7 +26,7 @@
  */
 
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, \
-		LENGTH = CONFIG_SPL_MAX_SIZE }
+		LENGTH = CONFIG_SPL_MAX_FOOTPRINT }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
index 496a194..3aed696 100644
--- a/include/configs/exynos5250-dt.h
+++ b/include/configs/exynos5250-dt.h
@@ -141,7 +141,7 @@
 /* specific .lds file */
 #define CONFIG_SPL_LDSCRIPT	"board/samsung/smdk5250/smdk5250-uboot-spl.lds"
 #define CONFIG_SPL_TEXT_BASE	0x02023400
-#define CONFIG_SPL_MAX_SIZE	(14 * 1024)
+#define CONFIG_SPL_MAX_FOOTPRINT	(14 * 1024)
 
 #define CONFIG_BOOTCOMMAND	"mmc read 40007000 451 2000; bootm 40007000"
 
-- 
1.7.10.4

[U-Boot] [PATCH v4 1/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Tom Rini]

On Fri, Apr 12, 2013 at 05:14:30PM +0200, Albert ARIBAUD wrote:

> +		CONFIG_SPL_MAX_FOOTPRINT
> +		Maximum size in memory allocated to the SPL, BSS included.
> +		When defined, the linker checks that the actual memory
> +		used by SPL from _start to __bss_end does not exceed it.
> +		CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not

must not, not should not.  And CONFIG_SPL_... for both.  You can just
fix that up when commiting, assuming no other comments require change :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130412/fd5fde0b/attachment.pgp>

[U-Boot] [PATCH v4 1/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Albert ARIBAUD]

Hi Tom,

On Fri, 12 Apr 2013 11:30:32 -0400, Tom Rini <trini@ti.com> wrote:

> On Fri, Apr 12, 2013 at 05:14:30PM +0200, Albert ARIBAUD wrote:
> 
> > +		CONFIG_SPL_MAX_FOOTPRINT
> > +		Maximum size in memory allocated to the SPL, BSS included.
> > +		When defined, the linker checks that the actual memory
> > +		used by SPL from _start to __bss_end does not exceed it.
> > +		CONFIG_MAX_FOOTPRINT and CONFIG_MAX_BSS_SIZE should not
> 
> must not, not should not.  And CONFIG_SPL_... for both.  You can just
> fix that up when commiting, assuming no other comments require change :)

Will do; I'll just wait for 24 hours, to make sure no other change is
requested.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v4 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

On Fri, 12 Apr 2013 17:14:29 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
> semantics across all of U-boot. This patch series aims at fixing this by
> splitting the maximum size into separate image (code + data + rodata +
> linker list) size on the one hand, and BSS size on the other hand.
> 
> Changes in v4:
> - reordered patches so that FOOTPRINT is introduced first,
>   then individual boards are switched to using it
> - rewrote README entries
> - converted to CONFIG_SPL_MAX_FOOTPRINT
> - limited SPL size to exactly 6 2K pages
> - converted to CONFIG_SPL_MAX_FOOTPRINT
> - converted to CONFIG_SPL_MAX_FOOTPRINT
> 
> Changes in v3:
> - introduced CONFIG_SPL_MAX_FOOTPRINT
> - fixed typo in BSS size test
> 
> Changes in v2:
> - removed mmutable in SPL linker file
> - brought back total size to 12K
> - fixed commit summary typoes
> - fixed spacing in commit summary
> 
> Albert ARIBAUD (4):
>   ARM: fix CONFIG_SPL_MAX_SIZE semantics
>   cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT
>   da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT
>   smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT
> 
>  README                                         |   18 ++++++++++++++++--
>  arch/arm/cpu/u-boot-spl.lds                    |   24 +++++++++++++-----------
>  arch/arm/cpu/u-boot.lds                        |    4 ----
>  board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
>  board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
>  board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
>  include/configs/cam_enc_4xx.h                  |    2 +-
>  include/configs/da850evm.h                     |    2 +-
>  include/configs/exynos5250-dt.h                |    2 +-
>  include/configs/tegra-common.h                 |    2 +-
>  10 files changed, 36 insertions(+), 24 deletions(-)
> 

Applied to u-boot-arm/master, with README corrected on the fly as
suggested by Tom.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v4 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Benoît Thébaudeau]

Hi Albert,

On Sunday, April 14, 2013 4:10:37 PM, Albert ARIBAUD wrote:
> On Fri, 12 Apr 2013 17:14:29 +0200, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> 
> > CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE did not have constant
> > semantics across all of U-boot. This patch series aims at fixing this by
> > splitting the maximum size into separate image (code + data + rodata +
> > linker list) size on the one hand, and BSS size on the other hand.
> > 
> > Changes in v4:
> > - reordered patches so that FOOTPRINT is introduced first,
> >   then individual boards are switched to using it
> > - rewrote README entries
> > - converted to CONFIG_SPL_MAX_FOOTPRINT
> > - limited SPL size to exactly 6 2K pages
> > - converted to CONFIG_SPL_MAX_FOOTPRINT
> > - converted to CONFIG_SPL_MAX_FOOTPRINT
> > 
> > Changes in v3:
> > - introduced CONFIG_SPL_MAX_FOOTPRINT
> > - fixed typo in BSS size test
> > 
> > Changes in v2:
> > - removed mmutable in SPL linker file
> > - brought back total size to 12K
> > - fixed commit summary typoes
> > - fixed spacing in commit summary
> > 
> > Albert ARIBAUD (4):
> >   ARM: fix CONFIG_SPL_MAX_SIZE semantics
> >   cam_enc_4xx: convert to CONFIG_SPL_MAX_FOOTPRINT
> >   da850evm, da850_am18xxevm: convert to CONFIG_SPL_MAX_FOOTPRINT
> >   smdk5250, snow: convert to CONFIG_SPL_MAX_FOOTPRINT
> > 
> >  README                                         |   18 ++++++++++++++++--
> >  arch/arm/cpu/u-boot-spl.lds                    |   24
> >  +++++++++++++-----------
> >  arch/arm/cpu/u-boot.lds                        |    4 ----
> >  board/ait/cam_enc_4xx/u-boot-spl.lds           |    2 +-
> >  board/davinci/da8xxevm/u-boot-spl-da850evm.lds |    2 +-
> >  board/samsung/smdk5250/smdk5250-uboot-spl.lds  |    2 +-
> >  include/configs/cam_enc_4xx.h                  |    2 +-
> >  include/configs/da850evm.h                     |    2 +-
> >  include/configs/exynos5250-dt.h                |    2 +-
> >  include/configs/tegra-common.h                 |    2 +-
> >  10 files changed, 36 insertions(+), 24 deletions(-)
> > 
> 
> Applied to u-boot-arm/master, with README corrected on the fly as
> suggested by Tom.

This on-the-fly correction is incomplete:
CONFIG_SPL_MAX_BSS_SIZE -> CONFIG_SPL_BSS_MAX_SIZE

Best regards,
Beno?t

[U-Boot] [PATCH v4 0/4] Homogeneize semantics of CONFIG_SPL_MAX_SIZE

[Albert ARIBAUD]

Hi Beno?t,

> > Applied to u-boot-arm/master, with README corrected on the fly as
> > suggested by Tom.
> 
> This on-the-fly correction is incomplete:
> CONFIG_SPL_MAX_BSS_SIZE -> CONFIG_SPL_BSS_MAX_SIZE
> 
> Best regards,
> Beno?t

Argh! Where were you when I was typing? :)

Well, since it is in the README, i.e. non-functional, there's no
point in rolling back; I'll make it a cosmetic patch.

Thanks for noticing it!

Amicalement,
-- 
Albert.