On 24.01.19 12:14, Boris Brezillon wrote:
On Thu, 24 Jan 2019 11:57:33 +0100
Stefan Roese <sr@xxxxxxx> 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,
Not good at all, I'm afraid:
root@mt7688:~# ./nandbiterrs /dev/mtd5 -i
[ 161.408159] nand: attempt to erase a bad/reserved block @0
libmtd: error!: MEMERASE64 ioctl failed for eraseblock 0 (mtd5)
error 5 (Input/output error)
Cannot erase block 0
root@mt7688:~# cat /sys/class/mtd/mtd5/bad_blocks
992
Without this last patch, I only have 2 bad blocks:
root@mt7688:~# cat /sys/class/mtd/mtd5/bad_blocks
2
As it seems, adding "column" to the buffer address is not a good
idea. Here a short log with some debugging:
[ 27.721104] spinand_read_from_cache_op (241): ret=32 nbytes=128 column=2048
This small patch below of your last one, seems to fix this issue
for me:
root@mt7688:~# cat /sys/class/mtd/mtd5/bad_blocks
2
root@mt7688:~# ./nandbiterrs /dev/mtd5 -i
incremental biterrors test
Successfully corrected 0 bit errors per subpage
Inserted biterror @ 1/7
Read reported 4 corrected bit errors
Successfully corrected 1 bit errors per subpage
Inserted biterror @ 3/7
Read reported 4 corrected bit errors
Successfully corrected 2 bit errors per subpage
Inserted biterror @ 5/7
Read reported 4 corrected bit errors
Successfully corrected 3 bit errors per subpage
Inserted biterror @ 7/7
Read reported 4 corrected bit errors
Successfully corrected 4 bit errors per subpage
Inserted biterror @ 8/7
Read reported 5 corrected bit errors
Successfully corrected 5 bit errors per subpage
Inserted biterror @ 10/7
Read reported 6 corrected bit errors
Successfully corrected 6 bit errors per subpage
Inserted biterror @ 12/7
Read reported 7 corrected bit errors
Successfully corrected 7 bit errors per subpage
Inserted biterror @ 14/7
Read reported 8 corrected bit errors
Successfully corrected 8 bit errors per subpage
Inserted biterror @ 17/7
Failed to recover 1 bitflips
Read error after 9 bit errors per page
What do you think? Could you fold it into your patch if its
acceptable?
Thanks,
Stefan
--->8---
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 659a0e716ee0..a5ccd4adc30f 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -218,6 +218,7 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
unsigned int nbytes = 0;
void *buf = NULL;
u16 column = 0;
+ int offs = 0;
ssize_t ret;
if (req->datalen) {
@@ -237,7 +238,7 @@ 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 + column);
+ ret = spi_mem_dirmap_read(rdesc, column, nbytes, buf + offs);
if (ret < 0)
return ret;
@@ -246,6 +247,7 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
nbytes -= ret;
column += ret;
+ offs += ret;
}
if (req->datalen)
@@ -273,6 +275,7 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
struct mtd_info *mtd = nanddev_to_mtd(nand);
struct spi_mem_dirmap_desc *wdesc;
unsigned int nbytes, column = 0;
+ int offs = 0;
ssize_t ret;
nbytes = nanddev_page_size(nand) + nanddev_per_page_oobsize(nand);
@@ -297,7 +300,7 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
while (nbytes) {
ret = spi_mem_dirmap_write(wdesc, column, nbytes,
- spinand->databuf + column);
+ spinand->databuf + offs);
if (ret < 0)
return ret;
@@ -306,6 +309,7 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
nbytes -= ret;
column += ret;
+ offs += ret;
}
return 0;
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
