Patchwork [U-Boot,2/2] cmd_sf: "sf update" preserve the final part of the last sector

login
register
mail settings
Submitter Gerlando Falauto
Date April 3, 2012, 3:14 p.m.
Message ID <1333466098-31783-3-git-send-email-gerlando.falauto@keymile.com>
Download mbox | patch
Permalink /patch/150468/
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Comments

Gerlando Falauto - April 3, 2012, 3:14 p.m.
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>
---
 common/cmd_sf.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)
Simon Glass - April 4, 2012, 12:33 a.m.
Hi Gerlando,

On Tue, Apr 3, 2012 at 8:14 AM, 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 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index d97d4a5..0666f52 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -134,8 +134,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);
> @@ -145,6 +147,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);
> +               /* 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;

If you wrote just the last part of the sector here then you could
perhaps avoid the memcpy() and the return NULL.

> +       }
> +       /* Rewrite the whole block from the source */
>        if (spi_flash_write(flash, offset, len, buf))
>                return "write";
>        return NULL;
> --
> 1.7.1
>

Regards,
Simon

Patch

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index d97d4a5..0666f52 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -134,8 +134,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);
@@ -145,6 +147,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);
+		/* 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;