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