| Message ID | 1289896880-15792-1-git-send-email-r64343@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
On 11/16/2010 09:41 AM, Jason Liu wrote: Hi Jason, > fix saveenv or env save command not work on mx51evk board. > with this patch, we can use savenv or env save to > store enviroments to mmc card slot 0 > > -#define CONFIG_ENV_SECT_SIZE (128 * 1024) > -#define CONFIG_ENV_SIZE CONFIG_ENV_SECT_SIZE > -#define CONFIG_ENV_IS_NOWHERE > +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) > +#define CONFIG_ENV_SIZE (2 * 64 * 1024) > +#define CONFIG_ENV_IS_IN_MMC > +#define CONFIG_SYS_MMC_ENV_DEV 0 Why do we need 128KB to save the environment in case of MMC ? It seems to me too much, because we are not constrained to the sector size as for flash devices. Can we save some time by saving or reading the environment reducing its size ? Or do you plan to save something more in this area ? Best regards, Stefano Babic
2010/11/17 Stefano Babic <sbabic@denx.de>: > On 11/16/2010 09:41 AM, Jason Liu wrote: > > Hi Jason, > >> fix saveenv or env save command not work on mx51evk board. >> with this patch, we can use savenv or env save to >> store enviroments to mmc card slot 0 > >> >> -#define CONFIG_ENV_SECT_SIZE (128 * 1024) >> -#define CONFIG_ENV_SIZE CONFIG_ENV_SECT_SIZE >> -#define CONFIG_ENV_IS_NOWHERE >> +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) >> +#define CONFIG_ENV_SIZE (2 * 64 * 1024) >> +#define CONFIG_ENV_IS_IN_MMC >> +#define CONFIG_SYS_MMC_ENV_DEV 0 > > Why do we need 128KB to save the environment in case of MMC ? It seems > to me too much, because we are not constrained to the sector size as for > flash devices. I set it according to the following reason, - Keep the same setting as the original when you commit the mx51 support patch. Why you select 128KB? :) - As I looked through other platform such as OMAP4 for MMC ENV setting, it's also set for 128KB - Leave much room for the user to store customer env. > > Can we save some time by saving or reading the environment reducing its > size ? Or do you plan to save something more in this area ? It's always the trade-off, set to 512B or less will save some time according to 128KB, but it will face much risk to change the code frequently to meet the increasing env size requirement. what's the size do you think is suitable? > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de > ===================================================================== > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
Dear Jason Liu, In message <AANLkTikB6NYTdz=4cMr_RDuhVXQjTa9zUKaed+vW4C+s@mail.gmail.com> you wrote: > > I set it according to the following reason, > > - Keep the same setting as the original when you commit the mx51 > support patch. Why you select 128KB? :) Copying settings without reflecting if they make sense for your own system is not really a good idea. > - As I looked through other platform such as OMAP4 for MMC ENV > setting, it's also set for 128KB Ditto. Please keep in mind that the environment is CRC protected; that means the larger the environment area, the more time will be needed for the CRC calculation when the system boots. And quick booting is definitely a topic these days, so please reconsider. > - Leave much room for the user to store customer env. Come on. Do you know how much text can be saved in 128 KiB? I have seen some really kinky environment settings before, where a "printenv" would scroll trough thousands of lines (well, hundrets at least) so it was next to impossible to find anything, and they needed something around 10 KiB or so. 16 KiB is more than enough for 99.9% of the users. > > Can we save some time by saving or reading the environment reducing its > > size ? Or do you plan to save something more in this area ? > > It's always the trade-off, set to 512B or less will save some time > according to 128KB, but it will face much risk to change the code > frequently to meet the increasing env size requirement. > what's the size do you think is suitable? 16 KiB. Best regards, Wolfgang Denk
2010/11/17 Wolfgang Denk <wd@denx.de>: > Dear Jason Liu, > > In message <AANLkTikB6NYTdz=4cMr_RDuhVXQjTa9zUKaed+vW4C+s@mail.gmail.com> you wrote: >> >> I set it according to the following reason, >> >> - Keep the same setting as the original when you commit the mx51 >> support patch. Why you select 128KB? :) > > Copying settings without reflecting if they make sense for your own > system is not really a good idea. Agree. But I think the original commit for 128KiB env size has been reviewed on the mail list and no one against it. So, I keep the same setting. If I change it, some guys main complain why it change, it will make it incompatible with the original env setting and make the customer env lost. > >> - As I looked through other platform such as OMAP4 for MMC ENV >> setting, it's also set for 128KB > > Ditto. Please keep in mind that the environment is CRC protected; > that means the larger the environment area, the more time will be > needed for the CRC calculation when the system boots. And quick > booting is definitely a topic these days, so please reconsider. OK, > >> - Leave much room for the user to store customer env. > > Come on. Do you know how much text can be saved in 128 KiB? I have > seen some really kinky environment settings before, where a "printenv" > would scroll trough thousands of lines (well, hundrets at least) so it > was next to impossible to find anything, and they needed something > around 10 KiB or so. 16 KiB is more than enough for 99.9% of the > users. We really don't know what data that customer will store. So leave much room for them is my first though, But consider the fast boot, what you said make sense. > >> > Can we save some time by saving or reading the environment reducing its >> > size ? Or do you plan to save something more in this area ? >> >> It's always the trade-off, set to 512B or less will save some time >> according to 128KB, but it will face much risk to change the code >> frequently to meet the increasing env size requirement. >> what's the size do you think is suitable? > > 16 KiB. OK, do you think we need change all the platform to reflect the 16K env size for MMC case? > > Best regards, > > Wolfgang Denk > > -- > 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@denx.de > "Consistency requires you to be as ignorant today as you were a year > ago." - Bernard Berenson >
Dear Jason Liu, In message <AANLkTim4k-PnuFDgWvYj3TY4mtLcz7Uzy1-TO9vExxbS@mail.gmail.com> you wrote: > > Agree. But I think the original commit for 128KiB env size has been > reviewed on the mail list and no one against it. Nobody claims 100.0% coverage in a review. Sometimes such things slip through, some times they are detected. > So, I keep the same setting. If I change it, some guys main complain > why it change, it will make it > incompatible with the original env setting and make the customer env lost. I doubt that. I think I have seen a fair share of different environment settings. There are many, many in the 1...2 KiB range. Very, very few exceed 4 KiB. You have to search really hard to find one that gets close to 8KiB. > OK, do you think we need change all the platform to reflect the 16K > env size for MMC case? Define "need". There is no urgent need - it's not a bug, the code is working, though inefficiently. It's an optimization thingy. Best regards, Wolfgang Denk
On 11/17/2010 02:44 AM, Jason Liu wrote: > > I set it according to the following reason, > > - Keep the same setting as the original when you commit the mx51 > support patch. Why you select 128KB? :) No idea, I cannot remember, probably I missed the point. When I commit the mx51 stuff, there is no possibility to store the environment, so there are no drawbacks, but I commit some dead code. > - As I looked through other platform such as OMAP4 for MMC ENV > setting, it's also set for 128KB Well, probably the value was copied even in from a flash setup. In case of flash (NAND or NOR) we must reserved at least 1 sector to store the environment, and because nowadays flash chips are bigger as in the past it is very common that the sector size is 128KB or even larger. However, this does not apply to MMC. A large environment space when we do not need requires more time to save and to read, last thing has consequences on the boot time. > - Leave much room for the user to store customer env. I do not think there someone needing 128KB to store the environment.... > It's always the trade-off, set to 512B or less will save some time > according to 128KB, but it will face much risk to change the code > frequently to meet the increasing env size requirement. > what's the size do you think is suitable? I have expected a value such as 8-16KB. Best regards, Stefano Babic
On 11/17/2010 08:29 AM, Jason Liu wrote: > Agree. But I think the original commit for 128KiB env size has been > reviewed on the mail list and no one against it. This does not mean that we cannot change this value when we find it is wrong.... And as I said previously, the value had no effect because CONFIG_ENV_NOWHERE was set. > So, I keep the same setting. If I change it, some guys main complain > why it change, There is not at the moment the possibility to store the environment in the mainline for the mx51evk, nobody complains about a feature that does not exist.... > We really don't know what data that customer will store. So leave much > room for them is > my first though, But consider the fast boot, what you said make sense. If someone really needs a so large environment, cand send an e-mail to this ML explaining his reason and posting a patch. It will be discussed here. > OK, do you think we need change all the platform to reflect the 16K > env size for MMC case? Your patch refers to the mx51evk. So change the value according to this discussion and post your patch again for inclusion in mainline. We will take into account for other boards when new patches will be submitted. Best regards, Stefano Babic
2010/11/17 Stefano Babic <sbabic@denx.de>: > On 11/17/2010 08:29 AM, Jason Liu wrote: >> Agree. But I think the original commit for 128KiB env size has been >> reviewed on the mail list and no one against it. > > This does not mean that we cannot change this value when we find it is > wrong.... As Wofgang said, it's not wrong. Just not optimization. > >> We really don't know what data that customer will store. So leave much >> room for them is >> my first though, But consider the fast boot, what you said make sense. > > If someone really needs a so large environment, cand send an e-mail to > this ML explaining his reason and posting a patch. It will be discussed > here. > >> OK, do you think we need change all the platform to reflect the 16K >> env size for MMC case? > > Your patch refers to the mx51evk. So change the value according to this > discussion and post your patch again for inclusion in mainline. We will > take into account for other boards when new patches will be submitted. OK, I will set it to 8KiB for mx51evk. > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de > ===================================================================== >
Dear Jason Liu, In message <AANLkTikSO-FaiRdBtSEvXOS6e3u2iMp5df-WRM3nmst+@mail.gmail.com> you wrote: > > OK, I will set it to 8KiB for mx51evk. You are falling from one extreme into the next one. Why not using 16 KiB as suggested? Best regards, Wolfgang Denk
On Wed, 17 Nov 2010 08:42:16 +0100 Wolfgang Denk <wd@denx.de> wrote: > Dear Jason Liu, > > In message <AANLkTim4k-PnuFDgWvYj3TY4mtLcz7Uzy1-TO9vExxbS@mail.gmail.com> you wrote: > > So, I keep the same setting. If I change it, some guys main complain > > why it change, it will make it > > incompatible with the original env setting and make the customer env lost. > > I doubt that. I think I have seen a fair share of different > environment settings. There are many, many in the 1...2 KiB range. > Very, very few exceed 4 KiB. You have to search really hard to find > one that gets close to 8KiB. I think he was talking about CRCs failing after U-Boot is upgraded -- whether the environment content is large or small, changing the size will change the CRC. The environment header doesn't say what the size is, or else compatibility could be maintained that way. Specifying the size in the environment header would also allow you to just CRC the part that has actual data, while allowing the environment to grow up to the entire sector if needed. -Scott
Dear Scott Wood, In message <20101117152851.767ecccf@udp111988uds.am.freescale.net> you wrote: > > > > So, I keep the same setting. If I change it, some guys main complain > > > why it change, it will make it > > > incompatible with the original env setting and make the customer env lost. I missed that part. Agreed, this is a problem, but a small one. In a well-maintained product, sucha configuration change should be properly maintained. Most probably users will receive a release note, which may (read: should) contain a description of the update procedure. In this specific case, users can use "env import" to restore (before overwriting) their old environmnt settings. > I think he was talking about CRCs failing after U-Boot is upgraded -- > whether the environment content is large or small, changing the size > will change the CRC. The environment header doesn't say what the size > is, or else compatibility could be maintained that way. You are right. I missed that point. > Specifying the size in the environment header would also allow you to > just CRC the part that has actual data, while allowing the environment > to grow up to the entire sector if needed. Eventually such a change is not even difficult to implement. The length field could be prepended, shifting the remaining data by 4 bytes. As before, "env import" could be used to recover old environment settings. Best regards, Wolfgang Denk
diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h index f98438d..4af492c 100644 --- a/include/configs/mx51evk.h +++ b/include/configs/mx51evk.h @@ -216,8 +216,9 @@ */ #define CONFIG_SYS_NO_FLASH -#define CONFIG_ENV_SECT_SIZE (128 * 1024) -#define CONFIG_ENV_SIZE CONFIG_ENV_SECT_SIZE -#define CONFIG_ENV_IS_NOWHERE +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) +#define CONFIG_ENV_SIZE (2 * 64 * 1024) +#define CONFIG_ENV_IS_IN_MMC +#define CONFIG_SYS_MMC_ENV_DEV 0 #endif
fix saveenv or env save command not work on mx51evk board. with this patch, we can use savenv or env save to store enviroments to mmc card slot 0 Signed-off-by: Jason Liu <r64343@freescale.com> --- include/configs/mx51evk.h | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)