[U-Boot] incremental environment updating

[Jerry Van Baren]

Mike Frysinger wrote:
> On Monday 13 April 2009 08:12:44 Jerry Van Baren wrote:
>> Mike Frysinger wrote:
>>> On Monday 13 April 2009 06:15:24 Wolfgang Denk wrote:
>>>> In message Mike wrote:
>>>>> currently the env code will erase the entire env storage before writing
>>>>> back out the current env, even if the env storage has enough empty
>>>>> space to store the current env.  for example, if CONFIG_ENV_SIZE is
>>>>> declared as 0x2000 but the current env only takes up ~0x300 bytes, the
>>>>> whole 0x2000 is erased and then the ~0x300 gets written out.  seems
>>>>> like we can get a pretty good return for fairly low effort if we
>>>>> appended env updates rather than erasing/writing every time ?  it'd
>>>>> certainly be faster. while systems with a dedicated sector this isnt so
>>>>> bad, but for people who have to embed the env in the middle of a large
>>>>> sector, this would be much faster most of the time.

Hmmm, rereading this, if the issue is simply the time to erase a sector, 
I don't think this is that big of a deal.  It takes ~2 seconds? to erase 
a sector and env variables are rarely changed in my experience.  Does 
your mileage vary?

>>>>> has there been previous discussion along these lines that i havent seen
>>>>> ?
>>>> This hasn't been discussed before. Interesting idea. However, I fail
>>>> to see how this could be implemented without changing the environment
>>>> format?
>>> that depends on how you want the compatibility to go.  being able to read
>>> old environments by newer u-boots is reasonable, but i dont think having
>>> old u- boots read newer environments makes realistic sense ?
>>>
>>> in terms of actual changes, i had a couple of ideas ... the current env
>>> format is: <crc><env><NUL>[undefined].  so if we logically extend the
>>> format where [undefined] is <crc><env><NUL>[...], then all existing env
>>> storage would be automatically imported.  considering most env storage
>>> out there uses a bit value of "0" to mean programmed and "1' to mean
>>> unprogrammed, it should be pretty easy to quickly detect where the
>>> appended envs stop.
>> Another concept is to append a complete new env on every write.  This is
>> less efficient than just a delta, but has a substantial advantage in
>> that it gives the user a way to erase env variables as well as change
>> them.  I suspect it would also be simpler to implement.
> 
> this is actually what i was talking about.  doing a delta would require the 
> env code to compare the old env and "whiteout" variables that were set but are 
> now unset, and it would take longer to boot up as the env code would have to 
> rebuild using the deltas.

I'm confused, are we actually proposing the same thing?  When you say 
"delta" and "whiteout" I picture additional definitions appended as a 
delta env lump to the original env lump, not a whole new env lump.

My counterproposal eliminates the delta determination software and 
totally eliminates having to invent a new way to "whiteout" to indicate 
removed variables (IIRC, it is valid to set a variable with no value).

Further, once you find "the" env lump, the env processing is 100% the 
same so the implementation of my counterproposal simply wraps that 
existing env handling code with a "find the latest lump" loop.

>> Since flash can change 1s to 0s, but cannot change them back, you could
>> redefine the env storage to be
>>    <next><crc><env><NUL>[undefined]
>> (note the added <next> at the start).  By definition, the valid env's
>> <next> == 0xFFFFFFFF.  To write a new env, simply write the offset of
>> [undefined] into the <next> location and then write a new env lump.
>>    <next><crc><env><NUL> <next><crc><env><NUL>[undefined]
>>     ^^^^ offset of -----> ^^^^ 0xFFFFFFFF
> 
> using a crc of 0x00000000 accomplishes the same thing and retains env format

Three problems:
1) 0x00000000 can be a valid CRC.  You either have a risk of failure 
(granted, very small) or you have to special case that instance.  Note 
that, if you don't handle the 0x00000000 case properly, you will 
probably brick the board.

2) Handling a redundant env may be more difficult.  OK, that probably 
isn't too bad, assuming you can handle the 0x00000000 CRC value properly 
since a bad env would have a non-zero invalid CRC.

3) My <next> offset is much faster since you don't have to step through 
the old env to find where the next one starts.  It also handles 
Wolfgang's point with NAND flashes needing to be aligned to blocks. 
(What do you do with a non-zero bad CRC?  Fall back to the redundant env?)

>> Either way, we would need a new command, say "compactenv" (kind of a
>> long command :-(, maybe there is a shorter German word ;-).  This would
>> erase the current env sector and rewrite it with one env, resetting the
>> clock.  This could also be done automatically if there is no room left
>> in the env storage area (both options would probably be good to implement).
> 
> i didnt intend for any external behavior change.  the env_*.c would take care 
> of the appending and/or reset step.  every thing else would continue to 
> operate the same way.  along those lines, i dont really see the need for 
> "compactenv" beyond a helper command when debugging implementation.  there 
> would be no need to manually run this step.
> -mike

OK.

gvb