diff mbox

[U-Boot,RFC,03/10] board_r: only assign gd when requested

Message ID 1401568344-4441-4-git-send-email-jeroen@myspectrum.nl
State RFC
Delegated to: Tom Rini
Headers show

Commit Message

Jeroen Hofstee May 31, 2014, 8:32 p.m. UTC 
When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles
the assignment of gd. At least in case of ARM/Aarch64 this means
board_init_r is alteady called with the new gd. Therefore only
assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
---
 common/board_r.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Glass June 3, 2014, 2:05 a.m. UTC | #1 
On 31 May 2014 14:32, Jeroen Hofstee <jeroen@myspectrum.nl> wrote:
> When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles
> the assignment of gd. At least in case of ARM/Aarch64 this means
> board_init_r is alteady called with the new gd. Therefore only
> assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.

I think you might be missing a signoff (you might consider using
patman). Apart from that it looks good.

Acked-by: Simon Gass <sjg@chromium.org>

> ---
>  common/board_r.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index 602a239..18bbe26 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
>         int i;
>  #endif
>
> -#ifndef CONFIG_X86
> +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
>         gd = new_gd;
>  #endif
>
> --
> 1.8.3.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Jeroen Hofstee June 3, 2014, 8:52 p.m. UTC | #2 
Hello Wolfgang / Stefan.

On za, 2014-05-31 at 22:32 +0200, Jeroen Hofstee wrote:
> When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles
> the assignment of gd. At least in case of ARM/Aarch64 this means
> board_init_r is alteady called with the new gd. Therefore only
> assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
> ---
>  common/board_r.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/board_r.c b/common/board_r.c
> index 602a239..18bbe26 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
>  	int i;
>  #endif
>  
> -#ifndef CONFIG_X86
> +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
>  	gd = new_gd;
>  #endif
>  

Can any of you confirm this change is fine for powerpc as well?

Regards,
Jeroen
Stefan Roese June 4, 2014, 3:52 a.m. UTC | #3 
Hi Jeroen,

(added York to cc as he introduced CONFIG_SYS_GENERIC_GLOBAL_DATA with 
patch 2a1680e3 [common/board_f: Initialized global data for generic board])

On 03.06.2014 22:52, Jeroen Hofstee wrote:
> Hello Wolfgang / Stefan.
>
> On za, 2014-05-31 at 22:32 +0200, Jeroen Hofstee wrote:
>> When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles
>> the assignment of gd. At least in case of ARM/Aarch64 this means
>> board_init_r is alteady called with the new gd. Therefore only
>> assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
>> ---
>>   common/board_r.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/board_r.c b/common/board_r.c
>> index 602a239..18bbe26 100644
>> --- a/common/board_r.c
>> +++ b/common/board_r.c
>> @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
>>   	int i;
>>   #endif
>>
>> -#ifndef CONFIG_X86
>> +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
>>   	gd = new_gd;
>>   #endif
>>
>
> Can any of you confirm this change is fine for powerpc as well?

powerpc doesn't define CONFIG_SYS_GENERIC_GLOBAL_DATA right now. And I'm 
not sure which powerpc boards use the common board_f/_r functions right 
now. Perhaps York knows?

Other than that I don't any problems, so:

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan
York Sun June 4, 2014, 4:19 a.m. UTC | #4 
On Jun 3, 2014, at 8:52 PM, Stefan Roese wrote:

> Hi Jeroen,
> 
> (added York to cc as he introduced CONFIG_SYS_GENERIC_GLOBAL_DATA with patch 2a1680e3 [common/board_f: Initialized global data for generic board])
> 
> On 03.06.2014 22:52, Jeroen Hofstee wrote:
>> Hello Wolfgang / Stefan.
>> 
>> On za, 2014-05-31 at 22:32 +0200, Jeroen Hofstee wrote:
>>> When CONFIG_SYS_GENERIC_GLOBAL_DATA is not set the arch handles
>>> the assignment of gd. At least in case of ARM/Aarch64 this means
>>> board_init_r is alteady called with the new gd. Therefore only
>>> assign gd if CONFIG_SYS_GENERIC_GLOBAL_DATA is defined.
>>> ---
>>>  common/board_r.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/common/board_r.c b/common/board_r.c
>>> index 602a239..18bbe26 100644
>>> --- a/common/board_r.c
>>> +++ b/common/board_r.c
>>> @@ -927,7 +927,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
>>>  	int i;
>>>  #endif
>>> 
>>> -#ifndef CONFIG_X86
>>> +#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
>>>  	gd = new_gd;
>>>  #endif
>>> 
>> 
>> Can any of you confirm this change is fine for powerpc as well?
> 
> powerpc doesn't define CONFIG_SYS_GENERIC_GLOBAL_DATA right now. And I'm not sure which powerpc boards use the common board_f/_r functions right now. Perhaps York knows?

I think this change is not right for powerpc. The idea of using CONFIG_SYS_GENERIC_GLOBAL_DATA is to use the stack for gd in board_init_f. PowerPC boards don't use this way. The gd is initialized and used before calling board_init_f.

PowerPC boards also use new_gd when calling board_init_r. If you add this macro, I don't think powerpc will continue to work.

This commit 15672c6dbd7e5a110773480ccfe47b98ba1dc6f8 converts some powerpc boards to use generic board.


York
diff mbox

Patch

diff --git a/common/board_r.c b/common/board_r.c
index 602a239..18bbe26 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -927,7 +927,7 @@  void board_init_r(gd_t *new_gd, ulong dest_addr)
 	int i;
 #endif
 
-#ifndef CONFIG_X86
+#ifdef CONFIG_SYS_GENERIC_GLOBAL_DATA
 	gd = new_gd;
 #endif