diff mbox

[U-Boot,4/4] ARM: tegra: increase CONFIG_SYS_TEXT_BASE

Message ID 1350424209-11186-4-git-send-email-swarren@wwwdotorg.org
State Superseded, archived
Delegated to: Tom Warren
Headers show

Commit Message

Stephen Warren Oct. 16, 2012, 9:50 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
overlap the main U-Boot.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 include/configs/tegra20-common.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Lucas Stach Oct. 16, 2012, 10:09 p.m. UTC | #1
Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
> From: Stephen Warren <swarren@nvidia.com>
> 
> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
> overlap the main U-Boot.
> 
Is there any specific reason why the SPL is now bigger than before? Or
is this just because of the general U-Boot rework (like serial multi
anywhere)? And by how much has it grown? This is really more out of
curiosity rather than any real objection.

Aside from this I think the general idea is reasonable, as we are not
shipping a particularly slim U-Boot on any Tegra platform, nor do we
have to hit a hard size limit, so for the series:

Acked-by: Lucas Stach <dev@lynxeye.de>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  include/configs/tegra20-common.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h
> index dc7444d..ced278d 100644
> --- a/include/configs/tegra20-common.h
> +++ b/include/configs/tegra20-common.h
> @@ -168,7 +168,7 @@
>  #define PHYS_SDRAM_1		NV_PA_SDRC_CS0
>  #define PHYS_SDRAM_1_SIZE	0x20000000	/* 512M */
>  
> -#define CONFIG_SYS_TEXT_BASE	0x0010c000
> +#define CONFIG_SYS_TEXT_BASE	0x0010d000
>  #define CONFIG_SYS_SDRAM_BASE	PHYS_SDRAM_1
>  
>  #define CONFIG_SYS_INIT_RAM_ADDR	CONFIG_STACKBASE
Stephen Warren Oct. 16, 2012, 10:43 p.m. UTC | #2
On 10/16/2012 04:09 PM, Lucas Stach wrote:
> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
>> overlap the main U-Boot.
>
> Is there any specific reason why the SPL is now bigger than before? Or
> is this just because of the general U-Boot rework (like serial multi
> anywhere)? And by how much has it grown? This is really more out of
> curiosity rather than any real objection.

Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
build flags" which was the last patch in the series that enabled SPL on
Tegra, and it overflows even there:

Configuring for ventana board...
   text	   data	    bss	    dec	    hex	filename
 226085	   4384	 274228	 504697	  7b379	./u-boot
  13857	    153	   4516	  18526	   485e	./spl/u-boot-spl

u-boot/master currently has:

Configuring for ventana board...
   text	   data	    bss	    dec	    hex	filename
 233579	   4432	 274368	 512379	  7d17b	./u-boot
  14382	    201	   4520	  19103	   4a9f	./spl/u-boot-spl

So, there is slight growth, but mainly I think we've just been getting
lucky.

Also, the overflow might possibly only have been exposed by the recent
serial rework; when I found the problem the serial rework caused on
Tegra, Allen mentioned that the missing BSS clearing hadn't been a
problem before since no global variables were used, but the serial
rework caused some to be.
Simon Glass Oct. 18, 2012, 12:05 a.m. UTC | #3
Hi Stephen.

On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/16/2012 04:09 PM, Lucas Stach wrote:
>> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
>>> overlap the main U-Boot.
>>
>> Is there any specific reason why the SPL is now bigger than before? Or
>> is this just because of the general U-Boot rework (like serial multi
>> anywhere)? And by how much has it grown? This is really more out of
>> curiosity rather than any real objection.
>
> Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
> build flags" which was the last patch in the series that enabled SPL on
> Tegra, and it overflows even there:
>
> Configuring for ventana board...
>    text    data     bss     dec     hex filename
>  226085    4384  274228  504697   7b379 ./u-boot
>   13857     153    4516   18526    485e ./spl/u-boot-spl
>
> u-boot/master currently has:
>
> Configuring for ventana board...
>    text    data     bss     dec     hex filename
>  233579    4432  274368  512379   7d17b ./u-boot
>   14382     201    4520   19103    4a9f ./spl/u-boot-spl
>
> So, there is slight growth, but mainly I think we've just been getting
> lucky.
>
> Also, the overflow might possibly only have been exposed by the recent
> serial rework; when I found the problem the serial rework caused on
> Tegra, Allen mentioned that the missing BSS clearing hadn't been a
> problem before since no global variables were used, but the serial
> rework caused some to be.

To ask the opposite question, is it worth increasing by a whole 16KB
so that the base address of U-Boot is a more aligned number?

Regards,
Simon
Stephen Warren Oct. 18, 2012, 3:20 a.m. UTC | #4
On 10/17/2012 06:05 PM, Simon Glass wrote:
> Hi Stephen.
> 
> On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/16/2012 04:09 PM, Lucas Stach wrote:
>>> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
>>>> overlap the main U-Boot.
>>>
>>> Is there any specific reason why the SPL is now bigger than before? Or
>>> is this just because of the general U-Boot rework (like serial multi
>>> anywhere)? And by how much has it grown? This is really more out of
>>> curiosity rather than any real objection.
>>
>> Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
>> build flags" which was the last patch in the series that enabled SPL on
>> Tegra, and it overflows even there:
>>
>> Configuring for ventana board...
>>    text    data     bss     dec     hex filename
>>  226085    4384  274228  504697   7b379 ./u-boot
>>   13857     153    4516   18526    485e ./spl/u-boot-spl
>>
>> u-boot/master currently has:
>>
>> Configuring for ventana board...
>>    text    data     bss     dec     hex filename
>>  233579    4432  274368  512379   7d17b ./u-boot
>>   14382     201    4520   19103    4a9f ./spl/u-boot-spl
>>
>> So, there is slight growth, but mainly I think we've just been getting
>> lucky.
>>
>> Also, the overflow might possibly only have been exposed by the recent
>> serial rework; when I found the problem the serial rework caused on
>> Tegra, Allen mentioned that the missing BSS clearing hadn't been a
>> problem before since no global variables were used, but the serial
>> rework caused some to be.
> 
> To ask the opposite question, is it worth increasing by a whole 16KB
> so that the base address of U-Boot is a more aligned number?

That would bloat the binary by about 12KB more than it needs to be. I
don't believe there's any particular need for the main U-Boot to be
built for any particular address, and we can just continue to bump up
this value as/when the SPL grows.
Tom Rini Oct. 18, 2012, 4:31 p.m. UTC | #5
On Wed, Oct 17, 2012 at 09:20:31PM -0600, Stephen Warren wrote:
> On 10/17/2012 06:05 PM, Simon Glass wrote:
> > Hi Stephen.
> > 
> > On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> On 10/16/2012 04:09 PM, Lucas Stach wrote:
> >>> Am Dienstag, den 16.10.2012, 15:50 -0600 schrieb Stephen Warren:
> >>>> From: Stephen Warren <swarren@nvidia.com>
> >>>>
> >>>> The SPL has grown. Increase CONFIG_SYS_TEXT_BASE so SPL's BSS does not
> >>>> overlap the main U-Boot.
> >>>
> >>> Is there any specific reason why the SPL is now bigger than before? Or
> >>> is this just because of the general U-Boot rework (like serial multi
> >>> anywhere)? And by how much has it grown? This is really more out of
> >>> curiosity rather than any real objection.
> >>
> >> Looking at this more, I built commit cca6076 "tegra20: Remove armv4t
> >> build flags" which was the last patch in the series that enabled SPL on
> >> Tegra, and it overflows even there:
> >>
> >> Configuring for ventana board...
> >>    text    data     bss     dec     hex filename
> >>  226085    4384  274228  504697   7b379 ./u-boot
> >>   13857     153    4516   18526    485e ./spl/u-boot-spl
> >>
> >> u-boot/master currently has:
> >>
> >> Configuring for ventana board...
> >>    text    data     bss     dec     hex filename
> >>  233579    4432  274368  512379   7d17b ./u-boot
> >>   14382     201    4520   19103    4a9f ./spl/u-boot-spl
> >>
> >> So, there is slight growth, but mainly I think we've just been getting
> >> lucky.
> >>
> >> Also, the overflow might possibly only have been exposed by the recent
> >> serial rework; when I found the problem the serial rework caused on
> >> Tegra, Allen mentioned that the missing BSS clearing hadn't been a
> >> problem before since no global variables were used, but the serial
> >> rework caused some to be.
> > 
> > To ask the opposite question, is it worth increasing by a whole 16KB
> > so that the base address of U-Boot is a more aligned number?
> 
> That would bloat the binary by about 12KB more than it needs to be. I
> don't believe there's any particular need for the main U-Boot to be
> built for any particular address, and we can just continue to bump up
> this value as/when the SPL grows.

Well, lets stop and think for a minute more.  Are we likely to add new
features to SPL on Tegra (direct OS booting, support in one binary for
both SPL-from-flash and SPL-from-something-else) ?  If so, lets increase
this to a reasonable maximum now rather than keep bumping and breaking.
On TI parts for example, we are limited by our SRAM size and set that
(minus a bit for stack) as how big we allow SPL to be so that we don't
have to tweak this value based on toolchain or feature changes (ELDK 4.2
vs current Linaro for example, can be a noticable size difference).
Stephen Warren Oct. 18, 2012, 8:42 p.m. UTC | #6
On 10/18/2012 10:31 AM, Tom Rini wrote:
> On Wed, Oct 17, 2012 at 09:20:31PM -0600, Stephen Warren wrote:
>> On 10/17/2012 06:05 PM, Simon Glass wrote:
>>> On Tue, Oct 16, 2012 at 3:43 PM, Stephen Warren
>>> <swarren@wwwdotorg.org> wrote:
>>>> On 10/16/2012 04:09 PM, Lucas Stach wrote:
...
>>> To ask the opposite question, is it worth increasing by a whole
>>> 16KB so that the base address of U-Boot is a more aligned
>>> number?
>> 
>> That would bloat the binary by about 12KB more than it needs to
>> be. I don't believe there's any particular need for the main
>> U-Boot to be built for any particular address, and we can just
>> continue to bump up this value as/when the SPL grows.
> 
> Well, lets stop and think for a minute more.  Are we likely to add
> new features to SPL on Tegra (direct OS booting, support in one
> binary for both SPL-from-flash and SPL-from-something-else) ?

I don't think so. The only purpose of the SPL on Tegra is to run from
SDRAM on the AVP CPU, set up clocks for the main Cortex-A9 core, and
to cause the A9 to start executing the concatenated main U-Boot image.
The SPL always runs from SDRAM.

(As background, the boot ROM sets up SDRAM on Tegra, and copies the
concatenated SPL+U-Boot binaries into SDRAM from whatever boot device,
so the typical reasons for using SPL don't exist on Tegra).

Allen Martin was thinking about getting the SPL to run from IRAM
rather than SDRAM, and I think only execute on the AVP CPU (e.g. for
use as a slimmed-down flashing tool downloaded via the boot ROM's USB
recovery mechanism). However, I think that would end up being an
entirely separate SPL build (since we'd need both, not cut over), so
the size requirements would not impact in any way the SPL-in-SDRAM
that we have today.
diff mbox

Patch

diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h
index dc7444d..ced278d 100644
--- a/include/configs/tegra20-common.h
+++ b/include/configs/tegra20-common.h
@@ -168,7 +168,7 @@ 
 #define PHYS_SDRAM_1		NV_PA_SDRC_CS0
 #define PHYS_SDRAM_1_SIZE	0x20000000	/* 512M */
 
-#define CONFIG_SYS_TEXT_BASE	0x0010c000
+#define CONFIG_SYS_TEXT_BASE	0x0010d000
 #define CONFIG_SYS_SDRAM_BASE	PHYS_SDRAM_1
 
 #define CONFIG_SYS_INIT_RAM_ADDR	CONFIG_STACKBASE