From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Tue, 13 Mar 2012 19:18:54 -0700
Subject: [U-Boot] SPI flash writing
In-Reply-To: <4F5F9103.1030807@keymile.com>
References: <4F5F9103.1030807@keymile.com>
Message-ID: <CAPnjgZ0eiP8OitO3g__dmLrF+UoC1v7f72RjnZOMWG=CXnHCow@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.

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

Also, why do you get verify failures? 'sf update' will auto-erase when
it needs to. Do you really have a chip which reports success but then
fails? Or is it just a problem with the size being too large?

>
> I'm willing to implement them, but I wanted to hear your thoughts first.
>

Always good.

Regards,
Simon

> Thanks,
> Gerland