| Message ID | 1398893511-22085-1-git-send-email-yorksun@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: > For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for > initializing LAWs, before calling function baord_inti_f(). This data > should not be cleared later. > > Signed-off-by: York Sun <yorksun@freescale.com> > --- > Change log > v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > > Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. > > common/board_f.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/common/board_f.c b/common/board_f.c > index cbdf06f..eebb377 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { > > void board_init_f(ulong boot_flags) > { > -#ifndef CONFIG_X86 > + /* > + * For MPC85xx, global data is initialized in cpu_init_early_f() and > + * used for init_law(). gd should not be cleared in this function. > + */ > +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) > gd_t data; > > gd = &data; It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) rather than growing a list here. Is there any reason why the set of targets for which zero_global_data() is skipped is different from the set of targets where the gd instantiation and assignment is skipped? -Scott
On 04/30/2014 03:45 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: >> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for >> initializing LAWs, before calling function baord_inti_f(). This data >> should not be cleared later. >> >> Signed-off-by: York Sun <yorksun@freescale.com> >> --- >> Change log >> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >> >> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. >> >> common/board_f.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/common/board_f.c b/common/board_f.c >> index cbdf06f..eebb377 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { >> >> void board_init_f(ulong boot_flags) >> { >> -#ifndef CONFIG_X86 >> + /* >> + * For MPC85xx, global data is initialized in cpu_init_early_f() and >> + * used for init_law(). gd should not be cleared in this function. >> + */ >> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) >> gd_t data; >> >> gd = &data; > > It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) > rather than growing a list here. That's do-able. > > Is there any reason why the set of targets for which zero_global_data() > is skipped is different from the set of targets where the gd > instantiation and assignment is skipped? I would think the list should be identical. But without proper testing, I am reluctant to copy the list. As you have suggested, start from 85xx first. Non-mpc85xx can be dealt with when they get converted. York
On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: > On 04/30/2014 03:45 PM, Scott Wood wrote: > > On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: > >> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for > >> initializing LAWs, before calling function baord_inti_f(). This data > >> should not be cleared later. > >> > >> Signed-off-by: York Sun <yorksun@freescale.com> > >> --- > >> Change log > >> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > >> > >> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. > >> > >> common/board_f.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/common/board_f.c b/common/board_f.c > >> index cbdf06f..eebb377 100644 > >> --- a/common/board_f.c > >> +++ b/common/board_f.c > >> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { > >> > >> void board_init_f(ulong boot_flags) > >> { > >> -#ifndef CONFIG_X86 > >> + /* > >> + * For MPC85xx, global data is initialized in cpu_init_early_f() and > >> + * used for init_law(). gd should not be cleared in this function. > >> + */ > >> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) > >> gd_t data; > >> > >> gd = &data; > > > > It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) > > rather than growing a list here. > > That's do-able. > > > > > Is there any reason why the set of targets for which zero_global_data() > > is skipped is different from the set of targets where the gd > > instantiation and assignment is skipped? > > I would think the list should be identical. But without proper testing, I am > reluctant to copy the list. As you have suggested, start from 85xx first. > Non-mpc85xx can be dealt with when they get converted. None of those other PPC targets currently use the generic board. They will be tested when they are converted. -Scott
On 04/30/2014 03:51 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: >> On 04/30/2014 03:45 PM, Scott Wood wrote: >>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: >>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for >>>> initializing LAWs, before calling function baord_inti_f(). This data >>>> should not be cleared later. >>>> >>>> Signed-off-by: York Sun <yorksun@freescale.com> >>>> --- >>>> Change log >>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >>>> >>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. >>>> >>>> common/board_f.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/common/board_f.c b/common/board_f.c >>>> index cbdf06f..eebb377 100644 >>>> --- a/common/board_f.c >>>> +++ b/common/board_f.c >>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { >>>> >>>> void board_init_f(ulong boot_flags) >>>> { >>>> -#ifndef CONFIG_X86 >>>> + /* >>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and >>>> + * used for init_law(). gd should not be cleared in this function. >>>> + */ >>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) >>>> gd_t data; >>>> >>>> gd = &data; >>> >>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) >>> rather than growing a list here. >> >> That's do-able. >> >>> >>> Is there any reason why the set of targets for which zero_global_data() >>> is skipped is different from the set of targets where the gd >>> instantiation and assignment is skipped? >> >> I would think the list should be identical. But without proper testing, I am >> reluctant to copy the list. As you have suggested, start from 85xx first. >> Non-mpc85xx can be dealt with when they get converted. > > None of those other PPC targets currently use the generic board. They > will be tested when they are converted. > Are you suggesting to copy the list, instead of only putting those tested? It may save other maintainer some effort of debugging. But I can't be sure they will all work. York
On Wed, 2014-04-30 at 15:56 -0700, York Sun wrote: > On 04/30/2014 03:51 PM, Scott Wood wrote: > > On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: > >> On 04/30/2014 03:45 PM, Scott Wood wrote: > >>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: > >>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for > >>>> initializing LAWs, before calling function baord_inti_f(). This data > >>>> should not be cleared later. > >>>> > >>>> Signed-off-by: York Sun <yorksun@freescale.com> > >>>> --- > >>>> Change log > >>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > >>>> > >>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. > >>>> > >>>> common/board_f.c | 6 +++++- > >>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/common/board_f.c b/common/board_f.c > >>>> index cbdf06f..eebb377 100644 > >>>> --- a/common/board_f.c > >>>> +++ b/common/board_f.c > >>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { > >>>> > >>>> void board_init_f(ulong boot_flags) > >>>> { > >>>> -#ifndef CONFIG_X86 > >>>> + /* > >>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and > >>>> + * used for init_law(). gd should not be cleared in this function. > >>>> + */ > >>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) > >>>> gd_t data; > >>>> > >>>> gd = &data; > >>> > >>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) > >>> rather than growing a list here. > >> > >> That's do-able. > >> > >>> > >>> Is there any reason why the set of targets for which zero_global_data() > >>> is skipped is different from the set of targets where the gd > >>> instantiation and assignment is skipped? > >> > >> I would think the list should be identical. But without proper testing, I am > >> reluctant to copy the list. As you have suggested, start from 85xx first. > >> Non-mpc85xx can be dealt with when they get converted. > > > > None of those other PPC targets currently use the generic board. They > > will be tested when they are converted. > > > > Are you suggesting to copy the list, instead of only putting those tested? I'm saying to use CONFIG_SYS_EARLY_GD for both things. > It may save other maintainer some effort of debugging. But I can't be > sure they will all work. What good reason could there be for wanting to skip clearing of a gd that was just allocated on the stack? -Scott
On 04/30/2014 03:57 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 15:56 -0700, York Sun wrote: >> On 04/30/2014 03:51 PM, Scott Wood wrote: >>> On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: >>>> On 04/30/2014 03:45 PM, Scott Wood wrote: >>>>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: >>>>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for >>>>>> initializing LAWs, before calling function baord_inti_f(). This data >>>>>> should not be cleared later. >>>>>> >>>>>> Signed-off-by: York Sun <yorksun@freescale.com> >>>>>> --- >>>>>> Change log >>>>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >>>>>> >>>>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. >>>>>> >>>>>> common/board_f.c | 6 +++++- >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/common/board_f.c b/common/board_f.c >>>>>> index cbdf06f..eebb377 100644 >>>>>> --- a/common/board_f.c >>>>>> +++ b/common/board_f.c >>>>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { >>>>>> >>>>>> void board_init_f(ulong boot_flags) >>>>>> { >>>>>> -#ifndef CONFIG_X86 >>>>>> + /* >>>>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and >>>>>> + * used for init_law(). gd should not be cleared in this function. >>>>>> + */ >>>>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) >>>>>> gd_t data; >>>>>> >>>>>> gd = &data; >>>>> >>>>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) >>>>> rather than growing a list here. >>>> >>>> That's do-able. >>>> >>>>> >>>>> Is there any reason why the set of targets for which zero_global_data() >>>>> is skipped is different from the set of targets where the gd >>>>> instantiation and assignment is skipped? >>>> >>>> I would think the list should be identical. But without proper testing, I am >>>> reluctant to copy the list. As you have suggested, start from 85xx first. >>>> Non-mpc85xx can be dealt with when they get converted. >>> >>> None of those other PPC targets currently use the generic board. They >>> will be tested when they are converted. >>> >> >> Are you suggesting to copy the list, instead of only putting those tested? > > I'm saying to use CONFIG_SYS_EARLY_GD for both things. > >> It may save other maintainer some effort of debugging. But I can't be >> sure they will all work. > > What good reason could there be for wanting to skip clearing of a gd > that was just allocated on the stack? > Relocating is OK. But clearing is not. At least the used LAWs variable is needed. There may be other variables as well. All data in gd is copied to new location. York
On Wed, 2014-04-30 at 16:40 -0700, York Sun wrote: > On 04/30/2014 03:57 PM, Scott Wood wrote: > > On Wed, 2014-04-30 at 15:56 -0700, York Sun wrote: > >> On 04/30/2014 03:51 PM, Scott Wood wrote: > >>> On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: > >>>> On 04/30/2014 03:45 PM, Scott Wood wrote: > >>>>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: > >>>>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for > >>>>>> initializing LAWs, before calling function baord_inti_f(). This data > >>>>>> should not be cleared later. > >>>>>> > >>>>>> Signed-off-by: York Sun <yorksun@freescale.com> > >>>>>> --- > >>>>>> Change log > >>>>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > >>>>>> > >>>>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. > >>>>>> > >>>>>> common/board_f.c | 6 +++++- > >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/common/board_f.c b/common/board_f.c > >>>>>> index cbdf06f..eebb377 100644 > >>>>>> --- a/common/board_f.c > >>>>>> +++ b/common/board_f.c > >>>>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { > >>>>>> > >>>>>> void board_init_f(ulong boot_flags) > >>>>>> { > >>>>>> -#ifndef CONFIG_X86 > >>>>>> + /* > >>>>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and > >>>>>> + * used for init_law(). gd should not be cleared in this function. > >>>>>> + */ > >>>>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) > >>>>>> gd_t data; > >>>>>> > >>>>>> gd = &data; > >>>>> > >>>>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) > >>>>> rather than growing a list here. > >>>> > >>>> That's do-able. > >>>> > >>>>> > >>>>> Is there any reason why the set of targets for which zero_global_data() > >>>>> is skipped is different from the set of targets where the gd > >>>>> instantiation and assignment is skipped? > >>>> > >>>> I would think the list should be identical. But without proper testing, I am > >>>> reluctant to copy the list. As you have suggested, start from 85xx first. > >>>> Non-mpc85xx can be dealt with when they get converted. > >>> > >>> None of those other PPC targets currently use the generic board. They > >>> will be tested when they are converted. > >>> > >> > >> Are you suggesting to copy the list, instead of only putting those tested? > > > > I'm saying to use CONFIG_SYS_EARLY_GD for both things. > > > >> It may save other maintainer some effort of debugging. But I can't be > >> sure they will all work. > > > > What good reason could there be for wanting to skip clearing of a gd > > that was just allocated on the stack? > > > > Relocating is OK. But clearing is not. At least the used LAWs variable is > needed. There may be other variables as well. All data in gd is copied to new > location. Where do you get relocating from (at this stage of boot -- of course it will get relocated when U-Boot gets relocated)? Either gd was initialized early, in which case we want to keep using it and not clear it, or it wasn't, in which case we want to allocate gd on the stack and clear it. BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder what is going on with gd on x86, and whether it makes sense to lump it in with CONFIG_SYS_EARLY_GD. -Scott
On 04/30/2014 04:44 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 16:40 -0700, York Sun wrote: >> On 04/30/2014 03:57 PM, Scott Wood wrote: >>> On Wed, 2014-04-30 at 15:56 -0700, York Sun wrote: >>>> On 04/30/2014 03:51 PM, Scott Wood wrote: >>>>> On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: >>>>>> On 04/30/2014 03:45 PM, Scott Wood wrote: >>>>>>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: >>>>>>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for >>>>>>>> initializing LAWs, before calling function baord_inti_f(). This data >>>>>>>> should not be cleared later. >>>>>>>> >>>>>>>> Signed-off-by: York Sun <yorksun@freescale.com> >>>>>>>> --- >>>>>>>> Change log >>>>>>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >>>>>>>> >>>>>>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. >>>>>>>> >>>>>>>> common/board_f.c | 6 +++++- >>>>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/common/board_f.c b/common/board_f.c >>>>>>>> index cbdf06f..eebb377 100644 >>>>>>>> --- a/common/board_f.c >>>>>>>> +++ b/common/board_f.c >>>>>>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { >>>>>>>> >>>>>>>> void board_init_f(ulong boot_flags) >>>>>>>> { >>>>>>>> -#ifndef CONFIG_X86 >>>>>>>> + /* >>>>>>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and >>>>>>>> + * used for init_law(). gd should not be cleared in this function. >>>>>>>> + */ >>>>>>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) >>>>>>>> gd_t data; >>>>>>>> >>>>>>>> gd = &data; >>>>>>> >>>>>>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) >>>>>>> rather than growing a list here. >>>>>> >>>>>> That's do-able. >>>>>> >>>>>>> >>>>>>> Is there any reason why the set of targets for which zero_global_data() >>>>>>> is skipped is different from the set of targets where the gd >>>>>>> instantiation and assignment is skipped? >>>>>> >>>>>> I would think the list should be identical. But without proper testing, I am >>>>>> reluctant to copy the list. As you have suggested, start from 85xx first. >>>>>> Non-mpc85xx can be dealt with when they get converted. >>>>> >>>>> None of those other PPC targets currently use the generic board. They >>>>> will be tested when they are converted. >>>>> >>>> >>>> Are you suggesting to copy the list, instead of only putting those tested? >>> >>> I'm saying to use CONFIG_SYS_EARLY_GD for both things. >>> >>>> It may save other maintainer some effort of debugging. But I can't be >>>> sure they will all work. >>> >>> What good reason could there be for wanting to skip clearing of a gd >>> that was just allocated on the stack? >>> >> >> Relocating is OK. But clearing is not. At least the used LAWs variable is >> needed. There may be other variables as well. All data in gd is copied to new >> location. > > Where do you get relocating from (at this stage of boot -- of course it > will get relocated when U-Boot gets relocated)? Either gd was > initialized early, in which case we want to keep using it and not clear > it, or it wasn't, in which case we want to allocate gd on the stack and > clear it. Exactly. gd is used before board_init_f() for many cases. > > BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder > what is going on with gd on x86, and whether it makes sense to lump it > in with CONFIG_SYS_EARLY_GD. > Maybe x86 maintainers can chime in? If we define such macro, it should probably sit right above board_init_f() so it can be seen easily. There is no other place it is needed, yet. York
On Wed, 2014-04-30 at 16:48 -0700, York Sun wrote: > On 04/30/2014 04:44 PM, Scott Wood wrote: > > On Wed, 2014-04-30 at 16:40 -0700, York Sun wrote: > >> On 04/30/2014 03:57 PM, Scott Wood wrote: > >>> On Wed, 2014-04-30 at 15:56 -0700, York Sun wrote: > >>>> On 04/30/2014 03:51 PM, Scott Wood wrote: > >>>>> On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: > >>>>>> On 04/30/2014 03:45 PM, Scott Wood wrote: > >>>>>>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: > >>>>>>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for > >>>>>>>> initializing LAWs, before calling function baord_inti_f(). This data > >>>>>>>> should not be cleared later. > >>>>>>>> > >>>>>>>> Signed-off-by: York Sun <yorksun@freescale.com> > >>>>>>>> --- > >>>>>>>> Change log > >>>>>>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > >>>>>>>> > >>>>>>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. > >>>>>>>> > >>>>>>>> common/board_f.c | 6 +++++- > >>>>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>>>>>> > >>>>>>>> diff --git a/common/board_f.c b/common/board_f.c > >>>>>>>> index cbdf06f..eebb377 100644 > >>>>>>>> --- a/common/board_f.c > >>>>>>>> +++ b/common/board_f.c > >>>>>>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { > >>>>>>>> > >>>>>>>> void board_init_f(ulong boot_flags) > >>>>>>>> { > >>>>>>>> -#ifndef CONFIG_X86 > >>>>>>>> + /* > >>>>>>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and > >>>>>>>> + * used for init_law(). gd should not be cleared in this function. > >>>>>>>> + */ > >>>>>>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) > >>>>>>>> gd_t data; > >>>>>>>> > >>>>>>>> gd = &data; > >>>>>>> > >>>>>>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) > >>>>>>> rather than growing a list here. > >>>>>> > >>>>>> That's do-able. > >>>>>> > >>>>>>> > >>>>>>> Is there any reason why the set of targets for which zero_global_data() > >>>>>>> is skipped is different from the set of targets where the gd > >>>>>>> instantiation and assignment is skipped? > >>>>>> > >>>>>> I would think the list should be identical. But without proper testing, I am > >>>>>> reluctant to copy the list. As you have suggested, start from 85xx first. > >>>>>> Non-mpc85xx can be dealt with when they get converted. > >>>>> > >>>>> None of those other PPC targets currently use the generic board. They > >>>>> will be tested when they are converted. > >>>>> > >>>> > >>>> Are you suggesting to copy the list, instead of only putting those tested? > >>> > >>> I'm saying to use CONFIG_SYS_EARLY_GD for both things. > >>> > >>>> It may save other maintainer some effort of debugging. But I can't be > >>>> sure they will all work. > >>> > >>> What good reason could there be for wanting to skip clearing of a gd > >>> that was just allocated on the stack? > >>> > >> > >> Relocating is OK. But clearing is not. At least the used LAWs variable is > >> needed. There may be other variables as well. All data in gd is copied to new > >> location. > > > > Where do you get relocating from (at this stage of boot -- of course it > > will get relocated when U-Boot gets relocated)? Either gd was > > initialized early, in which case we want to keep using it and not clear > > it, or it wasn't, in which case we want to allocate gd on the stack and > > clear it. > > Exactly. gd is used before board_init_f() for many cases. Yes, that's the whole point of CONFIG_SYS_EARLY_GD. What I'm saying is to forget about the current ifdef list around zero_global_data(), and replace it with CONFIG_SYS_EARLY_GD, which for now would only contain mpc85xx, mpc86xx, and x86. Other targets can skip the zeroing if and when they also skip the stack allocation and assignment. > > BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder > > what is going on with gd on x86, and whether it makes sense to lump it > > in with CONFIG_SYS_EARLY_GD. > > > > Maybe x86 maintainers can chime in? If we define such macro, it should probably > sit right above board_init_f() so it can be seen easily. There is no other place > it is needed, yet. I was thinking it would be set the same way other CONFIG symbols are set. -Scott
On 04/30/2014 04:52 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 16:48 -0700, York Sun wrote: >> On 04/30/2014 04:44 PM, Scott Wood wrote: >>> On Wed, 2014-04-30 at 16:40 -0700, York Sun wrote: >>>> On 04/30/2014 03:57 PM, Scott Wood wrote: >>>>> On Wed, 2014-04-30 at 15:56 -0700, York Sun wrote: >>>>>> On 04/30/2014 03:51 PM, Scott Wood wrote: >>>>>>> On Wed, 2014-04-30 at 15:48 -0700, York Sun wrote: >>>>>>>> On 04/30/2014 03:45 PM, Scott Wood wrote: >>>>>>>>> On Wed, 2014-04-30 at 14:31 -0700, York Sun wrote: >>>>>>>>>> For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for >>>>>>>>>> initializing LAWs, before calling function baord_inti_f(). This data >>>>>>>>>> should not be cleared later. >>>>>>>>>> >>>>>>>>>> Signed-off-by: York Sun <yorksun@freescale.com> >>>>>>>>>> --- >>>>>>>>>> Change log >>>>>>>>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >>>>>>>>>> >>>>>>>>>> Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. >>>>>>>>>> >>>>>>>>>> common/board_f.c | 6 +++++- >>>>>>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/common/board_f.c b/common/board_f.c >>>>>>>>>> index cbdf06f..eebb377 100644 >>>>>>>>>> --- a/common/board_f.c >>>>>>>>>> +++ b/common/board_f.c >>>>>>>>>> @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { >>>>>>>>>> >>>>>>>>>> void board_init_f(ulong boot_flags) >>>>>>>>>> { >>>>>>>>>> -#ifndef CONFIG_X86 >>>>>>>>>> + /* >>>>>>>>>> + * For MPC85xx, global data is initialized in cpu_init_early_f() and >>>>>>>>>> + * used for init_law(). gd should not be cleared in this function. >>>>>>>>>> + */ >>>>>>>>>> +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) >>>>>>>>>> gd_t data; >>>>>>>>>> >>>>>>>>>> gd = &data; >>>>>>>>> >>>>>>>>> It would be better to introduce a CONFIG_SYS_EARLY_GD (or similar) >>>>>>>>> rather than growing a list here. >>>>>>>> >>>>>>>> That's do-able. >>>>>>>> >>>>>>>>> >>>>>>>>> Is there any reason why the set of targets for which zero_global_data() >>>>>>>>> is skipped is different from the set of targets where the gd >>>>>>>>> instantiation and assignment is skipped? >>>>>>>> >>>>>>>> I would think the list should be identical. But without proper testing, I am >>>>>>>> reluctant to copy the list. As you have suggested, start from 85xx first. >>>>>>>> Non-mpc85xx can be dealt with when they get converted. >>>>>>> >>>>>>> None of those other PPC targets currently use the generic board. They >>>>>>> will be tested when they are converted. >>>>>>> >>>>>> >>>>>> Are you suggesting to copy the list, instead of only putting those tested? >>>>> >>>>> I'm saying to use CONFIG_SYS_EARLY_GD for both things. >>>>> >>>>>> It may save other maintainer some effort of debugging. But I can't be >>>>>> sure they will all work. >>>>> >>>>> What good reason could there be for wanting to skip clearing of a gd >>>>> that was just allocated on the stack? >>>>> >>>> >>>> Relocating is OK. But clearing is not. At least the used LAWs variable is >>>> needed. There may be other variables as well. All data in gd is copied to new >>>> location. >>> >>> Where do you get relocating from (at this stage of boot -- of course it >>> will get relocated when U-Boot gets relocated)? Either gd was >>> initialized early, in which case we want to keep using it and not clear >>> it, or it wasn't, in which case we want to allocate gd on the stack and >>> clear it. >> >> Exactly. gd is used before board_init_f() for many cases. > > Yes, that's the whole point of CONFIG_SYS_EARLY_GD. What I'm saying is > to forget about the current ifdef list around zero_global_data(), and > replace it with CONFIG_SYS_EARLY_GD, which for now would only contain > mpc85xx, mpc86xx, and x86. Other targets can skip the zeroing if and > when they also skip the stack allocation and assignment. I can agree on this. > >>> BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder >>> what is going on with gd on x86, and whether it makes sense to lump it >>> in with CONFIG_SYS_EARLY_GD. >>> >> >> Maybe x86 maintainers can chime in? If we define such macro, it should probably >> sit right above board_init_f() so it can be seen easily. There is no other place >> it is needed, yet. > > I was thinking it would be set the same way other CONFIG symbols are > set. > That will be in include/common.h for cross-platform macros. York
On Wed, 2014-04-30 at 17:01 -0700, York Sun wrote: > >>> BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder > >>> what is going on with gd on x86, and whether it makes sense to lump it > >>> in with CONFIG_SYS_EARLY_GD. > >>> > >> > >> Maybe x86 maintainers can chime in? If we define such macro, it should probably > >> sit right above board_init_f() so it can be seen easily. There is no other place > >> it is needed, yet. > > > > I was thinking it would be set the same way other CONFIG symbols are > > set. > > > > That will be in include/common.h for cross-platform macros. No, it's in the particular board files (or better, arch/subarch files included by board files) that need this. The point is to not have a big ifdef list, rather than to just move the big ifdef list. -Scott
On 04/30/2014 05:02 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 17:01 -0700, York Sun wrote: >>>>> BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder >>>>> what is going on with gd on x86, and whether it makes sense to lump it >>>>> in with CONFIG_SYS_EARLY_GD. >>>>> >>>> >>>> Maybe x86 maintainers can chime in? If we define such macro, it should probably >>>> sit right above board_init_f() so it can be seen easily. There is no other place >>>> it is needed, yet. >>> >>> I was thinking it would be set the same way other CONFIG symbols are >>> set. >>> >> >> That will be in include/common.h for cross-platform macros. > > No, it's in the particular board files (or better, arch/subarch files > included by board files) that need this. The point is to not have a big > ifdef list, rather than to just move the big ifdef list. > I got your idea. You are suggesting to define this macro in each arch which it needs to preserve gd. So we won't have a list. It is neat but may be easy to miss. York
On Wed, 2014-04-30 at 17:05 -0700, York Sun wrote: > On 04/30/2014 05:02 PM, Scott Wood wrote: > > On Wed, 2014-04-30 at 17:01 -0700, York Sun wrote: > >>>>> BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder > >>>>> what is going on with gd on x86, and whether it makes sense to lump it > >>>>> in with CONFIG_SYS_EARLY_GD. > >>>>> > >>>> > >>>> Maybe x86 maintainers can chime in? If we define such macro, it should probably > >>>> sit right above board_init_f() so it can be seen easily. There is no other place > >>>> it is needed, yet. > >>> > >>> I was thinking it would be set the same way other CONFIG symbols are > >>> set. > >>> > >> > >> That will be in include/common.h for cross-platform macros. > > > > No, it's in the particular board files (or better, arch/subarch files > > included by board files) that need this. The point is to not have a big > > ifdef list, rather than to just move the big ifdef list. > > > > I got your idea. You are suggesting to define this macro in each arch which it > needs to preserve gd. So we won't have a list. It is neat but may be easy to miss. It's how other such things are handled. We'd have a giant mess if we had lists of targets strewn throughout the codebase. -Scott
On 04/30/2014 05:11 PM, Scott Wood wrote: > On Wed, 2014-04-30 at 17:05 -0700, York Sun wrote: >> On 04/30/2014 05:02 PM, Scott Wood wrote: >>> On Wed, 2014-04-30 at 17:01 -0700, York Sun wrote: >>>>>>> BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder >>>>>>> what is going on with gd on x86, and whether it makes sense to lump it >>>>>>> in with CONFIG_SYS_EARLY_GD. >>>>>>> >>>>>> >>>>>> Maybe x86 maintainers can chime in? If we define such macro, it should probably >>>>>> sit right above board_init_f() so it can be seen easily. There is no other place >>>>>> it is needed, yet. >>>>> >>>>> I was thinking it would be set the same way other CONFIG symbols are >>>>> set. >>>>> >>>> >>>> That will be in include/common.h for cross-platform macros. >>> >>> No, it's in the particular board files (or better, arch/subarch files >>> included by board files) that need this. The point is to not have a big >>> ifdef list, rather than to just move the big ifdef list. >>> >> >> I got your idea. You are suggesting to define this macro in each arch which it >> needs to preserve gd. So we won't have a list. It is neat but may be easy to miss. > > It's how other such things are handled. We'd have a giant mess if we > had lists of targets strewn throughout the codebase. > This will be a negative logic, #ifndef CONFIG_SYS_EARLY_GD gd_t data; gd = &data; zero_global_data(); #endif York
On Wed, 2014-04-30 at 17:17 -0700, York Sun wrote: > On 04/30/2014 05:11 PM, Scott Wood wrote: > > On Wed, 2014-04-30 at 17:05 -0700, York Sun wrote: > >> On 04/30/2014 05:02 PM, Scott Wood wrote: > >>> On Wed, 2014-04-30 at 17:01 -0700, York Sun wrote: > >>>>>>> BTW, I see x86 also skips "gd = new_gd" in board_init_r(), so I wonder > >>>>>>> what is going on with gd on x86, and whether it makes sense to lump it > >>>>>>> in with CONFIG_SYS_EARLY_GD. > >>>>>>> > >>>>>> > >>>>>> Maybe x86 maintainers can chime in? If we define such macro, it should probably > >>>>>> sit right above board_init_f() so it can be seen easily. There is no other place > >>>>>> it is needed, yet. > >>>>> > >>>>> I was thinking it would be set the same way other CONFIG symbols are > >>>>> set. > >>>>> > >>>> > >>>> That will be in include/common.h for cross-platform macros. > >>> > >>> No, it's in the particular board files (or better, arch/subarch files > >>> included by board files) that need this. The point is to not have a big > >>> ifdef list, rather than to just move the big ifdef list. > >>> > >> > >> I got your idea. You are suggesting to define this macro in each arch which it > >> needs to preserve gd. So we won't have a list. It is neat but may be easy to miss. > > > > It's how other such things are handled. We'd have a giant mess if we > > had lists of targets strewn throughout the codebase. > > > > This will be a negative logic, > > #ifndef CONFIG_SYS_EARLY_GD > gd_t data; > > gd = &data; > zero_global_data(); > #endif Yes, that's fine. The "rule" against that is just for if/else where it's easy to avoid. -Scott
diff --git a/common/board_f.c b/common/board_f.c index cbdf06f..eebb377 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -970,7 +970,11 @@ static init_fnc_t init_sequence_f[] = { void board_init_f(ulong boot_flags) { -#ifndef CONFIG_X86 + /* + * For MPC85xx, global data is initialized in cpu_init_early_f() and + * used for init_law(). gd should not be cleared in this function. + */ +#if !defined(CONFIG_X86) && !defined(CONFIG_MPC85xx) && !defined(CONFIG_MPC86xx) gd_t data; gd = &data;
For powerpc SoCs (including mpc85xx, mpc86xx), global data is used for initializing LAWs, before calling function baord_inti_f(). This data should not be cleared later. Signed-off-by: York Sun <yorksun@freescale.com> --- Change log v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. Note, need other maintainers to fix 83xx, 5xxx, 512x as I don't have boards to verify. common/board_f.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)