ENV_IS_EMBEDDED vs CONFIG_ENV_IS_EMBEDDED
is there a reason to keep ENV_IS_EMBEDDED around ? i see a few places that use CONFIG_ENV_IS_EMBEDDED, but it seems to be largely useless at the moment as the define doesn't really do anything without ENV_IS_EMBEDDED.
in other words, i'd like to just do a sed on the tree to convert all ENV_IS_EMBEDDED instances to CONFIG_ENV_IS_EMBEDDED.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090614/6d387a0c/attachment-0001.pgp
Dear Mike Frysinger,
Quoted 4 lines · In message <[email protected]> you wrote:
is there a reason to keep
ENV_IS_EMBEDDEDaround ? i see a few places that useCONFIG_ENV_IS_EMBEDDED, but it seems to be largely useless at the moment as the define doesn't really do anything withoutENV_IS_EMBEDDED.
I'm not sure where CONFIG_ENV_IS_EMBEDDED (resp. CFG_ENV_IS_EMBEDDED) coming from. I guess from some comments ("short-cut compile-time test") that it was intended to enable using something like
COBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
Unfortunately it was not even documented in the README so we can only guess what it is supposed to mean :-( From original the software design point of view, CONFIG_ENV_IS_EMBEDDED is redundant at best, and usally just bogus, as the actual location of the environment
sectors (as determined by CONFIG_ENV_OFFSET or CONFIG_ENV_ADDR
combined with CONFIG_ENV_SECT_SIZE, plus eventually the definition of the corresponding "*_REDUND" settings) automatically determine if the environment is embedded or not - there is no need for such a variable.
Quoted 2 lines
in other words, i'd like to just do a sed on the tree to convert all
ENV_IS_EMBEDDEDinstances toCONFIG_ENV_IS_EMBEDDED.
That whould IMHO be wrong. We should rather check if there was a way to get rid of CONFIG_ENV_IS_EMBEDDED (which is probably hard because of it's use in the Makefile).
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Children are natural mimics who act like their parents despite every effort to teach them good manners.
Quoted 12 lines · On Monday 15 June 2009 05:02:42 Wolfgang Denk wrote:
Quoted 4 lines · Mike Frysinger
is there a reason to keep
ENV_IS_EMBEDDEDaround ? i see a few places that useCONFIG_ENV_IS_EMBEDDED, but it seems to be largely useless at the moment as the define doesn't really do anything withoutENV_IS_EMBEDDED.I'm not sure where
CONFIG_ENV_IS_EMBEDDED(resp.CFG_ENV_IS_EMBEDDED) coming from. I guess from some comments ("short-cut compile-time test") that it was intended to enable using something likeCOBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
Quoted 9 lines
Unfortunately it was not even documented in the README so we can only guess what it is supposed to mean :-( From original the software design point of view,
CONFIG_ENV_IS_EMBEDDEDis redundant at best, and usally just bogus, as the actual location of the environment
sectors (as determined byCONFIG_ENV_OFFSETorCONFIG_ENV_ADDR
combined withCONFIG_ENV_SECT_SIZE, plus eventually the definition of the corresponding "*_REDUND" settings) automatically determine if the environment is embedded or not - there is no need for such a variable.
i would actually prefer CONFIG_ENV_IS_EMBEDDED. otherwise we have to write the makefiles to account for all CONFIG_ENV_IS_IN_XXX variants. you can see the common/Makefile and tools/Makefile already suffer from this, and the recent patch i had to post to add missing config lines for some env sources.
so it would make life easier if these things bubbled up into just one config value -- easier for existing maintenance and easier for people adding new environment sources (like the newly added "mgdisk", whatever that is).
Quoted 6 lines
Quoted 2 lines
in other words, i'd like to just do a sed on the tree to convert all
ENV_IS_EMBEDDEDinstances toCONFIG_ENV_IS_EMBEDDED.That whould IMHO be wrong. We should rather check if there was a way to get rid of
CONFIG_ENV_IS_EMBEDDED(which is probably hard because of it's use in theMakefile).
well, is there a downside from moving from ENV_IS_EMBEDDED to CONFIG_ENV_IS_EMBEDDED ? we could at least make sure it is kept in sync in common headers by doing something like:
#if defined(CONFIG_ENV_IS_EMBEDDED) && !defined(ENV_IS_EMBEDDED)
# error you should not define CONFIG_ENV_IS_EMBEDDED yourself #endif #ifdef ENV_IS_EMBEDDED # define CONFIG_ENV_IS_EMBEDDED #endif -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20090619/474ade8c/attachment.pgp
Dear Mike Frysinger,
Quoted 3 lines · In message <[email protected]> you wrote:
well, is there a downside from moving from
ENV_IS_EMBEDDEDtoCONFIG_ENV_IS_EMBEDDED? we could at least make sure it is kept in sync in
Yes. It's IMHO silly to have to configure something manually (and keep it in sync with potential changes of some other defines) when it is as well possible to have that same value computed automatically at build time.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Why can you only have two doors on a chicken coop? If it had four it would be a chicken sedan.
Quoted 9 lines · On Friday 10 July 2009 18:17:26 Wolfgang Denk wrote:
Quoted 3 lines · Mike Frysinger
well, is there a downside from moving from
ENV_IS_EMBEDDEDtoCONFIG_ENV_IS_EMBEDDED? we could at least make sure it is kept in sync inYes. It's IMHO silly to have to configure something manually (and keep it in sync with potential changes of some other defines) when it is as well possible to have that same value computed automatically at build time.
so i should convert ENV_IS_EMBEDDED to CONFIG_ENV_IS_EMBEDDED or i should add to common code logic to force CONFIG_ENV_IS_EMBEDDED when ENV_IS_EMBEDDED is defined by the board ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090710/67e33eb9/attachment.pgp
Dear Mike Frysinger,
Quoted 9 lines · In message <[email protected]> you wrote:
Quoted 5 lines
Yes. It's IMHO silly to have to configure something manually (and keep it in sync with potential changes of some other defines) when it is as well possible to have that same value computed automatically at build time.
so i should convert
ENV_IS_EMBEDDEDtoCONFIG_ENV_IS_EMBEDDEDor i should add to common code logic to forceCONFIG_ENV_IS_EMBEDDEDwhenENV_IS_EMBEDDEDis defined by the board ?
If it's possible, generating CONFIG_ENV_IS_EMBEDDED whenENV_IS_EMBEDDED is defined would be much better - however, my understanding is that CONFIG_ENV_IS_EMBEDDED is needed by the make
proces,,i. e. before actually running any commands, whileENV_IS_EMBEDDED gets computed by the C preprocessor, i. e. too late for make decisions (unless you copy the part of code that computes ENV_IS_EMBEDDED into some special script / file and run it through the C preprocessor - which is not exactly a leaner design either, it seems).
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Cigarette, n.: A fire at one end, a fool at the other, and a bit of tobacco in between.
Quoted 10 lines · On Saturday 11 July 2009 03:47:28 Wolfgang Denk wrote:
Quoted 8 lines · Mike Frysinger
Quoted 4 lines
Yes. It's IMHO silly to have to configure something manually (and keep it in sync with potential changes of some other defines) when it is as well possible to have that same value computed automatically at build time.
so i should convert
ENV_IS_EMBEDDEDtoCONFIG_ENV_IS_EMBEDDEDor i should add to common code logic to forceCONFIG_ENV_IS_EMBEDDEDwhenENV_IS_EMBEDDEDis defined by the board ?
Quoted 9 lines
If it's possible, generating
CONFIG_ENV_IS_EMBEDDEDwhenENV_IS_EMBEDDEDis defined would be much better - however, my understanding is thatCONFIG_ENV_IS_EMBEDDEDis needed by the make
proces,,i. e. before actually running any commands, whileENV_IS_EMBEDDEDgets computed by the C preprocessor, i. e. too late for make decisions (unless you copy the part of code that computesENV_IS_EMBEDDEDinto some special script / file and run it through the C preprocessor - which is not exactly a leaner design either, it seems).
well, automating for NOR flash and preventing future bleed is easy to do. but there doesnt seem to be a way for automating for nand and onenand types, so board people would still need to opt-in manually for their boards. how does the attached patch look ? i cant test any of these non-Blackfin boards, but the logic should be straight forward ...
i guess part of the problem is that the meaning of ENV_IS_EMBEDDED has changed over time. going by environment.h, it would seem that the original point was that the environment is embedded in the runtime address map (between monitor_base and monitor_base+monitor_len), but it has since changed to mean it is embedded in the u-boot blob as stored in flash. but for eeprom, nand, nvram, and onenand, it means the environment is in a sector that is in the middle of the u-boot image stored in flash.
i dont know how you want to document this define, but if you want to revert to the original meaning (embedded at runtime), then i dont think it'll be hard for the other types to convert. after all, this should just be a linker script issue (punch a hole in the map for the environment).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-unify-CONFIG_-ENV_IS_EMBEDDED.patch
Type: text/x-patch
Size: 9014 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090724/ab15eb94/attachment.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090724/ab15eb94/attachment.pgp
Wolfgang Denk
AppliedDear Mike Frysinger,
Quoted 20 lines · In message <[email protected]> you wrote:
From
8ea84aa414e204c3259861da80d11ff7cc9fded7Mon Sep 17 00:00:00 2001
From: Mike Frysinger <[email protected]>
Date: Fri, 24 Jul 2009 15:40:52 -0400
Subject: [PATCH] unify {CONFIG_,}ENV_IS_EMBEDDEDSome boards have fallen out of sync by defining
CONFIG_ENV_IS_EMBEDDEDmanually. While it is useful to have this available to the build system, let's do it automatically rather than forcing people to opt into it.Signed-off-by Mike Frysinger <[email protected]>--- include/configs/EB+MCF-EV123.h | 1 - include/configs/M52277EVB.h | 1 - include/configs/M5235EVB.h | 1 - include/configs/M5272C3.h | 1 - include/configs/M5275EVB.h | 1 - include/configs/M5329EVB.h | 1 - include/configs/M5373EVB.h | 1 - include/configs/M54451EVB.h | 1 -
Quoted 8 lines
include/configs/M54455EVB.h | 1 - include/configs/M5475EVB.h | 1 - include/configs/M5485EVB.h | 1 - include/configs/OXC.h | 1 - include/configs/cobra5272.h | 1 - include/configs/pcu_e.h | 1 - include/environment.h | 33 +++++++++++++++++++++++++++------ 15 files changed, 27 insertions(+), 20 deletions(-)
Applied to "next" branch, thanks.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de If something is different, it's either better or worse, and usually both. - Larry Wall
PATCH 1/1 Fix issues introduced by 16fc32f: unify {CONFIG_, }ENV_IS_EMBEDDED
Commit 16fc32f introduced an error in include/environment.h, which makes u-boot fail to compile due to a missing #endif.
This also fixes a merge conflict remaining in include/configs/EB+MCF-EV123.h which was caused by the same commit
--- include/configs/EB+MCF-EV123.h | 7 ------- include/environment.h | 1 + 2 files changed, 1 insertions(+), 7 deletions(-)
include/configs/EB+MCF-EV123.h +0 -7
@@ -63,13 +63,6 @@#define CONFIG_ENV_ADDR 0xF003C000/* End of 256K */ #define CONFIG_ENV_SECT_SIZE 0x4000#define CONFIG_ENV_IS_IN_FLASH 1 -<<<<<<< HEAD:include/configs/EB+MCF-EV123.h -======= -/* -#define CONFIG_ENV_ADDR_REDUND 0xF0018000 -#define CONFIG_ENV_SECT_SIZE_REDUND 0x4000 -*/ ->>>>>>> unify {CONFIG_,}ENV_IS_EMBEDDED:include/configs/EB+MCF-EV123.h#else #define CONFIG_ENV_ADDR 0xFFE04000#define CONFIG_ENV_SECT_SIZE 0x2000
include/environment.h +1 -0
@@ -100,6 +100,7 @@ !defined (CONFIG_ENV_IS_IN_NAND ) && \ !defined(CONFIG_ENV_IS_IN_ONENAND)# error "CONFIG_ENV_IS_EMBEDDED not supported for your flash type" +# endif #endif /*
Signature · 1 line
1.6.0.4
PATCH 1/1 Fix issues introduced by 16fc32f: unify {CONFIG_, }ENV_IS_EMBEDDED
Quoted 4 lines · On Tuesday 25 August 2009 04:46:07 Albin Tonnerre wrote:
Commit 16fc32f introduced an error in
include/environment.h, which makes u-boot fail to compile due to a missing #endif.
This also fixes a merge conflict remaining ininclude/configs/EB+MCF-EV123.hwhich was caused by the same commit
this is only an issue for the "next" branch ...
my local git tree has the right number of #endif's, so i guess i must have sent out the wrong patch
guess this needs to get squashed into the relevant change rather than added as a new commit. thanks for the update.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090825/debab938/attachment.pgp
PATCH 1/1 Fix issues introduced by 16fc32f: unify {CONFIG_, }ENV_IS_EMBEDDED
Quoted 6 lines · Mike Frysinger
Quoted 4 lines · On Tuesday 25 August 2009 04:46:07 Albin Tonnerre wrote:
Commit 16fc32f introduced an error in
include/environment.h, which makes u-boot fail to compile due to a missing #endif.
This also fixes a merge conflict remaining ininclude/configs/EB+MCF-EV123.hwhich was caused by the same commit
Quoted 7 lines
this is only an issue for the "next" branch ...
my local git tree has the right number of #endif's, so i guess i must have sent out the wrong patch
guess this needs to get squashed into the relevant change rather than added as a new commit. thanks for the update.
How exactly would you do that? The commit actually got pushed, and AFAIK next doesn't get rebased
Regards,
Signature · 11 lines
Albin Tonnerre, Free Electrons Kernel, drivers and embedded Linux development, consulting, training and support. http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: Digital signature Url : http://lists.denx.de/pipermail/u-boot/attachments/20090825/dd1f719c/attachment.pgp
PATCH 1/1 Fix issues introduced by 16fc32f: unify {CONFIG_, }ENV_IS_EMBEDDED
Dear Albin Tonnerre,
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines
guess this needs to get squashed into the relevant change rather than added as a new commit. thanks for the update.
How exactly would you do that? The commit actually got pushed, and AFAIK next doesn't get rebased
It gets rebased when there is a need to do so. This seems to be such a case.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de 1 1 was a race-horse, 2 2 was 1 2. When 1 1 1 1 race, 2 2 1 1 2.
Wolfgang Denk
AppliedPATCH 1/1 Fix issues introduced by 16fc32f: unify {CONFIG_, }ENV_IS_EMBEDDED
In reply to Albin TonnerreDear Albin Tonnerre,
Quoted 10 lines · In message <1251189967-11702-1-git-send-email-albin.tonnerre@free-electrons.com…
Commit 16fc32f introduced an error in
include/environment.h, which makes u-boot fail to compile due to a missing #endif.
This also fixes a merge conflict remaining ininclude/configs/EB+MCF-EV123.hwhich was caused by the same commitSigned-off-by Albin Tonnerre <[email protected]>--- include/configs/EB+MCF-EV123.h | 7 ------- include/environment.h | 1 + 2 files changed, 1 insertions(+), 7 deletions(-)
Applied to "next" branch (and rebased "next" to clean up the mess).
Thanks for catching / fixing this.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de You don't have to worry about me. I might have been born yesterday... but I stayed up all night.