Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UC

Boris Brezillon
2019-01-24T11:14:52Z

On Thu, 24 Jan 2019 11:57:33 +0100
Stefan Roese <sr@denx.de> wrote:

> On 24.01.19 10:19, Boris Brezillon wrote:
>
> <snip>
>
> >> So spi_mem_dirmap_read() returns -EINVAL to spinand_write_to_cache_op()
> >> which then returns -EIO.
> >
> > Can you try with the following diff applied?
> > --->8---
> > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > index 52f17fc42daa..67c568f0c47f 100644
> > --- a/drivers/mtd/nand/spi/core.c
> > +++ b/drivers/mtd/nand/spi/core.c
> > @@ -238,7 +238,7 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> >
> > while (nbytes) {
> > ret = spi_mem_dirmap_read(rdesc, column, nbytes, buf);
> > - if (!ret || ret > nbytes)
> > + if (!ret || (ret > 0 && ret > nbytes))
> > ret = -EIO;
> >
> > if (ret < 0)
> > @@ -296,9 +296,9 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
> > wdesc = spinand->dirmaps[req->pos.plane].wdesc;
> >
> > while (nbytes) {
> > - ret = spi_mem_dirmap_read(wdesc, column, nbytes,
> > - spinand->databuf + column);
> > - if (!ret || ret > nbytes)
> > + ret = spi_mem_dirmap_write(wdesc, column, nbytes,
> > + spinand->databuf + column);
> > + if (!ret || (ret > 0 && ret > nbytes))
> > ret = -EIO;
> >
> > if (ret < 0)
> > @@ -761,21 +761,6 @@ static void spinand_destroy_dirmaps(struct spinand_device *spinand)
> > spinand_destroy_dirmap(spinand, i);
> > }
> >
> > -const struct spi_mem_op *
> > -spinand_find_supported_op(struct spinand_device *spinand,
> > - const struct spi_mem_op *ops,
> > - unsigned int nops)
> > -{
> > - unsigned int i;
> > -
> > - for (i = 0; i < nops; i++) {
> > - if (spi_mem_supports_op(spinand->spimem, &ops[i]))
> > - return &ops[i];
> > - }
> > -
> > - return NULL;
> > -}
> > -
> > static const struct nand_ops spinand_ops = {
> > .erase = spinand_erase,
> > .markbad = spinand_markbad,
> >
>
> Sure. Here some logs:
>
> root@mt7688:~# ./nandbiterrs /dev/mtd5 -i
> incremental biterrors test
> ECC failure, invalid data despite read success

Okay, so now we're back to a problem in the read path. Looks like the
buf pointer passed to spi_mem_dirmap_read() is only valid for the first
iteration of the "while (nbytes)" loop.

Can you try with this diff (and sorry for asking you to debug/test that,
but I no longer have access to a device with a SPI NAND)?

--->8---
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 52f17fc42daa..90da21b52bb5 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -237,13 +237,14 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
rdesc = spinand->dirmaps[req->pos.plane].rdesc;

while (nbytes) {
- ret = spi_mem_dirmap_read(rdesc, column, nbytes, buf);
- if (!ret || ret > nbytes)
- ret = -EIO;
-
+ ret = spi_mem_dirmap_read(rdesc, column, nbytes, buf + column);
if (ret < 0)
return ret;

+ if (!ret || ret > nbytes)
+ return -EIO;
+
+
nbytes -= ret;
column += ret;
}
@@ -296,14 +297,14 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
wdesc = spinand->dirmaps[req->pos.plane].wdesc;

while (nbytes) {
- ret = spi_mem_dirmap_read(wdesc, column, nbytes,
- spinand->databuf + column);
- if (!ret || ret > nbytes)
- ret = -EIO;
-
+ ret = spi_mem_dirmap_write(wdesc, column, nbytes,
+ spinand->databuf + column);
if (ret < 0)
return ret;

+ if (!ret || ret > nbytes)
+ return -EIO;
+
nbytes -= ret;
column += ret;
}
@@ -761,21 +762,6 @@ static void spinand_destroy_dirmaps(struct spinand_device *spinand)
spinand_destroy_dirmap(spinand, i);
}

-const struct spi_mem_op *
-spinand_find_supported_op(struct spinand_device *spinand,
- const struct spi_mem_op *ops,
- unsigned int nops)
-{
- unsigned int i;
-
- for (i = 0; i < nops; i++) {
- if (spi_mem_supports_op(spinand->spimem, &ops[i]))
- return &ops[i];
- }
-
- return NULL;
-}
-
static const struct nand_ops spinand_ops = {
.erase = spinand_erase,
.markbad = spinand_markbad,