From: Gerlando Falauto <gerlando.falauto@keymile.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 2/2] cmd_sf: "sf update" preserve the final part of the last sector
Date: Wed, 03 Jul 2013 17:35:13 +0200 [thread overview]
Message-ID: <51D444B1.2040405@keymile.com> (raw)
In-Reply-To: <CAD6G_RS3PNR1uJn8SYFaH5aA+A13HCWgA+iO361WtJsyefj0=Q@mail.gmail.com>
Hi,
On 07/03/2013 05:04 PM, Jagan Teki wrote:
> Hi,
>
> Thanks for the v2.
>
> On Fri, Jun 28, 2013 at 1:13 PM, Gerlando Falauto
> <gerlando.falauto@keymile.com> wrote:
>> Since "sf update" erases the last block as a whole, but only rewrites
>> the meaningful initial part of it, the rest would be left erased,
>> potentially erasing meaningful information.
>> So, as a safety measure, have it rewrite the original content.
>>
>> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
>> Cc: Valentin Longchamp <valentin.longchamp@keymile.com>
>> Cc: Holger Brunck <holger.brunck@keymile.com>
>> Acked-by: Simon Glass <sjg@chromium.org>
>> ---
>> common/cmd_sf.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
>> index ab35a94..fb87d24 100644
>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>> @@ -152,8 +152,10 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
>> {
>> debug("offset=%#x, sector_size=%#x, len=%#zx\n",
>> offset, flash->sector_size, len);
>> - if (spi_flash_read(flash, offset, len, cmp_buf))
>> + /* Read the entire sector so to allow for rewriting */
>> + if (spi_flash_read(flash, offset, flash->sector_size, cmp_buf))
>> return "read";
>> + /* Compare only what is meningful (len) */
>> if (memcmp(cmp_buf, buf, len) == 0) {
>> debug("Skip region %x size %zx: no change\n",
>> offset, len);
>> @@ -163,6 +165,17 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
>> /* Erase the entire sector */
>> if (spi_flash_erase(flash, offset, flash->sector_size))
>> return "erase";
>> + /* If it's a partial sector, preserve the existing part */
>> + if (len != flash->sector_size) {
>> + /* Overwrite the first part of the sector with input data */
>> + memcpy(cmp_buf, buf, len);
>
> Can comment about memcpy() here, as Simon is also asked in v1
> http://patchwork.ozlabs.org/patch/150468/
Whoops, sorry!
I most probably did not see the comment (stopped reading at the Acked-By
line) the first time around and *DEFINITELY* did not see it before
posting v2.
I now understand your point, Simon.
This would imply two spi_flash_write() operations as opposed to just
one, yet allowing for cleaner code.
I guess I was originally trying to both preserve the original behavior
(always write whole sectors) and save on the number of write operations.
But that does not make much sense as we're speaking of an SPI NOR flash
here, the only penalty from an extra write operation only comes from the
extra SPI command. Not really much.
I'll post a v3 soon.
My apologies again.
Thanks,
Gerlando
>
> --
> Thanks,
> Jagan.
>
>> + /* Rewrite the whole sector with original data at the end */
>> + if (spi_flash_write(flash, offset, flash->sector_size,
>> + cmp_buf))
>> + return "write";
>> + return NULL;
>> + }
>> + /* Rewrite the whole block from the source */
>> if (spi_flash_write(flash, offset, len, buf))
>> return "write";
>> return NULL;
>> --
>> 1.7.10.1
>>
prev parent reply other threads:[~2013-07-03 15:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-28 7:43 [U-Boot] [PATCH v2 0/2] SPI flash update command Gerlando Falauto
2013-06-28 7:43 ` [U-Boot] [PATCH v2 1/2] cmd_sf: let "sf update" erase last sector as a whole Gerlando Falauto
2013-06-28 7:43 ` [U-Boot] [PATCH v2 2/2] cmd_sf: "sf update" preserve the final part of the last sector Gerlando Falauto
2013-07-03 15:04 ` Jagan Teki
2013-07-03 15:35 ` Gerlando Falauto [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51D444B1.2040405@keymile.com \
--to=gerlando.falauto@keymile.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.