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

Heiko Schocher
2013-04-09T12:11:38Z

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