Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()

2019-03-10 Thread Simon Glass
Hi Simon,

On Wed, 20 Feb 2019 at 22:44, Simon Goldschmidt
 wrote:
>
>
>
> Am Do., 21. Feb. 2019, 03:48 hat Simon Glass  geschrieben:
>>
>> Hi Simon,
>>
>> On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt
>>  wrote:
>> >
>> > Am 09.11.2018 um 19:43 schrieb Tom Rini:
>> > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
>> > >
>> > >> There is a strange feature to set global_data to a data-section variable
>> > >> early in SPL. This only works if SPL actually has access to SRAM which 
>> > >> is
>> > >> not the case on x86, for eaxmple. Add a comment to this effect.
>> >
>> > Seems like I missed that one back in October.
>> >
>> > Does anyone have an idea why this variable ('static bd_t bdata') is
>> > hard-coded into section ".data"? To me this seems pretty unportable...
>> >
>> > For example, for a specific feature on socfpga (warm reboot CRC check),
>> > I would prefer to have the ".data" section empty and put 'bdata' into
>> > bss. This is currently not possible.
>> >
>> > But before sending a patch that somehow changes this behaviour, I'd like
>> > to know why this variable is put into ".data" instead of ".bss"
>>
>> BSS normally appears after data and is not actually part of the
>> u-boot-spi.bin image. If the device tree is appended to SPL then it
>> overlaps with BSS. Thus accessing BSS overwrites the DT. There are two
>> features to get around that - one is to pad out the BSS space so that:
>>
>> cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin
>>
>> places the DT after BSS.
>>
>> The other is to specify an address (typically in SDRAM) for BSS, so
>> that it is completely separate from the image.
>>
>> Note also that BSS is not always available in SPL. For example if SPL
>> is running from flash then the BSS may be mapped either to SDRAM
>> (which is not set up until later in SPL) or flash (which is not
>> writable at all). Of course the data section has the same problem.
>
>
> Ok, thanks for the insight.
>
>> Putting gd in the data section avoids one of the above problems.
>
>
> Actually it's bd here, not gd, so it's a little less widely used.
>
> And while I can follow your explanation, it convinces me even more that this 
> should be configurable, not hadcoded. It wastes space in SPL binary and may 
> not even work on some platforms...

Yes my comments were on gd, sorry about that.

Ideally it would default to something safe (i.e. not being in BSS or
data). The best things is probably to malloc() it, assuming that this
is enabled in SPL.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()

2019-02-20 Thread Simon Goldschmidt
Am Do., 21. Feb. 2019, 03:48 hat Simon Glass  geschrieben:

> Hi Simon,
>
> On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt
>  wrote:
> >
> > Am 09.11.2018 um 19:43 schrieb Tom Rini:
> > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
> > >
> > >> There is a strange feature to set global_data to a data-section
> variable
> > >> early in SPL. This only works if SPL actually has access to SRAM
> which is
> > >> not the case on x86, for eaxmple. Add a comment to this effect.
> >
> > Seems like I missed that one back in October.
> >
> > Does anyone have an idea why this variable ('static bd_t bdata') is
> > hard-coded into section ".data"? To me this seems pretty unportable...
> >
> > For example, for a specific feature on socfpga (warm reboot CRC check),
> > I would prefer to have the ".data" section empty and put 'bdata' into
> > bss. This is currently not possible.
> >
> > But before sending a patch that somehow changes this behaviour, I'd like
> > to know why this variable is put into ".data" instead of ".bss"
>
> BSS normally appears after data and is not actually part of the
> u-boot-spi.bin image. If the device tree is appended to SPL then it
> overlaps with BSS. Thus accessing BSS overwrites the DT. There are two
> features to get around that - one is to pad out the BSS space so that:
>
> cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin
>
> places the DT after BSS.
>
> The other is to specify an address (typically in SDRAM) for BSS, so
> that it is completely separate from the image.
>
> Note also that BSS is not always available in SPL. For example if SPL
> is running from flash then the BSS may be mapped either to SDRAM
> (which is not set up until later in SPL) or flash (which is not
> writable at all). Of course the data section has the same problem.
>

Ok, thanks for the insight.

Putting gd in the data section avoids one of the above problems.
>

Actually it's bd here, not gd, so it's a little less widely used.

And while I can follow your explanation, it convinces me even more that
this should be configurable, not hadcoded. It wastes space in SPL binary
and may not even work on some platforms...

Regards,
Simon


> Others may have more insight here.
>
> Regards,
> Simon
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()

2019-02-20 Thread Simon Glass
Hi Simon,

On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt
 wrote:
>
> Am 09.11.2018 um 19:43 schrieb Tom Rini:
> > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:
> >
> >> There is a strange feature to set global_data to a data-section variable
> >> early in SPL. This only works if SPL actually has access to SRAM which is
> >> not the case on x86, for eaxmple. Add a comment to this effect.
>
> Seems like I missed that one back in October.
>
> Does anyone have an idea why this variable ('static bd_t bdata') is
> hard-coded into section ".data"? To me this seems pretty unportable...
>
> For example, for a specific feature on socfpga (warm reboot CRC check),
> I would prefer to have the ".data" section empty and put 'bdata' into
> bss. This is currently not possible.
>
> But before sending a patch that somehow changes this behaviour, I'd like
> to know why this variable is put into ".data" instead of ".bss"

BSS normally appears after data and is not actually part of the
u-boot-spi.bin image. If the device tree is appended to SPL then it
overlaps with BSS. Thus accessing BSS overwrites the DT. There are two
features to get around that - one is to pad out the BSS space so that:

cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin

places the DT after BSS.

The other is to specify an address (typically in SDRAM) for BSS, so
that it is completely separate from the image.

Note also that BSS is not always available in SPL. For example if SPL
is running from flash then the BSS may be mapped either to SDRAM
(which is not set up until later in SPL) or flash (which is not
writable at all). Of course the data section has the same problem.

Putting gd in the data section avoids one of the above problems.

Others may have more insight here.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()

2019-02-11 Thread Simon Goldschmidt

Am 09.11.2018 um 19:43 schrieb Tom Rini:

On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:


There is a strange feature to set global_data to a data-section variable
early in SPL. This only works if SPL actually has access to SRAM which is
not the case on x86, for eaxmple. Add a comment to this effect.


Seems like I missed that one back in October.

Does anyone have an idea why this variable ('static bd_t bdata') is 
hard-coded into section ".data"? To me this seems pretty unportable...


For example, for a specific feature on socfpga (warm reboot CRC check), 
I would prefer to have the ".data" section empty and put 'bdata' into 
bss. This is currently not possible.


But before sending a patch that somehow changes this behaviour, I'd like 
to know why this variable is put into ".data" instead of ".bss"


Thanks,
Simon



Signed-off-by: Simon Glass 


Reviewed-by: Tom Rini 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()

2018-11-09 Thread Tom Rini
On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote:

> There is a strange feature to set global_data to a data-section variable
> early in SPL. This only works if SPL actually has access to SRAM which is
> not the case on x86, for eaxmple. Add a comment to this effect.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot