[U-Boot] [PATCH] cmd, nand: add an option to disable the verification when writing in raw mode

[U-Boot] [PATCH] cmd, nand: add an option to disable the verification when writing in raw mode

[Boris Brezillon]

Modern NANDs do not guarantee that data written in raw mode will not
contain bitflips just after writing them. This is fine since the number
of bitflips should be rather low and thus fixable by the ECC engine,
but since we are reading data in raw mode to verify if they match the
input data we cannot prevent failures if some bits are flipped.

The option of using standard mode to verify the data is not acceptable
either, since one of the usage of raw mode is to allow flashing images
that do not respect the standard NAND page layout or the default ECC
config (this is the case on Allwinner platforms, where the ROM code
tests several hardcoded configs, which are not necessarily matching the
NAND characteristics).

Add an extension to the nand write.raw command allowing one to disable
the verification step.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 cmd/nand.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/cmd/nand.c b/cmd/nand.c
index 583a18f..3a5e3a0 100644
--- a/cmd/nand.c
+++ b/cmd/nand.c
@@ -306,7 +306,7 @@ static void nand_print_and_set_info(int idx)
 }
 
 static int raw_access(struct mtd_info *mtd, ulong addr, loff_t off,
-		      ulong count, int read)
+		      ulong count, int read, int noverify)
 {
 	int ret = 0;
 
@@ -324,7 +324,7 @@ static int raw_access(struct mtd_info *mtd, ulong addr, loff_t off,
 			ret = mtd_read_oob(mtd, off, &ops);
 		} else {
 			ret = mtd_write_oob(mtd, off, &ops);
-			if (!ret)
+			if (!ret && !no_verify)
 				ret = nand_verify_page_oob(mtd, &ops, off);
 		}
 
@@ -546,6 +546,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		ulong pagecount = 1;
 		int read;
 		int raw = 0;
+		int no_verify = 0;
 
 		if (argc < 4)
 			goto usage;
@@ -557,9 +558,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 		s = strchr(cmd, '.');
 
-		if (s && !strcmp(s, ".raw")) {
+		if (s && !strncmp(s, ".raw", 4)) {
 			raw = 1;
 
+			if (!strcmp(s, ".raw.noverify"))
+				no_verify = 1;
+
 			if (mtd_arg_off(argv[3], &dev, &off, &size, &maxsize,
 					MTD_DEV_TYPE_NAND,
 					nand_info[dev]->size))
@@ -633,7 +637,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			else
 				ret = mtd_write_oob(mtd, off, &ops);
 		} else if (raw) {
-			ret = raw_access(mtd, addr, off, pagecount, read);
+			ret = raw_access(mtd, addr, off, pagecount, read,
+					 no_verify);
 		} else {
 			printf("Unknown nand command suffix '%s'.\n", s);
 			return 1;
@@ -758,7 +763,7 @@ static char nand_help_text[] =
 	"    read/write 'size' bytes starting at offset 'off'\n"
 	"    to/from memory address 'addr', skipping bad blocks.\n"
 	"nand read.raw - addr off|partition [count]\n"
-	"nand write.raw - addr off|partition [count]\n"
+	"nand write.raw[.noverify] - addr off|partition [count]\n"
 	"    Use read.raw/write.raw to avoid ECC and access the flash as-is.\n"
 #ifdef CONFIG_CMD_NAND_TRIMFFS
 	"nand write.trimffs - addr off|partition size\n"
-- 
2.7.4

[U-Boot] [PATCH] cmd, nand: add an option to disable the verification when writing in raw mode

[Tom Rini]

On Wed, Jun 15, 2016 at 10:42:18AM +0200, Boris Brezillon wrote:

> Modern NANDs do not guarantee that data written in raw mode will not
> contain bitflips just after writing them. This is fine since the number
> of bitflips should be rather low and thus fixable by the ECC engine,
> but since we are reading data in raw mode to verify if they match the
> input data we cannot prevent failures if some bits are flipped.
> 
> The option of using standard mode to verify the data is not acceptable
> either, since one of the usage of raw mode is to allow flashing images
> that do not respect the standard NAND page layout or the default ECC
> config (this is the case on Allwinner platforms, where the ROM code
> tests several hardcoded configs, which are not necessarily matching the
> NAND characteristics).
> 
> Add an extension to the nand write.raw command allowing one to disable
> the verification step.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160624/7f7ca895/attachment.sig>

[U-Boot] cmd, nand: add an option to disable the verification when writing in raw mode

[Scott Wood]

On Wed, Jun 15, 2016 at 10:42:18AM +0200, Boris Brezillon wrote:
> diff --git a/cmd/nand.c b/cmd/nand.c
> index 583a18f..3a5e3a0 100644
> --- a/cmd/nand.c
> +++ b/cmd/nand.c
> @@ -306,7 +306,7 @@ static void nand_print_and_set_info(int idx)
>  }
>  
>  static int raw_access(struct mtd_info *mtd, ulong addr, loff_t off,
> -		      ulong count, int read)
> +		      ulong count, int read, int noverify)
>  {
>  	int ret = 0;
>  
> @@ -324,7 +324,7 @@ static int raw_access(struct mtd_info *mtd, ulong addr, loff_t off,
>  			ret = mtd_read_oob(mtd, off, &ops);
>  		} else {
>  			ret = mtd_write_oob(mtd, off, &ops);
> -			if (!ret)
> +			if (!ret && !no_verify)
>  				ret = nand_verify_page_oob(mtd, &ops, off);

"noverify" versus "no_verify"...  I've fixed it while applying.

I assume/hope that you just sent an old version by mistake, and that this
was actually tested?

-Scott

[U-Boot] cmd, nand: add an option to disable the verification when writing in raw mode

[Boris Brezillon]

On Sun, 24 Jul 2016 20:41:30 -0500
Scott Wood <oss@buserror.net> wrote:

> On Wed, Jun 15, 2016 at 10:42:18AM +0200, Boris Brezillon wrote:
> > diff --git a/cmd/nand.c b/cmd/nand.c
> > index 583a18f..3a5e3a0 100644
> > --- a/cmd/nand.c
> > +++ b/cmd/nand.c
> > @@ -306,7 +306,7 @@ static void nand_print_and_set_info(int idx)
> >  }
> >  
> >  static int raw_access(struct mtd_info *mtd, ulong addr, loff_t off,
> > -		      ulong count, int read)
> > +		      ulong count, int read, int noverify)
> >  {
> >  	int ret = 0;
> >  
> > @@ -324,7 +324,7 @@ static int raw_access(struct mtd_info *mtd, ulong addr, loff_t off,
> >  			ret = mtd_read_oob(mtd, off, &ops);
> >  		} else {
> >  			ret = mtd_write_oob(mtd, off, &ops);
> > -			if (!ret)
> > +			if (!ret && !no_verify)
> >  				ret = nand_verify_page_oob(mtd, &ops, off);  
> 
> "noverify" versus "no_verify"...  I've fixed it while applying.

Oops, sorry about that.

> 
> I assume/hope that you just sent an old version by mistake, and that this
> was actually tested?

Yes, it was tested, just forgot to amend my commit :-(.
Thanks for fixing the problem.

Regards,

Boris