[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Jason Liu]

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(-)

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
-- 
1.7.0.4

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Stefano Babic]

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

-- 
=====================================================================
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 at denx.de
=====================================================================

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Jason Liu]

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 at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Wolfgang Denk]

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

-- 
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
"Consistency requires you to be as ignorant today as you were a  year
ago."                                              - Bernard Berenson

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Jason Liu]

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 at denx.de
> "Consistency requires you to be as ignorant today as you were a ?year
> ago." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?- Bernard Berenson
>

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Wolfgang Denk]

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

-- 
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
There is, however, a strange, musty smell in the air that reminds  me
of something...hmm...yes...I've got it...there's a VMS nearby, or I'm
a Blit.          - Larry Wall in Configure from the perl distribution

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Scott Wood]

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

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Wolfgang Denk]

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

-- 
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
"The algorithm to do that is extremely nasty. You might want  to  mug
someone with it."                   - M. Devine, Computer Science 340

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[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

-- 
=====================================================================
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 at denx.de
=====================================================================

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Jason Liu]

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 at denx.de
> =====================================================================
>

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Wolfgang Denk]

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

-- 
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
"I may be synthetic, but I'm not stupid"  -  the  artificial  person,
from _Aliens_

[U-Boot] [PATCH 1/1] mx51evk: savenv or env save command does not work

[Stefano Babic]

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

-- 
=====================================================================
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 at denx.de
=====================================================================