All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 5/10] common/env: Point default environment for GD
@ 2014-03-31 10:04 Prabhakar Kushwaha
  2014-04-01 22:04 ` Scott Wood
  0 siblings, 1 reply; 6+ messages in thread
From: Prabhakar Kushwaha @ 2014-03-31 10:04 UTC (permalink / raw)
  To: u-boot

GD(Global Data) structure has pointer to environment variable array.
but, it always point to default_environment assuming it is running from
final location.

So update GD pointer with env variable array during SPL boot.

Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
---
 common/env_common.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/env_common.c b/common/env_common.c
index c0bfc2f..043150a 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -162,6 +162,9 @@ int env_import(const char *buf, int check)
 	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
 			0, NULL)) {
 		gd->flags |= GD_FLG_ENV_READY;
+#ifdef CONFIG_SPL_BUILD
+		gd->env_addr = (unsigned long)ep->data;
+#endif
 		return 1;
 	}
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 5/10] common/env: Point default environment for GD
  2014-03-31 10:04 [U-Boot] [PATCH 5/10] common/env: Point default environment for GD Prabhakar Kushwaha
@ 2014-04-01 22:04 ` Scott Wood
  2014-04-02  3:42   ` Prabhakar Kushwaha
  0 siblings, 1 reply; 6+ messages in thread
From: Scott Wood @ 2014-04-01 22:04 UTC (permalink / raw)
  To: u-boot

On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
> GD(Global Data) structure has pointer to environment variable array.
> but, it always point to default_environment assuming it is running from
> final location.
> 
> So update GD pointer with env variable array during SPL boot.
> 
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> ---
>  common/env_common.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/env_common.c b/common/env_common.c
> index c0bfc2f..043150a 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -162,6 +162,9 @@ int env_import(const char *buf, int check)
>  	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
>  			0, NULL)) {
>  		gd->flags |= GD_FLG_ENV_READY;
> +#ifdef CONFIG_SPL_BUILD
> +		gd->env_addr = (unsigned long)ep->data;
> +#endif
>  		return 1;
>  	}
>  

Could you explain a bit more about how the environment is being loaded
during SPL, and how gd->env_addr gets set for non-SPL?

-Scott

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 5/10] common/env: Point default environment for GD
  2014-04-01 22:04 ` Scott Wood
@ 2014-04-02  3:42   ` Prabhakar Kushwaha
  2014-04-04 23:51     ` Scott Wood
  0 siblings, 1 reply; 6+ messages in thread
From: Prabhakar Kushwaha @ 2014-04-02  3:42 UTC (permalink / raw)
  To: u-boot


On 4/2/2014 3:34 AM, Scott Wood wrote:
> On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
>> GD(Global Data) structure has pointer to environment variable array.
>> but, it always point to default_environment assuming it is running from
>> final location.
>>
>> So update GD pointer with env variable array during SPL boot.
>>
>> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
>> ---
>>   common/env_common.c |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/common/env_common.c b/common/env_common.c
>> index c0bfc2f..043150a 100644
>> --- a/common/env_common.c
>> +++ b/common/env_common.c
>> @@ -162,6 +162,9 @@ int env_import(const char *buf, int check)
>>   	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
>>   			0, NULL)) {
>>   		gd->flags |= GD_FLG_ENV_READY;
>> +#ifdef CONFIG_SPL_BUILD
>> +		gd->env_addr = (unsigned long)ep->data;
>> +#endif
>>   		return 1;
>>   	}
>>   
> Could you explain a bit more about how the environment is being loaded
> during SPL, and how gd->env_addr gets set for non-SPL?
>
>

during SPL or NON- XIP boot (SD, SPI, NAND) in order to access env 
variables following 2 functions are called sequentially.
1. env_init() : Initialize gd->env_addr with default_environment array
2. env_relocate(). It copied data from flash to heap memory area and 
call env_import().
             here,  This env pointer *buf is used for importing.  but 
gd->env_addr never updated with new address of env variables.  It always 
remain pointed to default_environment array set in env_init().
It looks to be a issue for non-XIP boot.  As I was not confident about 
it. So limited to only SPL framework only.

for non-SPL i.e. XIP boot (NOR)
  env_init (): gd->env_addr always point to NOR address containing 
env_variables.  Because of XIP No need of copy from flash to buffer.  
Hence no problem.

Regards,
Prabhakar

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 5/10] common/env: Point default environment for GD
  2014-04-02  3:42   ` Prabhakar Kushwaha
@ 2014-04-04 23:51     ` Scott Wood
  2014-04-07  6:40       ` Prabhakar Kushwaha
  0 siblings, 1 reply; 6+ messages in thread
From: Scott Wood @ 2014-04-04 23:51 UTC (permalink / raw)
  To: u-boot

On Wed, 2014-04-02 at 09:12 +0530, Prabhakar Kushwaha wrote:
> On 4/2/2014 3:34 AM, Scott Wood wrote:
> > On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
> >> GD(Global Data) structure has pointer to environment variable array.
> >> but, it always point to default_environment assuming it is running from
> >> final location.
> >>
> >> So update GD pointer with env variable array during SPL boot.
> >>
> >> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
> >> ---
> >>   common/env_common.c |    3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/common/env_common.c b/common/env_common.c
> >> index c0bfc2f..043150a 100644
> >> --- a/common/env_common.c
> >> +++ b/common/env_common.c
> >> @@ -162,6 +162,9 @@ int env_import(const char *buf, int check)
> >>   	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
> >>   			0, NULL)) {
> >>   		gd->flags |= GD_FLG_ENV_READY;
> >> +#ifdef CONFIG_SPL_BUILD
> >> +		gd->env_addr = (unsigned long)ep->data;
> >> +#endif
> >>   		return 1;
> >>   	}
> >>   
> > Could you explain a bit more about how the environment is being loaded
> > during SPL, and how gd->env_addr gets set for non-SPL?
> >
> >
> 
> during SPL or NON- XIP boot (SD, SPI, NAND) in order to access env 
> variables following 2 functions are called sequentially.
> 1. env_init() : Initialize gd->env_addr with default_environment array
> 2. env_relocate(). It copied data from flash to heap memory area and 
> call env_import().
>              here,  This env pointer *buf is used for importing.  but 
> gd->env_addr never updated with new address of env variables.  It always 
> remain pointed to default_environment array set in env_init().
> It looks to be a issue for non-XIP boot.  As I was not confident about 
> it. So limited to only SPL framework only.
> 
> for non-SPL i.e. XIP boot (NOR)
>   env_init (): gd->env_addr always point to NOR address containing 
> env_variables.  Because of XIP No need of copy from flash to buffer.  
> Hence no problem.

That doesn't really answer my question of how it gets initialized for
NOR flash.  What line in what file does the initialization?  It looks
like the answer is env_init() -- so it doesn't "always" hold the
address.  Why can't env_init() do this on NAND, SPI, etc, provided you
actually have the environment available before relocation[1]?

I'm also not sure what it has to do with SPL as opposed to non-XIP.
Doesn't this also apply for non-SPL PBL boot, etc?  Why wouldn't you do
this in env_relocate_spec()?

BTW, I notice a few Freescale boards initialize this in their spl.c
file.

Also, you don't mention what the actual bug is you see.  What uses
gd->env_addr after relocation, other than api/?  I'm not saying that
api/ isn't important, but I'm wondering if you saw an effect elsewhere.

-Scott

[1] I realize that in many cases we don't, but that's something we need
to fix, since it's important to how DDR gets set up among other things.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 5/10] common/env: Point default environment for GD
  2014-04-04 23:51     ` Scott Wood
@ 2014-04-07  6:40       ` Prabhakar Kushwaha
  2014-04-08 23:00         ` Scott Wood
  0 siblings, 1 reply; 6+ messages in thread
From: Prabhakar Kushwaha @ 2014-04-07  6:40 UTC (permalink / raw)
  To: u-boot


On 4/5/2014 5:21 AM, Scott Wood wrote:
> On Wed, 2014-04-02 at 09:12 +0530, Prabhakar Kushwaha wrote:
>> On 4/2/2014 3:34 AM, Scott Wood wrote:
>>> On Mon, 2014-03-31 at 15:34 +0530, Prabhakar Kushwaha wrote:
>>>> GD(Global Data) structure has pointer to environment variable array.
>>>> but, it always point to default_environment assuming it is running from
>>>> final location.
>>>>
>>>> So update GD pointer with env variable array during SPL boot.
>>>>
>>>> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
>>>> ---
>>>>    common/env_common.c |    3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/common/env_common.c b/common/env_common.c
>>>> index c0bfc2f..043150a 100644
>>>> --- a/common/env_common.c
>>>> +++ b/common/env_common.c
>>>> @@ -162,6 +162,9 @@ int env_import(const char *buf, int check)
>>>>    	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
>>>>    			0, NULL)) {
>>>>    		gd->flags |= GD_FLG_ENV_READY;
>>>> +#ifdef CONFIG_SPL_BUILD
>>>> +		gd->env_addr = (unsigned long)ep->data;
>>>> +#endif
>>>>    		return 1;
>>>>    	}
>>>>    
>>> Could you explain a bit more about how the environment is being loaded
>>> during SPL, and how gd->env_addr gets set for non-SPL?
>>>
>>>
>> during SPL or NON- XIP boot (SD, SPI, NAND) in order to access env
>> variables following 2 functions are called sequentially.
>> 1. env_init() : Initialize gd->env_addr with default_environment array
>> 2. env_relocate(). It copied data from flash to heap memory area and
>> call env_import().
>>               here,  This env pointer *buf is used for importing.  but
>> gd->env_addr never updated with new address of env variables.  It always
>> remain pointed to default_environment array set in env_init().
>> It looks to be a issue for non-XIP boot.  As I was not confident about
>> it. So limited to only SPL framework only.
>>
>> for non-SPL i.e. XIP boot (NOR)
>>    env_init (): gd->env_addr always point to NOR address containing
>> env_variables.  Because of XIP No need of copy from flash to buffer.
>> Hence no problem.
> That doesn't really answer my question of how it gets initialized for
> NOR flash.  What line in what file does the initialization?  It looks
> like the answer is env_init() -- so it doesn't "always" hold the
> address.  Why can't env_init() do this on NAND, SPI, etc, provided you
> actually have the environment available before relocation[1]?
>
> I'm also not sure what it has to do with SPL as opposed to non-XIP.
> Doesn't this also apply for non-SPL PBL boot, etc?  Why wouldn't you do
> this in env_relocate_spec()?
>
> BTW, I notice a few Freescale boards initialize this in their spl.c
> file.
>
> Also, you don't mention what the actual bug is you see.  What uses
> gd->env_addr after relocation, other than api/?  I'm not saying that
> api/ isn't important, but I'm wondering if you saw an effect elsewhere.
>
> -Scott
>
> [1] I realize that in many cases we don't, but that's something we need
> to fix, since it's important to how DDR gets set up among other things.
>

I analyzed further with P5020_NAND boot.  i found following observation:
1. env_init happens in board_init_f
2. DDR always initialized with default_environment
3. env_relocate happens in board_init_r

Looks like, during non-XIP & non - SPL boot, ddr can only be initialized 
with fixed settings.
is it a accepted behavior? if Yes, DDR init parameters cannot be changed 
like ECC on/off.

Regards,
Prabhakar

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 5/10] common/env: Point default environment for GD
  2014-04-07  6:40       ` Prabhakar Kushwaha
@ 2014-04-08 23:00         ` Scott Wood
  0 siblings, 0 replies; 6+ messages in thread
From: Scott Wood @ 2014-04-08 23:00 UTC (permalink / raw)
  To: u-boot

On Mon, 2014-04-07 at 12:10 +0530, Prabhakar Kushwaha wrote:
> On 4/5/2014 5:21 AM, Scott Wood wrote:
> > That doesn't really answer my question of how it gets initialized for
> > NOR flash.  What line in what file does the initialization?  It looks
> > like the answer is env_init() -- so it doesn't "always" hold the
> > address.  Why can't env_init() do this on NAND, SPI, etc, provided you
> > actually have the environment available before relocation[1]?
> >
> > I'm also not sure what it has to do with SPL as opposed to non-XIP.
> > Doesn't this also apply for non-SPL PBL boot, etc?  Why wouldn't you do
> > this in env_relocate_spec()?
> >
> > BTW, I notice a few Freescale boards initialize this in their spl.c
> > file.
> >
> > Also, you don't mention what the actual bug is you see.  What uses
> > gd->env_addr after relocation, other than api/?  I'm not saying that
> > api/ isn't important, but I'm wondering if you saw an effect elsewhere.
> >
> > -Scott
> >
> > [1] I realize that in many cases we don't, but that's something we need
> > to fix, since it's important to how DDR gets set up among other things.
> >
> 
> I analyzed further with P5020_NAND boot.  i found following observation:
> 1. env_init happens in board_init_f
> 2. DDR always initialized with default_environment
> 3. env_relocate happens in board_init_r
> 
> Looks like, during non-XIP & non - SPL boot, ddr can only be initialized 
> with fixed settings.
> is it a accepted behavior? if Yes, DDR init parameters cannot be changed 
> like ECC on/off.

It is the current behavior on many boards, but it needs to be fixed (as
I noted above).

-Scott

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-04-08 23:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-31 10:04 [U-Boot] [PATCH 5/10] common/env: Point default environment for GD Prabhakar Kushwaha
2014-04-01 22:04 ` Scott Wood
2014-04-02  3:42   ` Prabhakar Kushwaha
2014-04-04 23:51     ` Scott Wood
2014-04-07  6:40       ` Prabhakar Kushwaha
2014-04-08 23:00         ` Scott Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.