From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerlando Falauto Date: Wed, 14 Mar 2012 07:58:55 +0100 Subject: [U-Boot] SPI flash writing In-Reply-To: References: <4F5F9103.1030807@keymile.com> Message-ID: <4F6041AF.1090204@keymile.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 03/14/2012 03:18 AM, Simon Glass wrote: > Hi Gerlando, > > On Tue, Mar 13, 2012 at 11:25 AM, Gerlando Falauto > 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