| Message ID | 20180111214202.24531-1-miquel.raynal@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] mtd: nand: Fix nand_do_read_oob() return value | expand |
On Thu, 11 Jan 2018 22:42:02 +0100 Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Starting from commit 041e4575f034 ("mtd: nand: handle ECC errors in > OOB"), nand_do_read_oob() (from the NAND core) did return 0 or a > negative error, and the MTD layer expected it. > > However, the trend for the NAND layer is now to return an error or a > positive number of bitflips. Deciding which status to return to the user > belongs to the MTD layer. > > Commit e47f68587b82 ("mtd: check for max_bitflips in mtd_read_oob()") > brought this logic to the mtd_read_oob() function while the return value > coming from nand_do_read_oob() (called by the ->_read_oob() hook) was > left unchanged. > > Fixes: e47f68587b82 ("mtd: check for max_bitflips in mtd_read_oob()") > Cc: stable@vger.kernel.org > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > --- > > Changes since v1: > - s/->ecc.read_oob() hook/->_read_oob() hook/ in the commit message > - Fixed the compilation issue > > drivers/mtd/nand/nand_base.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 469220065b8b..440d9f5d5b17 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -3876,7 +3876,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from, > if (mtd->ecc_stats.failed - stats.failed) > return -EBADMSG; > > - return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0; > + return ret; This is wrong, you're only returning the max number of bitflips of the last OOB read. > } > > /**
Hi Boris, On Fri, 12 Jan 2018 08:52:25 +0100 Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > On Thu, 11 Jan 2018 22:42:02 +0100 > Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > > > Starting from commit 041e4575f034 ("mtd: nand: handle ECC errors in > > OOB"), nand_do_read_oob() (from the NAND core) did return 0 or a > > negative error, and the MTD layer expected it. > > > > However, the trend for the NAND layer is now to return an error or a > > positive number of bitflips. Deciding which status to return to the > > user belongs to the MTD layer. > > > > Commit e47f68587b82 ("mtd: check for max_bitflips in > > mtd_read_oob()") brought this logic to the mtd_read_oob() function > > while the return value coming from nand_do_read_oob() (called by > > the ->_read_oob() hook) was left unchanged. > > > > Fixes: e47f68587b82 ("mtd: check for max_bitflips in > > mtd_read_oob()") Cc: stable@vger.kernel.org > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > > --- > > > > Changes since v1: > > - s/->ecc.read_oob() hook/->_read_oob() hook/ in the commit > > message > > - Fixed the compilation issue > > > > drivers/mtd/nand/nand_base.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/nand/nand_base.c > > b/drivers/mtd/nand/nand_base.c index 469220065b8b..440d9f5d5b17 > > 100644 --- a/drivers/mtd/nand/nand_base.c > > +++ b/drivers/mtd/nand/nand_base.c > > @@ -3876,7 +3876,7 @@ static int nand_do_read_oob(struct mtd_info > > *mtd, loff_t from, if (mtd->ecc_stats.failed - stats.failed) > > return -EBADMSG; > > > > - return mtd->ecc_stats.corrected - stats.corrected ? > > -EUCLEAN : 0; > > + return ret; > > This is wrong, you're only returning the max number of bitflips of the > last OOB read. I forgot about the while loop, that's right. There will be a v3 :) Thanks, Miquèl
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 469220065b8b..440d9f5d5b17 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3876,7 +3876,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from, if (mtd->ecc_stats.failed - stats.failed) return -EBADMSG; - return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0; + return ret; } /**
Starting from commit 041e4575f034 ("mtd: nand: handle ECC errors in OOB"), nand_do_read_oob() (from the NAND core) did return 0 or a negative error, and the MTD layer expected it. However, the trend for the NAND layer is now to return an error or a positive number of bitflips. Deciding which status to return to the user belongs to the MTD layer. Commit e47f68587b82 ("mtd: check for max_bitflips in mtd_read_oob()") brought this logic to the mtd_read_oob() function while the return value coming from nand_do_read_oob() (called by the ->_read_oob() hook) was left unchanged. Fixes: e47f68587b82 ("mtd: check for max_bitflips in mtd_read_oob()") Cc: stable@vger.kernel.org Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> --- Changes since v1: - s/->ecc.read_oob() hook/->_read_oob() hook/ in the commit message - Fixed the compilation issue drivers/mtd/nand/nand_base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)