diff mbox series

flash_handler: skip writing to 0xff filled pages on nand

Message ID 20171124150752.3999-1-torben.hohn@linutronix.de
State Accepted
Headers show
Series flash_handler: skip writing to 0xff filled pages on nand | expand

Commit Message

Torben Hohn Nov. 24, 2017, 3:07 p.m. UTC
This is necessary when flashing an ubi image. A block filled with 0ff
does not have an ECC made of 0xff on certein hardware ECC alorithms, So
this already flips some bits, when written.
However, ubi treats a page full of 0xff as empty, which is
not true here. And the result is that data written to such an empty
page yields ECC Errors.

This is basically Code taken from nand-utils/nandwrite.c from
git://git.infradead.org/mtd-utils.git

See http://lists.infradead.org/pipermail/linux-mtd/2016-December/070793.html
for the patch that starts the discussion and the mtd ML.

The relevant Commits in mtd-utils are the following ones:

---------------------------------------------------------------------------
commit 15c21334b201dc54870cfd3e9697307c95f7e4dc
Author: Kees Trommel <ctrommel@linvm302.aimsys.nl>
Date:   Tue Dec 6 09:21:19 2016 +0100

    nandwrite: add skip-all-ff-pages-option

    Signed-off-by: Kees Trommel <ctrommel@linvm302.aimsys.nl>
    Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>


commit f5b47ef5112326b06fd875982379769c55a71703
Author: Marek Vasut <marex@denx.de>
Date:   Fri Dec 9 02:47:41 2016 +0100

    nandwrite: Factor out buffer checking code

    Pull the buffer content checking code into separate function and
    simplify the code invoking it slightly.

    Signed-off-by: Marek Vasut <marex@denx.de>
    Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
    Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---------------------------------------------------------------------------

Signed-off-by: Torben Hohn <torben.hohn@linutronix.de>
---
 handlers/flash_handler.c | 45 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 8 deletions(-)

Comments

Stefano Babic Nov. 24, 2017, 3:55 p.m. UTC | #1
On 24/11/2017 16:07, Torben Hohn wrote:
> This is necessary when flashing an ubi image. A block filled with 0ff
> does not have an ECC made of 0xff on certein hardware ECC alorithms, So
> this already flips some bits, when written.
> However, ubi treats a page full of 0xff as empty, which is
> not true here. And the result is that data written to such an empty
> page yields ECC Errors.
> 
> This is basically Code taken from nand-utils/nandwrite.c from
> git://git.infradead.org/mtd-utils.git
> 
> See http://lists.infradead.org/pipermail/linux-mtd/2016-December/070793.html
> for the patch that starts the discussion and the mtd ML.
> 
> The relevant Commits in mtd-utils are the following ones:
> 
> ---------------------------------------------------------------------------
> commit 15c21334b201dc54870cfd3e9697307c95f7e4dc
> Author: Kees Trommel <ctrommel@linvm302.aimsys.nl>
> Date:   Tue Dec 6 09:21:19 2016 +0100
> 
>     nandwrite: add skip-all-ff-pages-option
> 
>     Signed-off-by: Kees Trommel <ctrommel@linvm302.aimsys.nl>
>     Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
> 
> 
> commit f5b47ef5112326b06fd875982379769c55a71703
> Author: Marek Vasut <marex@denx.de>
> Date:   Fri Dec 9 02:47:41 2016 +0100
> 
>     nandwrite: Factor out buffer checking code
> 
>     Pull the buffer content checking code into separate function and
>     simplify the code invoking it slightly.
> 
>     Signed-off-by: Marek Vasut <marex@denx.de>
>     Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>     Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
> ---------------------------------------------------------------------------
> 
> Signed-off-by: Torben Hohn <torben.hohn@linutronix.de>
> ---
>  handlers/flash_handler.c | 45 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/handlers/flash_handler.c b/handlers/flash_handler.c
> index 081c2a4..10ea0f7 100644
> --- a/handlers/flash_handler.c
> +++ b/handlers/flash_handler.c
> @@ -49,6 +49,32 @@
>  
>  void flash_handler(void);
>  
> +/* Check whether buffer is filled with character 'pattern' */
> +static inline int buffer_check_pattern(unsigned char *buffer, size_t size,
> +                                       unsigned char pattern)
> +{
> +        /* Invalid input */
> +        if (!buffer || (size == 0))
> +                return 0;
> +
> +        /* No match on first byte */
> +        if (*buffer != pattern)
> +                return 0;
> +
> +        /* First byte matched and buffer is 1 byte long, OK. */
> +        if (size == 1)
> +                return 1;
> +
> +        /*
> +         * Check buffer longer than 1 byte. We already know that buffer[0]
> +         * matches the pattern, so the test below only checks whether the
> +         * buffer[0...size-2] == buffer[1...size-1] , which is a test for
> +         * whether the buffer is filled with constant value.
> +         */
> +        return !memcmp(buffer, buffer + 1, size - 1);
> +}
> +
> +
>  /*
>   * Writing to the NAND must take into account ECC errors
>   * and BAD sectors.
> @@ -204,14 +230,17 @@ static int flash_write_nand(int mtdnum, struct img_type *img)
>  
>  		}
>  
> -		/* Write out data */
> -		ret = mtd_write(flash->libmtd, mtd, fd, mtdoffset / mtd->eb_size,
> -				mtdoffset % mtd->eb_size,
> -				writebuf,
> -				mtd->min_io_size,
> -				NULL,
> -				0,
> -				MTD_OPS_PLACE_OOB);
> +		ret =0;
> +		if (!buffer_check_pattern(writebuf, mtd->min_io_size, 0xff)) {
> +			/* Write out data */
> +			ret = mtd_write(flash->libmtd, mtd, fd, mtdoffset / mtd->eb_size,
> +					mtdoffset % mtd->eb_size,
> +					writebuf,
> +					mtd->min_io_size,
> +					NULL,
> +					0,
> +					MTD_OPS_PLACE_OOB);
> +		}
>  		if (ret) {
>  			long long i;
>  			if (errno != EIO) {
> 

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/handlers/flash_handler.c b/handlers/flash_handler.c
index 081c2a4..10ea0f7 100644
--- a/handlers/flash_handler.c
+++ b/handlers/flash_handler.c
@@ -49,6 +49,32 @@ 
 
 void flash_handler(void);
 
+/* Check whether buffer is filled with character 'pattern' */
+static inline int buffer_check_pattern(unsigned char *buffer, size_t size,
+                                       unsigned char pattern)
+{
+        /* Invalid input */
+        if (!buffer || (size == 0))
+                return 0;
+
+        /* No match on first byte */
+        if (*buffer != pattern)
+                return 0;
+
+        /* First byte matched and buffer is 1 byte long, OK. */
+        if (size == 1)
+                return 1;
+
+        /*
+         * Check buffer longer than 1 byte. We already know that buffer[0]
+         * matches the pattern, so the test below only checks whether the
+         * buffer[0...size-2] == buffer[1...size-1] , which is a test for
+         * whether the buffer is filled with constant value.
+         */
+        return !memcmp(buffer, buffer + 1, size - 1);
+}
+
+
 /*
  * Writing to the NAND must take into account ECC errors
  * and BAD sectors.
@@ -204,14 +230,17 @@  static int flash_write_nand(int mtdnum, struct img_type *img)
 
 		}
 
-		/* Write out data */
-		ret = mtd_write(flash->libmtd, mtd, fd, mtdoffset / mtd->eb_size,
-				mtdoffset % mtd->eb_size,
-				writebuf,
-				mtd->min_io_size,
-				NULL,
-				0,
-				MTD_OPS_PLACE_OOB);
+		ret =0;
+		if (!buffer_check_pattern(writebuf, mtd->min_io_size, 0xff)) {
+			/* Write out data */
+			ret = mtd_write(flash->libmtd, mtd, fd, mtdoffset / mtd->eb_size,
+					mtdoffset % mtd->eb_size,
+					writebuf,
+					mtd->min_io_size,
+					NULL,
+					0,
+					MTD_OPS_PLACE_OOB);
+		}
 		if (ret) {
 			long long i;
 			if (errno != EIO) {