| Message ID | 1369030048-26130-5-git-send-email-ying.zhang@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Andy Fleming |
| Headers | show |
Please change the title and the rest of the changelog to describe what functionality you're adding and why. On 05/20/2013 01:07:27 AM, ying.zhang@freescale.com wrote: > diff --git a/common/env_common.c b/common/env_common.c > index 906b41f..8cb81e9 100644 > --- a/common/env_common.c > +++ b/common/env_common.c > @@ -156,7 +156,6 @@ int set_default_vars(int nvars, char * const > vars[]) > H_NOCLEAR | H_INTERACTIVE, nvars, vars); > } > > -#ifndef CONFIG_SPL_BUILD > /* > * Check if CRC is valid and (if yes) import the environment. > * Note that "buf" may or may not be aligned. > @@ -188,7 +187,6 @@ int env_import(const char *buf, int check) > > return 0; > } > -#endif This ifndef was introduced by Ilya Yanok in commit 7ac2fe2da21d292aeaf3af74e5c80de9ce9dab56. Ilya, what are the consequences of removing this? Is there some other symbol we can use here? > diff --git a/include/configs/MPC8313ERDB.h > b/include/configs/MPC8313ERDB.h > index c28dfe0..a2bdcff 100644 > --- a/include/configs/MPC8313ERDB.h > +++ b/include/configs/MPC8313ERDB.h > @@ -40,7 +40,9 @@ > #define CONFIG_SPL_INIT_MINIMAL > #define CONFIG_SPL_SERIAL_SUPPORT > #define CONFIG_SPL_NAND_SUPPORT > +#ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_NAND_MINIMAL > +#endif > #define CONFIG_SPL_FLUSH_IMAGE > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND > diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > index 8b13b10..5bdd44a 100644 > --- a/include/configs/P1022DS.h > +++ b/include/configs/P1022DS.h > @@ -41,7 +41,9 @@ > #define CONFIG_SPL_INIT_MINIMAL > #define CONFIG_SPL_SERIAL_SUPPORT > #define CONFIG_SPL_NAND_SUPPORT > +#ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_NAND_MINIMAL > +#endif > #define CONFIG_SPL_FLUSH_IMAGE > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > diff --git a/include/configs/p1_p2_rdb_pc.h > b/include/configs/p1_p2_rdb_pc.h > index 7ed634b..bc48d62 100644 > --- a/include/configs/p1_p2_rdb_pc.h > +++ b/include/configs/p1_p2_rdb_pc.h > @@ -159,7 +159,9 @@ > #define CONFIG_SPL_INIT_MINIMAL > #define CONFIG_SPL_SERIAL_SUPPORT > #define CONFIG_SPL_NAND_SUPPORT > +#ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_NAND_MINIMAL > +#endif > #define CONFIG_SPL_FLUSH_IMAGE > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" Are you sure this belongs in this patch? -Scott
> diff --git a/include/configs/MPC8313ERDB.h > b/include/configs/MPC8313ERDB.h > index c28dfe0..a2bdcff 100644 > --- a/include/configs/MPC8313ERDB.h > +++ b/include/configs/MPC8313ERDB.h > @@ -40,7 +40,9 @@ > #define CONFIG_SPL_INIT_MINIMAL > #define CONFIG_SPL_SERIAL_SUPPORT > #define CONFIG_SPL_NAND_SUPPORT > +#ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_NAND_MINIMAL > +#endif > #define CONFIG_SPL_FLUSH_IMAGE > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND > diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > index 8b13b10..5bdd44a 100644 > --- a/include/configs/P1022DS.h > +++ b/include/configs/P1022DS.h > @@ -41,7 +41,9 @@ > #define CONFIG_SPL_INIT_MINIMAL > #define CONFIG_SPL_SERIAL_SUPPORT > #define CONFIG_SPL_NAND_SUPPORT > +#ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_NAND_MINIMAL > +#endif > #define CONFIG_SPL_FLUSH_IMAGE > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > diff --git a/include/configs/p1_p2_rdb_pc.h > b/include/configs/p1_p2_rdb_pc.h > index 7ed634b..bc48d62 100644 > --- a/include/configs/p1_p2_rdb_pc.h > +++ b/include/configs/p1_p2_rdb_pc.h > @@ -159,7 +159,9 @@ > #define CONFIG_SPL_INIT_MINIMAL > #define CONFIG_SPL_SERIAL_SUPPORT > #define CONFIG_SPL_NAND_SUPPORT > +#ifdef CONFIG_SPL_BUILD > #define CONFIG_SPL_NAND_MINIMAL > +#endif > #define CONFIG_SPL_FLUSH_IMAGE > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" Are you sure this belongs in this patch? [Zhang Ying] Yes, it is necessary. Because CONFIG_SPL_NAND_MINIMAL has been used in the file law.c and tlb.c in this patch.
On 05/21/2013 09:15:08 PM, Zhang Ying-B40530 wrote: > > diff --git a/include/configs/MPC8313ERDB.h > > b/include/configs/MPC8313ERDB.h > > index c28dfe0..a2bdcff 100644 > > --- a/include/configs/MPC8313ERDB.h > > +++ b/include/configs/MPC8313ERDB.h > > @@ -40,7 +40,9 @@ > > #define CONFIG_SPL_INIT_MINIMAL > > #define CONFIG_SPL_SERIAL_SUPPORT > > #define CONFIG_SPL_NAND_SUPPORT > > +#ifdef CONFIG_SPL_BUILD > > #define CONFIG_SPL_NAND_MINIMAL > > +#endif > > #define CONFIG_SPL_FLUSH_IMAGE > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND > > diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > > index 8b13b10..5bdd44a 100644 > > --- a/include/configs/P1022DS.h > > +++ b/include/configs/P1022DS.h > > @@ -41,7 +41,9 @@ > > #define CONFIG_SPL_INIT_MINIMAL > > #define CONFIG_SPL_SERIAL_SUPPORT > > #define CONFIG_SPL_NAND_SUPPORT > > +#ifdef CONFIG_SPL_BUILD > > #define CONFIG_SPL_NAND_MINIMAL > > +#endif > > #define CONFIG_SPL_FLUSH_IMAGE > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > diff --git a/include/configs/p1_p2_rdb_pc.h > > b/include/configs/p1_p2_rdb_pc.h > > index 7ed634b..bc48d62 100644 > > --- a/include/configs/p1_p2_rdb_pc.h > > +++ b/include/configs/p1_p2_rdb_pc.h > > @@ -159,7 +159,9 @@ > > #define CONFIG_SPL_INIT_MINIMAL > > #define CONFIG_SPL_SERIAL_SUPPORT > > #define CONFIG_SPL_NAND_SUPPORT > > +#ifdef CONFIG_SPL_BUILD > > #define CONFIG_SPL_NAND_MINIMAL > > +#endif > > #define CONFIG_SPL_FLUSH_IMAGE > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > Are you sure this belongs in this patch? > [Zhang Ying] > Yes, it is necessary. Because CONFIG_SPL_NAND_MINIMAL has been used > in the file law.c and tlb.c in this patch. What I mean is that it should probably have been done earlier, when you introduced CONFIG_SPL_NAND_MINIMAL. -Scott
-----Original Message----- From: Wood Scott-B07421 Sent: Wednesday, May 22, 2013 11:46 PM To: Zhang Ying-B40530 Cc: Wood Scott-B07421; u-boot@lists.denx.de; afleming@gmail.com; Xie Xiaobo-R63061; Ilya Yanok Subject: Re: [PATCH 5/6] spl: Make CONFIG_SPL_BUILD contain more functionality On 05/21/2013 09:15:08 PM, Zhang Ying-B40530 wrote: > > diff --git a/include/configs/MPC8313ERDB.h > > b/include/configs/MPC8313ERDB.h > > index c28dfe0..a2bdcff 100644 > > --- a/include/configs/MPC8313ERDB.h > > +++ b/include/configs/MPC8313ERDB.h > > @@ -40,7 +40,9 @@ > > #define CONFIG_SPL_INIT_MINIMAL > > #define CONFIG_SPL_SERIAL_SUPPORT > > #define CONFIG_SPL_NAND_SUPPORT > > +#ifdef CONFIG_SPL_BUILD > > #define CONFIG_SPL_NAND_MINIMAL > > +#endif > > #define CONFIG_SPL_FLUSH_IMAGE > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND > > diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > > index 8b13b10..5bdd44a 100644 > > --- a/include/configs/P1022DS.h > > +++ b/include/configs/P1022DS.h > > @@ -41,7 +41,9 @@ > > #define CONFIG_SPL_INIT_MINIMAL > > #define CONFIG_SPL_SERIAL_SUPPORT > > #define CONFIG_SPL_NAND_SUPPORT > > +#ifdef CONFIG_SPL_BUILD > > #define CONFIG_SPL_NAND_MINIMAL > > +#endif > > #define CONFIG_SPL_FLUSH_IMAGE > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > diff --git a/include/configs/p1_p2_rdb_pc.h > > b/include/configs/p1_p2_rdb_pc.h > > index 7ed634b..bc48d62 100644 > > --- a/include/configs/p1_p2_rdb_pc.h > > +++ b/include/configs/p1_p2_rdb_pc.h > > @@ -159,7 +159,9 @@ > > #define CONFIG_SPL_INIT_MINIMAL > > #define CONFIG_SPL_SERIAL_SUPPORT > > #define CONFIG_SPL_NAND_SUPPORT > > +#ifdef CONFIG_SPL_BUILD > > #define CONFIG_SPL_NAND_MINIMAL > > +#endif > > #define CONFIG_SPL_FLUSH_IMAGE > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > Are you sure this belongs in this patch? > [Zhang Ying] > Yes, it is necessary. Because CONFIG_SPL_NAND_MINIMAL has been used > in the file law.c and tlb.c in this patch. What I mean is that it should probably have been done earlier, when you introduced CONFIG_SPL_NAND_MINIMAL. [Zhang Ying] I can understand you mean. Because the symbol "CONFIG_SPL_NAND_MINIMAL" has been useless, I think no need to split out a separate patch.
On Tue, May 21, 2013 at 02:42:02PM -0500, Scott Wood wrote: > Please change the title and the rest of the changelog to describe > what functionality you're adding and why. > > On 05/20/2013 01:07:27 AM, ying.zhang@freescale.com wrote: > >diff --git a/common/env_common.c b/common/env_common.c > >index 906b41f..8cb81e9 100644 > >--- a/common/env_common.c > >+++ b/common/env_common.c > >@@ -156,7 +156,6 @@ int set_default_vars(int nvars, char * const > >vars[]) > > H_NOCLEAR | H_INTERACTIVE, nvars, vars); > > } > > > >-#ifndef CONFIG_SPL_BUILD > > /* > > * Check if CRC is valid and (if yes) import the environment. > > * Note that "buf" may or may not be aligned. > >@@ -188,7 +187,6 @@ int env_import(const char *buf, int check) > > > > return 0; > > } > >-#endif > > This ifndef was introduced by Ilya Yanok in commit > 7ac2fe2da21d292aeaf3af74e5c80de9ce9dab56. > > Ilya, what are the consequences of removing this? Is there some > other symbol we can use here? This was likely done so that we could fit the combination of USB RNDIS networking in SPL into the size constraint we have and the need for more than just unused section discarding to get rid of codepaths we know won't be taken (but couldn't compile-time determine).
On Mon, May 20, 2013 at 02:07:27PM +0800, ying.zhang@freescale.com wrote: > From: Ying Zhang <b40530@freescale.com> > > There was some functionality will be used in the SPL. They > had been excluded by ifndef CONFIG_SPL_BUILD. Now, put it > into the SPL. > > Signed-off-by: Ying Zhang <b40530@freescale.com> > --- > arch/powerpc/cpu/mpc85xx/tlb.c | 2 +- > arch/powerpc/cpu/mpc8xxx/law.c | 4 ++-- In these cases can we not just always build them, aside from when CONFIG_NAND_SPL is set and rely on link-time discard here? Otherwise: > -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) > +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_NAND_MINIMAL) Should become, I believe (and this isn't whitespaced properly): #if !defined(CONFIG_NAND_SPL) && !(defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_NAND_MINIMAL)) So that: > diff --git a/include/configs/MPC8313ERDB.h b/include/configs/MPC8313ERDB.h [snip] > diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h [snip] > diff --git a/include/configs/p1_p2_rdb_pc.h b/include/configs/p1_p2_rdb_pc.h Can then all be dropped.
On Fri, May 24, 2013 at 02:08:07PM -0500, Scott Wood wrote: > On 05/24/2013 11:11:01 AM, Tom Rini wrote: > >On Mon, May 20, 2013 at 02:07:27PM +0800, ying.zhang@freescale.com > >wrote: > > > >> From: Ying Zhang <b40530@freescale.com> > >> > >> There was some functionality will be used in the SPL. They > >> had been excluded by ifndef CONFIG_SPL_BUILD. Now, put it > >> into the SPL. > >> > >> Signed-off-by: Ying Zhang <b40530@freescale.com> > >> --- > >> arch/powerpc/cpu/mpc85xx/tlb.c | 2 +- > >> arch/powerpc/cpu/mpc8xxx/law.c | 4 ++-- > > > >In these cases can we not just always build them, aside from when > >CONFIG_NAND_SPL is set and rely on link-time discard here? Otherwise: > > > > > >> -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) > >> +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_NAND_MINIMAL) > > > >Should become, I believe (and this isn't whitespaced properly): > >#if !defined(CONFIG_NAND_SPL) && !(defined(CONFIG_SPL_BUILD) && > >defined(CONFIG_SPL_NAND_MINIMAL)) > > > >So that: > > > >> diff --git a/include/configs/MPC8313ERDB.h > >b/include/configs/MPC8313ERDB.h > >[snip] > >> diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > >[snip] > >> diff --git a/include/configs/p1_p2_rdb_pc.h > >b/include/configs/p1_p2_rdb_pc.h > > > >Can then all be dropped. > > Possibly, but it would be nice to limit SPL symbols to only be > defined for the SPL part of the build, so we don't have to add > checks for CONFIG_SPL_BUILD all over the place. Currently this > won't work for symbols that makefiles look at, though there was a > patch to fix that, which I referred to elsewhere in the these > threads. At the high level, yes, I agree it would be good to clean up everyones configs around CONFIG_SPL/CONFIG_SPL_BUILD.
So, about this patch, what need I do? -----Original Message----- From: Tom Rini [mailto:tom.rini@gmail.com] On Behalf Of Tom Rini Sent: Saturday, May 25, 2013 3:18 AM To: Wood Scott-B07421 Cc: u-boot@lists.denx.de; Zhang Ying-B40530; afleming@gmail.com; Xie Xiaobo-R63061; Zhang Ying-B40530 Subject: Re: [U-Boot] [PATCH 5/6] spl: Make CONFIG_SPL_BUILD contain more functionality On Fri, May 24, 2013 at 02:08:07PM -0500, Scott Wood wrote: > On 05/24/2013 11:11:01 AM, Tom Rini wrote: > >On Mon, May 20, 2013 at 02:07:27PM +0800, ying.zhang@freescale.com > >wrote: > > > >> From: Ying Zhang <b40530@freescale.com> > >> > >> There was some functionality will be used in the SPL. They > >> had been excluded by ifndef CONFIG_SPL_BUILD. Now, put it > >> into the SPL. > >> > >> Signed-off-by: Ying Zhang <b40530@freescale.com> > >> --- > >> arch/powerpc/cpu/mpc85xx/tlb.c | 2 +- > >> arch/powerpc/cpu/mpc8xxx/law.c | 4 ++-- > > > >In these cases can we not just always build them, aside from when > >CONFIG_NAND_SPL is set and rely on link-time discard here? Otherwise: > > > > > >> -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) > >> +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_NAND_MINIMAL) > > > >Should become, I believe (and this isn't whitespaced properly): > >#if !defined(CONFIG_NAND_SPL) && !(defined(CONFIG_SPL_BUILD) && > >defined(CONFIG_SPL_NAND_MINIMAL)) > > > >So that: > > > >> diff --git a/include/configs/MPC8313ERDB.h > >b/include/configs/MPC8313ERDB.h > >[snip] > >> diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > >[snip] > >> diff --git a/include/configs/p1_p2_rdb_pc.h > >b/include/configs/p1_p2_rdb_pc.h > > > >Can then all be dropped. > > Possibly, but it would be nice to limit SPL symbols to only be > defined for the SPL part of the build, so we don't have to add > checks for CONFIG_SPL_BUILD all over the place. Currently this > won't work for symbols that makefiles look at, though there was a > patch to fix that, which I referred to elsewhere in the these > threads. At the high level, yes, I agree it would be good to clean up everyones configs around CONFIG_SPL/CONFIG_SPL_BUILD.
-----Original Message----- From: Wood Scott-B07421 Sent: Wednesday, May 29, 2013 6:34 AM To: Zhang Ying-B40530 Cc: Wood Scott-B07421; u-boot@lists.denx.de; afleming@gmail.com; Xie Xiaobo-R63061; Ilya Yanok Subject: Re: [PATCH 5/6] spl: Make CONFIG_SPL_BUILD contain more functionality On 05/22/2013 08:26:31 PM, Zhang Ying-B40530 wrote: > > > -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, May 22, 2013 11:46 PM > To: Zhang Ying-B40530 > Cc: Wood Scott-B07421; u-boot@lists.denx.de; afleming@gmail.com; Xie > Xiaobo-R63061; Ilya Yanok > Subject: Re: [PATCH 5/6] spl: Make CONFIG_SPL_BUILD contain more > functionality > > On 05/21/2013 09:15:08 PM, Zhang Ying-B40530 wrote: > > > diff --git a/include/configs/MPC8313ERDB.h > > > b/include/configs/MPC8313ERDB.h > > > index c28dfe0..a2bdcff 100644 > > > --- a/include/configs/MPC8313ERDB.h > > > +++ b/include/configs/MPC8313ERDB.h > > > @@ -40,7 +40,9 @@ > > > #define CONFIG_SPL_INIT_MINIMAL > > > #define CONFIG_SPL_SERIAL_SUPPORT > > > #define CONFIG_SPL_NAND_SUPPORT > > > +#ifdef CONFIG_SPL_BUILD > > > #define CONFIG_SPL_NAND_MINIMAL > > > +#endif > > > #define CONFIG_SPL_FLUSH_IMAGE > > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND > > > diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h > > > index 8b13b10..5bdd44a 100644 > > > --- a/include/configs/P1022DS.h > > > +++ b/include/configs/P1022DS.h > > > @@ -41,7 +41,9 @@ > > > #define CONFIG_SPL_INIT_MINIMAL > > > #define CONFIG_SPL_SERIAL_SUPPORT > > > #define CONFIG_SPL_NAND_SUPPORT > > > +#ifdef CONFIG_SPL_BUILD > > > #define CONFIG_SPL_NAND_MINIMAL > > > +#endif > > > #define CONFIG_SPL_FLUSH_IMAGE > > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > > > diff --git a/include/configs/p1_p2_rdb_pc.h > > > b/include/configs/p1_p2_rdb_pc.h > > > index 7ed634b..bc48d62 100644 > > > --- a/include/configs/p1_p2_rdb_pc.h > > > +++ b/include/configs/p1_p2_rdb_pc.h > > > @@ -159,7 +159,9 @@ > > > #define CONFIG_SPL_INIT_MINIMAL > > > #define CONFIG_SPL_SERIAL_SUPPORT > > > #define CONFIG_SPL_NAND_SUPPORT > > > +#ifdef CONFIG_SPL_BUILD > > > #define CONFIG_SPL_NAND_MINIMAL > > > +#endif > > > #define CONFIG_SPL_FLUSH_IMAGE > > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > Are you sure this belongs in this patch? > > [Zhang Ying] > > Yes, it is necessary. Because CONFIG_SPL_NAND_MINIMAL has been used > > in the file law.c and tlb.c in this patch. > > What I mean is that it should probably have been done earlier, when > you > introduced CONFIG_SPL_NAND_MINIMAL. > [Zhang Ying] > I can understand you mean. Because the symbol > "CONFIG_SPL_NAND_MINIMAL" has been useless, I think no need to split > out a separate patch. OK, it looks like CONFIG_SPL_NAND_MINIMAL was there before your patchset. I think that was an accidental left-over and should have been removed (it was replaced with CONFIG_SPL_NAND_DRIVERS, _BASE, and _ECC). Could you describe what specifically you're using it to mean here? [Zhang Ying] CONFIG_SPL_NAND_MINIMAL is effective only for mpc85xx NAND SPL. If it is set, said the SPL as small as possible and the size is not more than 4K.
On 05/28/2013 09:11:17 PM, Zhang Ying-B40530 wrote: > > > -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, May 29, 2013 6:34 AM > To: Zhang Ying-B40530 > Cc: Wood Scott-B07421; u-boot@lists.denx.de; afleming@gmail.com; Xie > Xiaobo-R63061; Ilya Yanok > Subject: Re: [PATCH 5/6] spl: Make CONFIG_SPL_BUILD contain more > functionality > > On 05/22/2013 08:26:31 PM, Zhang Ying-B40530 wrote: > > > > > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Wednesday, May 22, 2013 11:46 PM > > To: Zhang Ying-B40530 > > Cc: Wood Scott-B07421; u-boot@lists.denx.de; afleming@gmail.com; Xie > > Xiaobo-R63061; Ilya Yanok > > Subject: Re: [PATCH 5/6] spl: Make CONFIG_SPL_BUILD contain more > > functionality > > > > On 05/21/2013 09:15:08 PM, Zhang Ying-B40530 wrote: > > > > diff --git a/include/configs/MPC8313ERDB.h > > > > b/include/configs/MPC8313ERDB.h > > > > index c28dfe0..a2bdcff 100644 > > > > --- a/include/configs/MPC8313ERDB.h > > > > +++ b/include/configs/MPC8313ERDB.h > > > > @@ -40,7 +40,9 @@ > > > > #define CONFIG_SPL_INIT_MINIMAL > > > > #define CONFIG_SPL_SERIAL_SUPPORT > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > +#ifdef CONFIG_SPL_BUILD > > > > #define CONFIG_SPL_NAND_MINIMAL > > > > +#endif > > > > #define CONFIG_SPL_FLUSH_IMAGE > > > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND > > > > diff --git a/include/configs/P1022DS.h > b/include/configs/P1022DS.h > > > > index 8b13b10..5bdd44a 100644 > > > > --- a/include/configs/P1022DS.h > > > > +++ b/include/configs/P1022DS.h > > > > @@ -41,7 +41,9 @@ > > > > #define CONFIG_SPL_INIT_MINIMAL > > > > #define CONFIG_SPL_SERIAL_SUPPORT > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > +#ifdef CONFIG_SPL_BUILD > > > > #define CONFIG_SPL_NAND_MINIMAL > > > > +#endif > > > > #define CONFIG_SPL_FLUSH_IMAGE > > > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > > > > > diff --git a/include/configs/p1_p2_rdb_pc.h > > > > b/include/configs/p1_p2_rdb_pc.h > > > > index 7ed634b..bc48d62 100644 > > > > --- a/include/configs/p1_p2_rdb_pc.h > > > > +++ b/include/configs/p1_p2_rdb_pc.h > > > > @@ -159,7 +159,9 @@ > > > > #define CONFIG_SPL_INIT_MINIMAL > > > > #define CONFIG_SPL_SERIAL_SUPPORT > > > > #define CONFIG_SPL_NAND_SUPPORT > > > > +#ifdef CONFIG_SPL_BUILD > > > > #define CONFIG_SPL_NAND_MINIMAL > > > > +#endif > > > > #define CONFIG_SPL_FLUSH_IMAGE > > > > #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" > > > > > > Are you sure this belongs in this patch? > > > [Zhang Ying] > > > Yes, it is necessary. Because CONFIG_SPL_NAND_MINIMAL has been > used > > > in the file law.c and tlb.c in this patch. > > > > What I mean is that it should probably have been done earlier, when > > you > > introduced CONFIG_SPL_NAND_MINIMAL. > > [Zhang Ying] > > I can understand you mean. Because the symbol > > "CONFIG_SPL_NAND_MINIMAL" has been useless, I think no need to split > > out a separate patch. > > OK, it looks like CONFIG_SPL_NAND_MINIMAL was there before your > patchset. I think that was an accidental left-over and should have > been removed (it was replaced with CONFIG_SPL_NAND_DRIVERS, _BASE, and > _ECC). > > Could you describe what specifically you're using it to mean here? > [Zhang Ying] > CONFIG_SPL_NAND_MINIMAL is effective only for mpc85xx NAND SPL. No, it's just a mistake that should be removed. It doesn't mean anything. There are other SPL targets that use minimal NAND drivers (e.g. mxc_nand_spl.c) that don't define this. -Scott
diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c index 0dff37f..a44a89c 100644 --- a/arch/powerpc/cpu/mpc85xx/tlb.c +++ b/arch/powerpc/cpu/mpc85xx/tlb.c @@ -55,7 +55,7 @@ void init_tlbs(void) return ; } -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_NAND_MINIMAL) void read_tlbcam_entry(int idx, u32 *valid, u32 *tsize, unsigned long *epn, phys_addr_t *rpn) { diff --git a/arch/powerpc/cpu/mpc8xxx/law.c b/arch/powerpc/cpu/mpc8xxx/law.c index 6f9d568..0df62b6 100644 --- a/arch/powerpc/cpu/mpc8xxx/law.c +++ b/arch/powerpc/cpu/mpc8xxx/law.c @@ -92,7 +92,7 @@ void disable_law(u8 idx) return; } -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_NAND_MINIMAL) static int get_law_entry(u8 i, struct law_entry *e) { u32 lawar; @@ -122,7 +122,7 @@ int set_next_law(phys_addr_t addr, enum law_size sz, enum law_trgt_if id) return idx; } -#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_BUILD) +#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SPL_NAND_MINIMAL) int set_last_law(phys_addr_t addr, enum law_size sz, enum law_trgt_if id) { u32 idx; diff --git a/common/env_common.c b/common/env_common.c index 906b41f..8cb81e9 100644 --- a/common/env_common.c +++ b/common/env_common.c @@ -156,7 +156,6 @@ int set_default_vars(int nvars, char * const vars[]) H_NOCLEAR | H_INTERACTIVE, nvars, vars); } -#ifndef CONFIG_SPL_BUILD /* * Check if CRC is valid and (if yes) import the environment. * Note that "buf" may or may not be aligned. @@ -188,7 +187,6 @@ int env_import(const char *buf, int check) return 0; } -#endif void env_relocate(void) { diff --git a/include/configs/MPC8313ERDB.h b/include/configs/MPC8313ERDB.h index c28dfe0..a2bdcff 100644 --- a/include/configs/MPC8313ERDB.h +++ b/include/configs/MPC8313ERDB.h @@ -40,7 +40,9 @@ #define CONFIG_SPL_INIT_MINIMAL #define CONFIG_SPL_SERIAL_SUPPORT #define CONFIG_SPL_NAND_SUPPORT +#ifdef CONFIG_SPL_BUILD #define CONFIG_SPL_NAND_MINIMAL +#endif #define CONFIG_SPL_FLUSH_IMAGE #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" #define CONFIG_SPL_MPC83XX_WAIT_FOR_NAND diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h index 8b13b10..5bdd44a 100644 --- a/include/configs/P1022DS.h +++ b/include/configs/P1022DS.h @@ -41,7 +41,9 @@ #define CONFIG_SPL_INIT_MINIMAL #define CONFIG_SPL_SERIAL_SUPPORT #define CONFIG_SPL_NAND_SUPPORT +#ifdef CONFIG_SPL_BUILD #define CONFIG_SPL_NAND_MINIMAL +#endif #define CONFIG_SPL_FLUSH_IMAGE #define CONFIG_SPL_TARGET "u-boot-with-spl.bin" diff --git a/include/configs/p1_p2_rdb_pc.h b/include/configs/p1_p2_rdb_pc.h index 7ed634b..bc48d62 100644 --- a/include/configs/p1_p2_rdb_pc.h +++ b/include/configs/p1_p2_rdb_pc.h @@ -159,7 +159,9 @@ #define CONFIG_SPL_INIT_MINIMAL #define CONFIG_SPL_SERIAL_SUPPORT #define CONFIG_SPL_NAND_SUPPORT +#ifdef CONFIG_SPL_BUILD #define CONFIG_SPL_NAND_MINIMAL +#endif #define CONFIG_SPL_FLUSH_IMAGE #define CONFIG_SPL_TARGET "u-boot-with-spl.bin"