| Message ID | 1322467058-30532-1-git-send-email-grinberg@compulab.co.il |
|---|---|
| State | Accepted |
| Delegated to: | Wolfgang Denk |
| Headers | show |
Dear Igor Grinberg, In message <1322467058-30532-1-git-send-email-grinberg@compulab.co.il> you wrote: > When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, > the environment.h file does not get included resulting in unrecognized > env_t type. > Fix this by moving the include directive. Hm... the fix seems wrong to me. What happens now if ENV_IS_EMBEDDED is set, but CONFIG_BUILD_ENVCRC is _not_ set? Did this change pass a MAKEALL for example for ppc? Best regards, Wolfgang Denk
Dear Igor Grinberg, In message <1322467058-30532-1-git-send-email-grinberg@compulab.co.il> you wrote: > When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, > the environment.h file does not get included resulting in unrecognized > env_t type. > Fix this by moving the include directive. ... > -extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); ... > +extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); Also, instead of moving this line, you should rather insert something like "#include <u-boot/crc.h>" ... Best regards, Wolfgang Denk
Hi Wolfgang, On 11/28/11 10:18, Wolfgang Denk wrote: > Dear Igor Grinberg, > > In message <1322467058-30532-1-git-send-email-grinberg@compulab.co.il> you wrote: >> When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, >> the environment.h file does not get included resulting in unrecognized >> env_t type. >> Fix this by moving the include directive. > > ... >> -extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); > ... >> +extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); > > Also, instead of moving this line, you should rather insert something > like "#include <u-boot/crc.h>" ... I had no intend to clean this file, but I can do this change in a separate patch.
Hi Wolfgang, On 11/28/11 10:07, Wolfgang Denk wrote: > Dear Igor Grinberg, > > In message <1322467058-30532-1-git-send-email-grinberg@compulab.co.il> you wrote: >> When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, >> the environment.h file does not get included resulting in unrecognized >> env_t type. >> Fix this by moving the include directive. > > Hm... the fix seems wrong to me. What happens now if ENV_IS_EMBEDDED > is set, but CONFIG_BUILD_ENVCRC is _not_ set? Well, you should look into that file... it says: #if defined(ENV_IS_EMBEDDED) && !defined(CONFIG_BUILD_ENVCRC) # define CONFIG_BUILD_ENVCRC 1 #endif So this will not be a problem. As for the logic of it... I don't know what was the original intend. > > Did this change pass a MAKEALL for example for ppc? I don't have the tool chain for ppc. I think the original fix has been tested by Stefano. Is there a requirement to have all the supported architectures tool chains, for submitting patches? Stefano, Mike, can you, please, test it on PPC and Blackfin? Thanks
Dear Igor Grinberg, In message <4ED34E9B.7040200@compulab.co.il> you wrote: > > > Hm... the fix seems wrong to me. What happens now if ENV_IS_EMBEDDED > > is set, but CONFIG_BUILD_ENVCRC is _not_ set? > > Well, you should look into that file... It's faster for me to ask you to do that :-) > > Did this change pass a MAKEALL for example for ppc? > > I don't have the tool chain for ppc. > I think the original fix has been tested by Stefano. > Is there a requirement to have all the supported > architectures tool chains, for submitting patches? Not for all, but http://www.denx.de/wiki/U-Boot/Patches says "Please also run MAKEALL for _at_least_one_other_architecture_ than the one you made your modifications in." Best regards, Wolfgang Denk
On Mon, Nov 28, 2011 at 04:04, Igor Grinberg wrote:
> I don't have the tool chain for ppc.
http://dev.gentoo.org/~vapier/u-boot/
-mike
On Monday 28 November 2011 02:57:38 Igor Grinberg wrote: > When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, > the environment.h file does not get included resulting in unrecognized > env_t type. > Fix this by moving the include directive. seems to work for me now; thanks! Tested-by: Mike Frysinger <vapier@gentoo.org> -mike
On 11/28/11 20:52, Mike Frysinger wrote: > On Mon, Nov 28, 2011 at 04:04, Igor Grinberg wrote: >> I don't have the tool chain for ppc. > > http://dev.gentoo.org/~vapier/u-boot/ 10x Mike
On 11/28/11 17:15, Wolfgang Denk wrote: > Dear Igor Grinberg, > > In message <4ED34E9B.7040200@compulab.co.il> you wrote: >> >>> Hm... the fix seems wrong to me. What happens now if ENV_IS_EMBEDDED >>> is set, but CONFIG_BUILD_ENVCRC is _not_ set? >> >> Well, you should look into that file... > > It's faster for me to ask you to do that :-) I have already done that (before sending the patch) as soon as I saw the email from Mike, reporting the problem. > >>> Did this change pass a MAKEALL for example for ppc? >> >> I don't have the tool chain for ppc. >> I think the original fix has been tested by Stefano. >> Is there a requirement to have all the supported >> architectures tool chains, for submitting patches? > > Not for all, but http://www.denx.de/wiki/U-Boot/Patches says "Please > also run MAKEALL for _at_least_one_other_architecture_ than the one you > made your modifications in." 10x for looking into that for me ;-)
On 11/29/11 06:25, Mike Frysinger wrote: > On Monday 28 November 2011 02:57:38 Igor Grinberg wrote: >> When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, >> the environment.h file does not get included resulting in unrecognized >> env_t type. >> Fix this by moving the include directive. > > seems to work for me now; thanks! > Tested-by: Mike Frysinger <vapier@gentoo.org> 10x Mike - it is for Blackfin, right? Wolfgang, can you apply the fix now?
ping! This fixes a compile breakage and IMO should be applied before 2011.12 is out. Can someone, please apply it? On 11/28/11 09:57, Igor Grinberg wrote: > When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, > the environment.h file does not get included resulting in unrecognized > env_t type. > Fix this by moving the include directive. > > Reported-by: Mike Frysinger <vapier@gentoo.org> > Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > --- > tools/envcrc.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/envcrc.c b/tools/envcrc.c > index 111d9f6..51e3f54 100644 > --- a/tools/envcrc.c > +++ b/tools/envcrc.c > @@ -61,7 +61,6 @@ > #endif /* CONFIG_ENV_IS_IN_FLASH */ > > #if defined(ENV_IS_EMBEDDED) && !defined(CONFIG_BUILD_ENVCRC) > -# include <environment.h> > # define CONFIG_BUILD_ENVCRC 1 > #endif > > @@ -74,13 +73,14 @@ > #define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE) > > > -extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); > - > #ifdef CONFIG_BUILD_ENVCRC > +# include <environment.h> > extern unsigned int env_size; > extern env_t environment; > #endif /* CONFIG_BUILD_ENVCRC */ > > +extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); > + > int main (int argc, char **argv) > { > #ifdef CONFIG_BUILD_ENVCRC
Dear Igor Grinberg, In message <1322467058-30532-1-git-send-email-grinberg@compulab.co.il> you wrote: > When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, > the environment.h file does not get included resulting in unrecognized > env_t type. > Fix this by moving the include directive. > > Reported-by: Mike Frysinger <vapier@gentoo.org> > Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > --- > tools/envcrc.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) Applied, thanks. Best regards, Wolfgang Denk
diff --git a/tools/envcrc.c b/tools/envcrc.c index 111d9f6..51e3f54 100644 --- a/tools/envcrc.c +++ b/tools/envcrc.c @@ -61,7 +61,6 @@ #endif /* CONFIG_ENV_IS_IN_FLASH */ #if defined(ENV_IS_EMBEDDED) && !defined(CONFIG_BUILD_ENVCRC) -# include <environment.h> # define CONFIG_BUILD_ENVCRC 1 #endif @@ -74,13 +73,14 @@ #define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE) -extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); - #ifdef CONFIG_BUILD_ENVCRC +# include <environment.h> extern unsigned int env_size; extern env_t environment; #endif /* CONFIG_BUILD_ENVCRC */ +extern uint32_t crc32 (uint32_t, const unsigned char *, unsigned int); + int main (int argc, char **argv) { #ifdef CONFIG_BUILD_ENVCRC
When ENV_IS_EMBEDDED is not set, but CONFIG_BUILD_ENVCRC is set, the environment.h file does not get included resulting in unrecognized env_t type. Fix this by moving the include directive. Reported-by: Mike Frysinger <vapier@gentoo.org> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> --- tools/envcrc.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)