[U-Boot] SPI flash writing

[Gerlando Falauto <gerlando.falauto@keymile.com>]

On 03/14/2012 03:18 AM, Simon Glass wrote:
> Hi Gerlando,
>
> On Tue, Mar 13, 2012 at 11:25 AM, Gerlando Falauto
> <gerlando.falauto@keymile.com>  wrote:
>> Hi everyone,
>>
>> [I took the liberty to Cc: Mike and Simon as they have provided patches in
>> the area]
>>
>> I struggled for a while trying to update a Kirkwood-based board to the
>> latest u-boot (with Keymile's patches).
>>
>> As it turned out, our update procedure:
>>
>> sf probe 0;sf erase 0 50000;sf write ${load_addr_r} 0 ${filesize}
>>
>> mistakenly expects a maximum size of 0x50000 (327680) bytes for
>> u-boot.kwb. Sadly, the latest u-boot trunk results in a binary size for that
>> board which is dangerously close to that limit. Hence, after adding some
>> innocent lines of code, the update procedure could brick the board (for no
>> evident reason and with no error message whatsoever) if the binary size
>> crosses that boundary.
>>
>> It turns out somebody else also picked up this "magic" number:
>> http://lacie-nas.org/doku.php?id=uboot#update_u-boot_mainline
>>
>> And others have bricked their board, most likely for the same reason:
>> http://www.trimslice.com/forum/viewtopic.php?f=16&t=462
>>
>> Also, something bad could happen if you make a mistake in the opposite
>> direction (use too big a number for the write size):
>> http://sequanux.org/pipermail/lacie-nas/2012-March/000378.html
>>
>>  From what I can understand, writing into a sector which has not been erased
>> first is an acceptable behaviour of the flash interface, it will just set to
>> zero whatever bits are not zero already, without reporting any error
>> whatsoever.
>>
>> Even though any change we introduce now would only apply to upgrades FROM
>> future versions, I think it might be worth fixing this somehow.
>> I believe several things could be easily done here:
>>
>> 1) a "+" syntax to the "sf update" command so it can be used with
>> ${filesize} as a parameter, and/or some "read,replace,erase,overwrite" block
>> mechanism for the last (incomplete) block
>
> Sounds reasonable, although I wonder if it is worth worrying about
> preserved the rest of the contents of the last block.

Probably unimportant, as everything you'd ever want to write would be 
block aligned. But I think it still makes sense to make the semantics of 
the update command (which could be also thought of as a way of patching 
regions of arbitrary alignment and length) consistent with linux's 
/dev/mtd, which sort of allows you to do any such things (hiding from 
the user any notion of sector/block).
But please correct me if I'm talking nonsense.

>> 2) an out-of-boundary-check againts the flash size so at least a warning is
>> issued when you use too big a size value
>
> Should be easy enough.
>>
>> 3) a command line option ("sf write -v" and/or to "sf update -v"), or an
>> entirely new command (like "sf writeverify", "sf updateverify") to read back
>> after writing so to double-check what really ended up being written to the
>> flash before it's too late.
>
> I'd like a -V (instead of -v which could perhaps be used for verbose).
>
> But as Mike mentions I wonder if we could/should do this generally for
> all flash?

I agree it would totally make sense.

> Also, why do you get verify failures? 'sf update' will auto-erase when
> it needs to.

True. The option would probably make more sense with the "write" command.

> Do you really have a chip which reports success but then
> fails? Or is it just a problem with the size being too large?

Uhm, there's actually two different problems.
One, "forgetting" to erase enough blocks. This is where -V might come 
useful, but with "write" only.
Two, accidentally writing/updating past the flash size. Here -V would 
not help much (unless you wrap around the whole flash and past the 
starting address again!), but size checking should at least warn you.

Best,
Gerlando