Patchwork [U-Boot,v2] mmc: fix env in mmc with redundant compile error

login
register
mail settings
Submitter Bo Shen
Date May 15, 2013, 1:38 a.m.
Message ID <1368581897-11261-1-git-send-email-voice.shen@atmel.com>
Download mbox | patch
Permalink /patch/243857/
State Accepted, archived
Delegated to: Andy Fleming
Headers show

Comments

Bo Shen - May 15, 2013, 1:38 a.m.
The commit d196bd8 (env_mmc: add support for redundant environment)
introduce the following compile error when enable redundant
environment support with MMC
---8<---
env_mmc.c:149: error: 'env_t' has no member named 'flags'
env_mmc.c:248: error: 'env_t' has no member named 'flags'
env_mmc.c:248: error: 'env_t' has no member named 'flags'
env_mmc.c:250: error: 'env_t' has no member named 'flags'
env_mmc.c:250: error: 'env_t' has no member named 'flags'
env_mmc.c:252: error: 'env_t' has no member named 'flags'
env_mmc.c:252: error: 'env_t' has no member named 'flags'
env_mmc.c:254: error: 'env_t' has no member named 'flags'
env_mmc.c:254: error: 'env_t' has no member named 'flags'
env_mmc.c:267: error: 'env_t' has no member named 'flags'
make[1]: *** [env_mmc.o] Error 1
--->8---

Add this patch to fix it

Signed-off-by: Bo Shen <voice.shen@atmel.com>
Reviewed-by: Michael Heimpold <mhei@heimpold.de>
---
Changes in v2:
  - s/reduandant/redundant in commit message

 include/environment.h |    6 ++++++
 1 file changed, 6 insertions(+)
Andy Fleming - May 15, 2013, 10:31 p.m.
On Tue, May 14, 2013 at 8:38 PM, Bo Shen <voice.shen@atmel.com> wrote:

> The commit d196bd8 (env_mmc: add support for redundant environment)
> introduce the following compile error when enable redundant
> environment support with MMC
> ---8<---
> env_mmc.c:149: error: 'env_t' has no member named 'flags'
> env_mmc.c:248: error: 'env_t' has no member named 'flags'
> env_mmc.c:248: error: 'env_t' has no member named 'flags'
> env_mmc.c:250: error: 'env_t' has no member named 'flags'
> env_mmc.c:250: error: 'env_t' has no member named 'flags'
> env_mmc.c:252: error: 'env_t' has no member named 'flags'
> env_mmc.c:252: error: 'env_t' has no member named 'flags'
> env_mmc.c:254: error: 'env_t' has no member named 'flags'
> env_mmc.c:254: error: 'env_t' has no member named 'flags'
> env_mmc.c:267: error: 'env_t' has no member named 'flags'
> make[1]: *** [env_mmc.o] Error 1
> --->8---
>
> Add this patch to fix it
>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> Reviewed-by: Michael Heimpold <mhei@heimpold.de>
> ---
> Changes in v2:
>   - s/reduandant/redundant in commit message
>
>  include/environment.h |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/environment.h b/include/environment.h
> index 4c6a37b..460ccb4 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -75,6 +75,12 @@
>  # endif
>  #endif /* CONFIG_ENV_IS_IN_FLASH */
>
> +#if defined(CONFIG_ENV_IS_IN_MMC)
> +# ifdef CONFIG_ENV_OFFSET_REDUND
> +#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
> +# endif
> +#endif
> +
>


If CONFIG_SYS_REDUNDAND_ENVIRONMENT must be defined when
CONFIG_ENV_OFFSET_REDUND is defined, then isn't it, itself, redundant?

Further, if defining CONFIG_ENV_OFFSET_REDUND is enough to make env_mmc.c
access a variable that is protected by CONFIG_SYS_REDUNDAND_ENVIRONMENT,
then doesn't that mean that those accesses are improperly protected (or
that the "flags" field is)?

It looks like we keep patching over this problem, rather than fixing it....

Andy

PS - Is there a reason it's REDUNDAND, and not REDUNDANT?
Tom Rini - May 15, 2013, 11:48 p.m.
On Wed, May 15, 2013 at 05:31:30PM -0500, Andy Fleming wrote:

> On Tue, May 14, 2013 at 8:38 PM, Bo Shen <voice.shen@atmel.com> wrote:
> 
> > The commit d196bd8 (env_mmc: add support for redundant environment)
> > introduce the following compile error when enable redundant
> > environment support with MMC
> > ---8<---
> > env_mmc.c:149: error: 'env_t' has no member named 'flags'
> > env_mmc.c:248: error: 'env_t' has no member named 'flags'
> > env_mmc.c:248: error: 'env_t' has no member named 'flags'
> > env_mmc.c:250: error: 'env_t' has no member named 'flags'
> > env_mmc.c:250: error: 'env_t' has no member named 'flags'
> > env_mmc.c:252: error: 'env_t' has no member named 'flags'
> > env_mmc.c:252: error: 'env_t' has no member named 'flags'
> > env_mmc.c:254: error: 'env_t' has no member named 'flags'
> > env_mmc.c:254: error: 'env_t' has no member named 'flags'
> > env_mmc.c:267: error: 'env_t' has no member named 'flags'
> > make[1]: *** [env_mmc.o] Error 1
> > --->8---
> >
> > Add this patch to fix it
> >
> > Signed-off-by: Bo Shen <voice.shen@atmel.com>
> > Reviewed-by: Michael Heimpold <mhei@heimpold.de>
> > ---
> > Changes in v2:
> >   - s/reduandant/redundant in commit message
> >
> >  include/environment.h |    6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/include/environment.h b/include/environment.h
> > index 4c6a37b..460ccb4 100644
> > --- a/include/environment.h
> > +++ b/include/environment.h
> > @@ -75,6 +75,12 @@
> >  # endif
> >  #endif /* CONFIG_ENV_IS_IN_FLASH */
> >
> > +#if defined(CONFIG_ENV_IS_IN_MMC)
> > +# ifdef CONFIG_ENV_OFFSET_REDUND
> > +#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
> > +# endif
> > +#endif
> > +
> >
> 
> 
> If CONFIG_SYS_REDUNDAND_ENVIRONMENT must be defined when
> CONFIG_ENV_OFFSET_REDUND is defined, then isn't it, itself, redundant?
> 
> Further, if defining CONFIG_ENV_OFFSET_REDUND is enough to make env_mmc.c
> access a variable that is protected by CONFIG_SYS_REDUNDAND_ENVIRONMENT,
> then doesn't that mean that those accesses are improperly protected (or
> that the "flags" field is)?
> 
> It looks like we keep patching over this problem, rather than fixing it....
> 
> Andy
> 
> PS - Is there a reason it's REDUNDAND, and not REDUNDANT?

The answer for all of the above is, history.  A problem is that we need
CONFIG_SYS_REDUNDAND_ENVIRONMENT if any of:
- CONFIG_ENV_OFFSET_REDUND
- CONFIG_ENV_ADDR_REDUND
- CONFIG_ENV_UBI_VOLUME_REDUND

are set.  And the config variable is used in both common/env_ubi.c
(which would be easy to switch to UBI_VOLUME_REDUND) and tools/envcrc.c,
which would be harder.  And I don't think we can just whack flags as
being always on as that would invalidate peoples environment, from a
quick peek at things.
Andy Fleming - June 13, 2013, 10:36 p.m.
On Wed, May 15, 2013 at 09:38:16AM +0800, Bo Shen wrote:
> The commit d196bd8 (env_mmc: add support for redundant environment)
> introduce the following compile error when enable redundant
> environment support with MMC
> ---8<---
> env_mmc.c:149: error: 'env_t' has no member named 'flags'
> env_mmc.c:248: error: 'env_t' has no member named 'flags'
> env_mmc.c:248: error: 'env_t' has no member named 'flags'
> env_mmc.c:250: error: 'env_t' has no member named 'flags'
> env_mmc.c:250: error: 'env_t' has no member named 'flags'
> env_mmc.c:252: error: 'env_t' has no member named 'flags'
> env_mmc.c:252: error: 'env_t' has no member named 'flags'
> env_mmc.c:254: error: 'env_t' has no member named 'flags'
> env_mmc.c:254: error: 'env_t' has no member named 'flags'
> env_mmc.c:267: error: 'env_t' has no member named 'flags'
> make[1]: *** [env_mmc.o] Error 1
> --->8---
> 
> Add this patch to fix it
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> Reviewed-by: Michael Heimpold <mhei@heimpold.de>

Applied, thanks.

Andy

Patch

diff --git a/include/environment.h b/include/environment.h
index 4c6a37b..460ccb4 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -75,6 +75,12 @@ 
 # endif
 #endif	/* CONFIG_ENV_IS_IN_FLASH */
 
+#if defined(CONFIG_ENV_IS_IN_MMC)
+# ifdef CONFIG_ENV_OFFSET_REDUND
+#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
+# endif
+#endif
+
 #if defined(CONFIG_ENV_IS_IN_NAND)
 # if defined(CONFIG_ENV_OFFSET_OOB)
 #  ifdef CONFIG_ENV_OFFSET_REDUND