| Message ID | 1398906061-7119-1-git-send-email-yorksun@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Hi York, On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: > Some platforms (including mpc85xx, mpc86xx) use global data before calling > function baord_inti_f(). This data should not be cleared later. Any arch > which needs to preserve global data should define CONFIG_SYS_EARLY_GD. > > Signed-off-by: York Sun <yorksun@freescale.com> > CC: Scott Wood <scottwood@freescale.com> > CC: Simon Glass <sjg@chromium.org> > --- > Change log > > v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list > v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > > README | 3 +++ > arch/powerpc/include/asm/config.h | 2 ++ > arch/x86/include/asm/config.h | 1 + > common/board_f.c | 10 +++++----- > 4 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/README b/README > index 12758dc..1a51b11 100644 > --- a/README > +++ b/README > @@ -440,6 +440,9 @@ The following options need to be configured: > supported, core will start to execute uboot when wakes up. > > - Generic CPU options: > + CONFIG_SYS_EARLY_GD > + Defines global data is used before board_init_f(). > + This looks like a good solution, but I believe that 'early' is in fact where we are headed. So perhaps you should create an inverted option and define it for archs that don't support it? For what you have, I think CONFIG_SYS_EARLY_GLOBAL_DATA might be more descriptive. Also I think you should describe the purpose more precisely here - it's not just the use that you need to discuss. - The CPU init code must set it up and zero it - Then board_init_f() doesn't zero it > CONFIG_SYS_BIG_ENDIAN, CONFIG_SYS_LITTLE_ENDIAN > > Defines the endianess of the CPU. Implementation of those > diff --git a/arch/powerpc/include/asm/config.h b/arch/powerpc/include/asm/config.h > index 423a6fb..4b6103b 100644 > --- a/arch/powerpc/include/asm/config.h > +++ b/arch/powerpc/include/asm/config.h > @@ -10,11 +10,13 @@ > #ifdef CONFIG_MPC85xx > #include <asm/config_mpc85xx.h> > #define CONFIG_SYS_FSL_DDR > +#define CONFIG_SYS_EARLY_GD > #endif > > #ifdef CONFIG_MPC86xx > #include <asm/config_mpc86xx.h> > #define CONFIG_SYS_FSL_DDR > +#define CONFIG_SYS_EARLY_GD > #endif > > #ifdef CONFIG_MPC83xx > diff --git a/arch/x86/include/asm/config.h b/arch/x86/include/asm/config.h > index f06a15c..940461d 100644 > --- a/arch/x86/include/asm/config.h > +++ b/arch/x86/include/asm/config.h > @@ -7,5 +7,6 @@ > #ifndef _ASM_CONFIG_H_ > #define _ASM_CONFIG_H_ > > +#define CONFIG_SYS_EARLY_GD > #define CONFIG_SYS_GENERIC_BOARD > #endif > diff --git a/common/board_f.c b/common/board_f.c > index cbdf06f..0823bed 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -970,20 +970,20 @@ static init_fnc_t init_sequence_f[] = { > > void board_init_f(ulong boot_flags) > { > -#ifndef CONFIG_X86 > + /* > + * For some archtecture, global data is initialized and used before > + * calling this function. The data should be preserved. > + */ > +#ifndef CONFIG_SYS_EARLY_GD > gd_t data; > > gd = &data; > -#endif > > /* > * Clear global data before it is accessed at debug print > * in initcall_run_list. Otherwise the debug print probably > * get the wrong vaule of gd->have_console. > */ > -#if !defined(CONFIG_CPM2) && !defined(CONFIG_MPC512X) && \ > - !defined(CONFIG_MPC83xx) && !defined(CONFIG_MPC85xx) && \ > - !defined(CONFIG_MPC86xx) && !defined(CONFIG_X86) > zero_global_data(); > #endif As a separate issue/patch, I think this should be defined for ARM also. Regards, Simon
On 05/01/2014 08:54 AM, Simon Glass wrote: > Hi York, > > On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: >> Some platforms (including mpc85xx, mpc86xx) use global data before calling >> function baord_inti_f(). This data should not be cleared later. Any arch >> which needs to preserve global data should define CONFIG_SYS_EARLY_GD. >> >> Signed-off-by: York Sun <yorksun@freescale.com> >> CC: Scott Wood <scottwood@freescale.com> >> CC: Simon Glass <sjg@chromium.org> >> --- >> Change log >> >> v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list >> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >> >> README | 3 +++ >> arch/powerpc/include/asm/config.h | 2 ++ >> arch/x86/include/asm/config.h | 1 + >> common/board_f.c | 10 +++++----- >> 4 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/README b/README >> index 12758dc..1a51b11 100644 >> --- a/README >> +++ b/README >> @@ -440,6 +440,9 @@ The following options need to be configured: >> supported, core will start to execute uboot when wakes up. >> >> - Generic CPU options: >> + CONFIG_SYS_EARLY_GD >> + Defines global data is used before board_init_f(). >> + > > This looks like a good solution, but I believe that 'early' is in fact > where we are headed. So perhaps you should create an inverted option > and define it for archs that don't support it? I don't disagree for the long term. But since we just start to convert to generic board, fewer archs need this than those don't. I suggest we try this and when most archs convert to generic board, we can invert it if needed. > > For what you have, I think CONFIG_SYS_EARLY_GLOBAL_DATA might be more > descriptive. Also I think you should describe the purpose more > precisely here - it's not just the use that you need to discuss. > > - The CPU init code must set it up and zero it > - Then board_init_f() doesn't zero it > Will add these. York
On Thu, 2014-05-01 at 14:16 -0700, York Sun wrote: > On 05/01/2014 08:54 AM, Simon Glass wrote: > > Hi York, > > > > On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: > >> Some platforms (including mpc85xx, mpc86xx) use global data before calling > >> function baord_inti_f(). This data should not be cleared later. Any arch > >> which needs to preserve global data should define CONFIG_SYS_EARLY_GD. > >> > >> Signed-off-by: York Sun <yorksun@freescale.com> > >> CC: Scott Wood <scottwood@freescale.com> > >> CC: Simon Glass <sjg@chromium.org> > >> --- > >> Change log > >> > >> v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list > >> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > >> > >> README | 3 +++ > >> arch/powerpc/include/asm/config.h | 2 ++ > >> arch/x86/include/asm/config.h | 1 + > >> common/board_f.c | 10 +++++----- > >> 4 files changed, 11 insertions(+), 5 deletions(-) > >> > >> diff --git a/README b/README > >> index 12758dc..1a51b11 100644 > >> --- a/README > >> +++ b/README > >> @@ -440,6 +440,9 @@ The following options need to be configured: > >> supported, core will start to execute uboot when wakes up. > >> > >> - Generic CPU options: > >> + CONFIG_SYS_EARLY_GD > >> + Defines global data is used before board_init_f(). > >> + > > > > This looks like a good solution, but I believe that 'early' is in fact > > where we are headed. So perhaps you should create an inverted option > > and define it for archs that don't support it? > > I don't disagree for the long term. But since we just start to convert to > generic board, fewer archs need this than those don't. I suggest we try this and > when most archs convert to generic board, we can invert it if needed. Why wait to do it right? Which arches actually don't need this, given that Simon said he thinks ARM needs it as well? -Scott
On 05/01/2014 02:21 PM, Scott Wood wrote: > On Thu, 2014-05-01 at 14:16 -0700, York Sun wrote: >> On 05/01/2014 08:54 AM, Simon Glass wrote: >>> Hi York, >>> >>> On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: >>>> Some platforms (including mpc85xx, mpc86xx) use global data before calling >>>> function baord_inti_f(). This data should not be cleared later. Any arch >>>> which needs to preserve global data should define CONFIG_SYS_EARLY_GD. >>>> >>>> Signed-off-by: York Sun <yorksun@freescale.com> >>>> CC: Scott Wood <scottwood@freescale.com> >>>> CC: Simon Glass <sjg@chromium.org> >>>> --- >>>> Change log >>>> >>>> v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list >>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >>>> >>>> README | 3 +++ >>>> arch/powerpc/include/asm/config.h | 2 ++ >>>> arch/x86/include/asm/config.h | 1 + >>>> common/board_f.c | 10 +++++----- >>>> 4 files changed, 11 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/README b/README >>>> index 12758dc..1a51b11 100644 >>>> --- a/README >>>> +++ b/README >>>> @@ -440,6 +440,9 @@ The following options need to be configured: >>>> supported, core will start to execute uboot when wakes up. >>>> >>>> - Generic CPU options: >>>> + CONFIG_SYS_EARLY_GD >>>> + Defines global data is used before board_init_f(). >>>> + >>> >>> This looks like a good solution, but I believe that 'early' is in fact >>> where we are headed. So perhaps you should create an inverted option >>> and define it for archs that don't support it? >> >> I don't disagree for the long term. But since we just start to convert to >> generic board, fewer archs need this than those don't. I suggest we try this and >> when most archs convert to generic board, we can invert it if needed. > > Why wait to do it right? Which arches actually don't need this, given > that Simon said he thinks ARM needs it as well? > Adding something I can't test? Even Simon said he thinks ARM needs it, it is not what current code has. If I revert this logic, #ifdef CONFIG_SYS_GLOBAL_DATA_STACK gd_dt data; gd = &data; zero_global_data(); #endif I need to add #define CONFIG_SYS_GLOBAL_DATA_STACK to the board/arch who needs it. I have no idea which needs in the 70+ boards with CONFIG_DISPLAY_BOARDINFO defined. York
On Thu, 2014-05-01 at 14:29 -0700, York Sun wrote: > On 05/01/2014 02:21 PM, Scott Wood wrote: > > On Thu, 2014-05-01 at 14:16 -0700, York Sun wrote: > >> On 05/01/2014 08:54 AM, Simon Glass wrote: > >>> Hi York, > >>> > >>> On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: > >>>> Some platforms (including mpc85xx, mpc86xx) use global data before calling > >>>> function baord_inti_f(). This data should not be cleared later. Any arch > >>>> which needs to preserve global data should define CONFIG_SYS_EARLY_GD. > >>>> > >>>> Signed-off-by: York Sun <yorksun@freescale.com> > >>>> CC: Scott Wood <scottwood@freescale.com> > >>>> CC: Simon Glass <sjg@chromium.org> > >>>> --- > >>>> Change log > >>>> > >>>> v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list > >>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. > >>>> > >>>> README | 3 +++ > >>>> arch/powerpc/include/asm/config.h | 2 ++ > >>>> arch/x86/include/asm/config.h | 1 + > >>>> common/board_f.c | 10 +++++----- > >>>> 4 files changed, 11 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/README b/README > >>>> index 12758dc..1a51b11 100644 > >>>> --- a/README > >>>> +++ b/README > >>>> @@ -440,6 +440,9 @@ The following options need to be configured: > >>>> supported, core will start to execute uboot when wakes up. > >>>> > >>>> - Generic CPU options: > >>>> + CONFIG_SYS_EARLY_GD > >>>> + Defines global data is used before board_init_f(). > >>>> + > >>> > >>> This looks like a good solution, but I believe that 'early' is in fact > >>> where we are headed. So perhaps you should create an inverted option > >>> and define it for archs that don't support it? > >> > >> I don't disagree for the long term. But since we just start to convert to > >> generic board, fewer archs need this than those don't. I suggest we try this and > >> when most archs convert to generic board, we can invert it if needed. > > > > Why wait to do it right? Which arches actually don't need this, given > > that Simon said he thinks ARM needs it as well? > > > > Adding something I can't test? By that rationale nobody could ever make changes that aren't localized to a few boards. That type of development produces the sort of mess that the generic board is meant to remedy. > Even Simon said he thinks ARM needs it, it is not what current code has. If I > revert this logic, > > #ifdef CONFIG_SYS_GLOBAL_DATA_STACK This looks like a global stack of data, not global data on the stack. How about CONFIG_SYS_GENERIC_GLOBAL_DATA? > gd_dt data; > > gd = &data; > zero_global_data(); > #endif > > I need to add #define CONFIG_SYS_GLOBAL_DATA_STACK to the board/arch who needs > it. I have no idea which needs in the 70+ boards with CONFIG_DISPLAY_BOARDINFO > defined. I see only 10 boards that currently use CONFIG_SYS_GENERIC_BOARD in include/configs, plus sandbox and x86 which hide it in config.mk. I don't know what CONFIG_DISPLAY_BOARDINFO has to do with it. To invert the symbol, add the new symbol to all of the current users. Removal from ARM boards can come afterward if it's confirmed that it's the right thing to do. -Scott
+Albert Hi, On 1 May 2014 15:41, Scott Wood <scottwood@freescale.com> wrote: > On Thu, 2014-05-01 at 14:29 -0700, York Sun wrote: >> On 05/01/2014 02:21 PM, Scott Wood wrote: >> > On Thu, 2014-05-01 at 14:16 -0700, York Sun wrote: >> >> On 05/01/2014 08:54 AM, Simon Glass wrote: >> >>> Hi York, >> >>> >> >>> On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: >> >>>> Some platforms (including mpc85xx, mpc86xx) use global data before calling >> >>>> function baord_inti_f(). This data should not be cleared later. Any arch >> >>>> which needs to preserve global data should define CONFIG_SYS_EARLY_GD. >> >>>> >> >>>> Signed-off-by: York Sun <yorksun@freescale.com> >> >>>> CC: Scott Wood <scottwood@freescale.com> >> >>>> CC: Simon Glass <sjg@chromium.org> >> >>>> --- >> >>>> Change log >> >>>> >> >>>> v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list >> >>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >> >>>> >> >>>> README | 3 +++ >> >>>> arch/powerpc/include/asm/config.h | 2 ++ >> >>>> arch/x86/include/asm/config.h | 1 + >> >>>> common/board_f.c | 10 +++++----- >> >>>> 4 files changed, 11 insertions(+), 5 deletions(-) >> >>>> >> >>>> diff --git a/README b/README >> >>>> index 12758dc..1a51b11 100644 >> >>>> --- a/README >> >>>> +++ b/README >> >>>> @@ -440,6 +440,9 @@ The following options need to be configured: >> >>>> supported, core will start to execute uboot when wakes up. >> >>>> >> >>>> - Generic CPU options: >> >>>> + CONFIG_SYS_EARLY_GD >> >>>> + Defines global data is used before board_init_f(). >> >>>> + >> >>> >> >>> This looks like a good solution, but I believe that 'early' is in fact >> >>> where we are headed. So perhaps you should create an inverted option >> >>> and define it for archs that don't support it? >> >> >> >> I don't disagree for the long term. But since we just start to convert to >> >> generic board, fewer archs need this than those don't. I suggest we try this and >> >> when most archs convert to generic board, we can invert it if needed. >> > >> > Why wait to do it right? Which arches actually don't need this, given >> > that Simon said he thinks ARM needs it as well? Thanks for working on this, York. I looked at ARM and I think it is similar. See arch/arm/lib/crt0.S. It sets up gd - the only problem is that it does not zero the area unfortunately. But once that is added we can switch ARM over. >> > >> >> Adding something I can't test? > > By that rationale nobody could ever make changes that aren't localized > to a few boards. That type of development produces the sort of mess > that the generic board is meant to remedy. > >> Even Simon said he thinks ARM needs it, it is not what current code has. If I >> revert this logic, >> >> #ifdef CONFIG_SYS_GLOBAL_DATA_STACK > > This looks like a global stack of data, not global data on the stack. > > How about CONFIG_SYS_GENERIC_GLOBAL_DATA? > >> gd_dt data; >> >> gd = &data; >> zero_global_data(); >> #endif >> >> I need to add #define CONFIG_SYS_GLOBAL_DATA_STACK to the board/arch who needs >> it. I have no idea which needs in the 70+ boards with CONFIG_DISPLAY_BOARDINFO >> defined. > > I see only 10 boards that currently use CONFIG_SYS_GENERIC_BOARD in > include/configs, plus sandbox and x86 which hide it in config.mk. I > don't know what CONFIG_DISPLAY_BOARDINFO has to do with it. To invert > the symbol, add the new symbol to all of the current users. Removal > from ARM boards can come afterward if it's confirmed that it's the right > thing to do. SGTM. Regards, Simon
On 05/01/2014 02:41 PM, Scott Wood wrote: > On Thu, 2014-05-01 at 14:29 -0700, York Sun wrote: >> On 05/01/2014 02:21 PM, Scott Wood wrote: >>> On Thu, 2014-05-01 at 14:16 -0700, York Sun wrote: >>>> On 05/01/2014 08:54 AM, Simon Glass wrote: >>>>> Hi York, >>>>> >>>>> On 30 April 2014 18:01, York Sun <yorksun@freescale.com> wrote: >>>>>> Some platforms (including mpc85xx, mpc86xx) use global data before calling >>>>>> function baord_inti_f(). This data should not be cleared later. Any arch >>>>>> which needs to preserve global data should define CONFIG_SYS_EARLY_GD. >>>>>> >>>>>> Signed-off-by: York Sun <yorksun@freescale.com> >>>>>> CC: Scott Wood <scottwood@freescale.com> >>>>>> CC: Simon Glass <sjg@chromium.org> >>>>>> --- >>>>>> Change log >>>>>> >>>>>> v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list >>>>>> v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. >>>>>> >>>>>> README | 3 +++ >>>>>> arch/powerpc/include/asm/config.h | 2 ++ >>>>>> arch/x86/include/asm/config.h | 1 + >>>>>> common/board_f.c | 10 +++++----- >>>>>> 4 files changed, 11 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/README b/README >>>>>> index 12758dc..1a51b11 100644 >>>>>> --- a/README >>>>>> +++ b/README >>>>>> @@ -440,6 +440,9 @@ The following options need to be configured: >>>>>> supported, core will start to execute uboot when wakes up. >>>>>> >>>>>> - Generic CPU options: >>>>>> + CONFIG_SYS_EARLY_GD >>>>>> + Defines global data is used before board_init_f(). >>>>>> + >>>>> >>>>> This looks like a good solution, but I believe that 'early' is in fact >>>>> where we are headed. So perhaps you should create an inverted option >>>>> and define it for archs that don't support it? >>>> >>>> I don't disagree for the long term. But since we just start to convert to >>>> generic board, fewer archs need this than those don't. I suggest we try this and >>>> when most archs convert to generic board, we can invert it if needed. >>> >>> Why wait to do it right? Which arches actually don't need this, given >>> that Simon said he thinks ARM needs it as well? >>> >> >> Adding something I can't test? > > By that rationale nobody could ever make changes that aren't localized > to a few boards. That type of development produces the sort of mess > that the generic board is meant to remedy. > >> Even Simon said he thinks ARM needs it, it is not what current code has. If I >> revert this logic, >> >> #ifdef CONFIG_SYS_GLOBAL_DATA_STACK > > This looks like a global stack of data, not global data on the stack. > > How about CONFIG_SYS_GENERIC_GLOBAL_DATA? > >> gd_dt data; >> >> gd = &data; >> zero_global_data(); >> #endif >> >> I need to add #define CONFIG_SYS_GLOBAL_DATA_STACK to the board/arch who needs >> it. I have no idea which needs in the 70+ boards with CONFIG_DISPLAY_BOARDINFO >> defined. > > I see only 10 boards that currently use CONFIG_SYS_GENERIC_BOARD in > include/configs, plus sandbox and x86 which hide it in config.mk. I > don't know what CONFIG_DISPLAY_BOARDINFO has to do with it. To invert > the symbol, add the new symbol to all of the current users. Removal > from ARM boards can come afterward if it's confirmed that it's the right > thing to do. I made a typo when I search it. Yes, it is only 10 boards. I can add the new macro to their arch header file. York York
diff --git a/README b/README index 12758dc..1a51b11 100644 --- a/README +++ b/README @@ -440,6 +440,9 @@ The following options need to be configured: supported, core will start to execute uboot when wakes up. - Generic CPU options: + CONFIG_SYS_EARLY_GD + Defines global data is used before board_init_f(). + CONFIG_SYS_BIG_ENDIAN, CONFIG_SYS_LITTLE_ENDIAN Defines the endianess of the CPU. Implementation of those diff --git a/arch/powerpc/include/asm/config.h b/arch/powerpc/include/asm/config.h index 423a6fb..4b6103b 100644 --- a/arch/powerpc/include/asm/config.h +++ b/arch/powerpc/include/asm/config.h @@ -10,11 +10,13 @@ #ifdef CONFIG_MPC85xx #include <asm/config_mpc85xx.h> #define CONFIG_SYS_FSL_DDR +#define CONFIG_SYS_EARLY_GD #endif #ifdef CONFIG_MPC86xx #include <asm/config_mpc86xx.h> #define CONFIG_SYS_FSL_DDR +#define CONFIG_SYS_EARLY_GD #endif #ifdef CONFIG_MPC83xx diff --git a/arch/x86/include/asm/config.h b/arch/x86/include/asm/config.h index f06a15c..940461d 100644 --- a/arch/x86/include/asm/config.h +++ b/arch/x86/include/asm/config.h @@ -7,5 +7,6 @@ #ifndef _ASM_CONFIG_H_ #define _ASM_CONFIG_H_ +#define CONFIG_SYS_EARLY_GD #define CONFIG_SYS_GENERIC_BOARD #endif diff --git a/common/board_f.c b/common/board_f.c index cbdf06f..0823bed 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -970,20 +970,20 @@ static init_fnc_t init_sequence_f[] = { void board_init_f(ulong boot_flags) { -#ifndef CONFIG_X86 + /* + * For some archtecture, global data is initialized and used before + * calling this function. The data should be preserved. + */ +#ifndef CONFIG_SYS_EARLY_GD gd_t data; gd = &data; -#endif /* * Clear global data before it is accessed at debug print * in initcall_run_list. Otherwise the debug print probably * get the wrong vaule of gd->have_console. */ -#if !defined(CONFIG_CPM2) && !defined(CONFIG_MPC512X) && \ - !defined(CONFIG_MPC83xx) && !defined(CONFIG_MPC85xx) && \ - !defined(CONFIG_MPC86xx) && !defined(CONFIG_X86) zero_global_data(); #endif
Some platforms (including mpc85xx, mpc86xx) use global data before calling function baord_inti_f(). This data should not be cleared later. Any arch which needs to preserve global data should define CONFIG_SYS_EARLY_GD. Signed-off-by: York Sun <yorksun@freescale.com> CC: Scott Wood <scottwood@freescale.com> CC: Simon Glass <sjg@chromium.org> --- Change log v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx. README | 3 +++ arch/powerpc/include/asm/config.h | 2 ++ arch/x86/include/asm/config.h | 1 + common/board_f.c | 10 +++++----- 4 files changed, 11 insertions(+), 5 deletions(-)