[PATCH v2 0/2] NAND and NAND-BBT improvements

[PATCH v2 0/2] NAND and NAND-BBT improvements

[Brian Norris]

Hi,

This is a v2 resubmit of part of a previous series. Some of the previous
patches have already been accepted (and so are omitted here, of course) and
others are still under discussion. I only include the reviewed,
ready-for-submission ones here.

Only v2 changes: comment wording + "Reviewed-by" tags

Of note: the first patch addresses a problem with mtd_read_oob() and the recent
max_bitflips changes by Mike Dunn. If it is satisfactory, it may be worth
merging it in the 3.5-rc cycle (along with its previously-accepted dependent
patch), as it would help avoid an API inconsistency that recently emerged.

Thanks,
Brian

Brian Norris (2):
  mtd: check for max_bitflips in mtd_read_oob()
  mtd: nand: remove unused 'int' return codes

 drivers/mtd/mtdcore.c       | 15 ++++++++++++++-
 drivers/mtd/nand/nand_bbt.c | 19 ++++++++-----------
 2 files changed, 22 insertions(+), 12 deletions(-)

-- 
1.7.11.1

[PATCH v2 1/2] mtd: check for max_bitflips in mtd_read_oob()

[Brian Norris]

mtd_read_oob() has some unexpected similarities to mtd_read(). For
instance, when ops->datbuf != NULL, nand_base.c might return max_bitflips;
however, when ops->datbuf == NULL, nand_base's code potentially could
return -EUCLEAN (no in-tree drivers do this yet). In any case where the
driver might return max_bitflips, we should translate this into an
appropriate return code using the bitflip_threshold.

Essentially, mtd_read_oob() duplicates the logic from mtd_read().

This prevents users of mtd_read_oob() from receiving a positive return
value (i.e., from max_bitflips) and interpreting it as an unknown error.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Reviewed-by Mike Dunn <mikedunn@newsguy.com>
---
v2: reword a few comments, add Reviewed-by

 drivers/mtd/mtdcore.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index fcfce24..44b58c2 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -860,10 +860,23 @@ EXPORT_SYMBOL_GPL(mtd_panic_write);
 
 int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 {
+	int ret_code;
 	ops->retlen = ops->oobretlen = 0;
 	if (!mtd->_read_oob)
 		return -EOPNOTSUPP;
-	return mtd->_read_oob(mtd, from, ops);
+	/*
+	 * In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
+	 * similar to mtd->_read() and may return a non-negative integer
+	 * representing max bitflips. In other cases, mtd->_read_oob() may
+	 * return -EUCLEAN. In all cases, perform similar logic to mtd_read()
+	 * (see above).
+	 */
+	ret_code = mtd->_read_oob(mtd, from, ops);
+	if (unlikely(ret_code < 0))
+		return ret_code;
+	if (mtd->ecc_strength == 0)
+		return 0;	/* device lacks ecc */
+	return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
 }
 EXPORT_SYMBOL_GPL(mtd_read_oob);
 
-- 
1.7.11.1

[PATCH v2 2/2] mtd: nand: remove unused 'int' return codes

[Brian Norris]

The return codes for read_abs_bbts() and search_read_bbts() are always
non-zero, and so don't have much meaning. Just remove them.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
v2: only add Reviewed-by tag

 drivers/mtd/nand/nand_bbt.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 2d1d2fa..9a5dd43 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -370,8 +370,8 @@ static u32 bbt_get_ver_offs(struct mtd_info *mtd, struct nand_bbt_descr *td)
  * Read the bad block table(s) for all chips starting at a given page. We
  * assume that the bbt bits are in consecutive order.
  */
-static int read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
-			 struct nand_bbt_descr *td, struct nand_bbt_descr *md)
+static void read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
+			  struct nand_bbt_descr *td, struct nand_bbt_descr *md)
 {
 	struct nand_chip *this = mtd->priv;
 
@@ -392,7 +392,6 @@ static int read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
 		pr_info("Bad block table at page %d, version 0x%02X\n",
 			 md->pages[0], md->version[0]);
 	}
-	return 1;
 }
 
 /* Scan a given block full */
@@ -623,7 +622,9 @@ static int search_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_descr
  *
  * Search and read the bad block table(s).
  */
-static int search_read_bbts(struct mtd_info *mtd, uint8_t * buf, struct nand_bbt_descr *td, struct nand_bbt_descr *md)
+static void search_read_bbts(struct mtd_info *mtd, uint8_t * buf,
+			     struct nand_bbt_descr *td,
+			     struct nand_bbt_descr *md)
 {
 	/* Search the primary table */
 	search_bbt(mtd, buf, td);
@@ -631,9 +632,6 @@ static int search_read_bbts(struct mtd_info *mtd, uint8_t * buf, struct nand_bbt
 	/* Search the mirror table */
 	if (md)
 		search_bbt(mtd, buf, md);
-
-	/* Force result check */
-	return 1;
 }
 
 /**
@@ -1159,14 +1157,13 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 
 	/* Is the bbt at a given page? */
 	if (td->options & NAND_BBT_ABSPAGE) {
-		res = read_abs_bbts(mtd, buf, td, md);
+		read_abs_bbts(mtd, buf, td, md);
 	} else {
 		/* Search the bad block table using a pattern in oob */
-		res = search_read_bbts(mtd, buf, td, md);
+		search_read_bbts(mtd, buf, td, md);
 	}
 
-	if (res)
-		res = check_create(mtd, buf, bd);
+	res = check_create(mtd, buf, bd);
 
 	/* Prevent the bbt regions from erasing / writing */
 	mark_bbt_region(mtd, td);
-- 
1.7.11.1

Re: [PATCH v2 0/2] NAND and NAND-BBT improvements

[Brian Norris]

Hello maintainers,

On Mon, Jul 16, 2012 at 11:38 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> This is a v2 resubmit of part of a previous series. Some of the previous
> patches have already been accepted (and so are omitted here, of course) and
> others are still under discussion. I only include the reviewed,
> ready-for-submission ones here.
>
> Only v2 changes: comment wording + "Reviewed-by" tags

Can I get a response for these patches? They've been reviewed and
waiting for several weeks now.

> Of note: the first patch addresses a problem with mtd_read_oob() and the recent
> max_bitflips changes by Mike Dunn. If it is satisfactory, it may be worth
> merging it in the 3.5-rc cycle (along with its previously-accepted dependent
> patch), as it would help avoid an API inconsistency that recently emerged.

The 3.5-rc cycle has long passed now, and the important mtd_read_oob()
fix will probably even miss 3.6 now. Is anybody listening?

Thanks,
Brian

Re: [PATCH v2 0/2] NAND and NAND-BBT improvements

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 2346 bytes --]

On Tue, 2012-08-07 at 10:42 -0700, Brian Norris wrote:
> Hello maintainers,
> 
> On Mon, Jul 16, 2012 at 11:38 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > This is a v2 resubmit of part of a previous series. Some of the previous
> > patches have already been accepted (and so are omitted here, of course) and
> > others are still under discussion. I only include the reviewed,
> > ready-for-submission ones here.
> >
> > Only v2 changes: comment wording + "Reviewed-by" tags
> 
> Can I get a response for these patches? They've been reviewed and
> waiting for several weeks now.

Sorry Brian. I did not want to upset you because you are doing good job
in MTD, but I simply do not have bandwidth to handle all the incoming
material quickly enough. This is not payed job for me.

Actually, David is supposed to handle that. But he has not enough time.
So at some point I stepped forward and created the l2 tree to help him,
just because I like MTD stuff. So this is basically my hobby, and I like
to feel that I am useful.

If someone wants to speed things up, he can always start reviewing
others's patches and send his/her Reviewed-by/Tested-by/etc. Then I'd
process the incoming patches faster.

Actually Shmulik is doing great job and he is very helpful.

BTW, I know how it feels when your patches are not getting attention for
long time. My ext4 changes missed 2 merge windows recently. And the
maintainer was taking other patches, which he considered more important.

I am trying to treat all the incoming patches the same, and simply
handle them in FIFO order, with occasional exceptions. I think it is
fair.

> > Of note: the first patch addresses a problem with mtd_read_oob() and the recent
> > max_bitflips changes by Mike Dunn. If it is satisfactory, it may be worth
> > merging it in the 3.5-rc cycle (along with its previously-accepted dependent
> > patch), as it would help avoid an API inconsistency that recently emerged.
> 
> The 3.5-rc cycle has long passed now, and the important mtd_read_oob()
> fix will probably even miss 3.6 now. Is anybody listening?

Actually all the MTD changes missed 3.6. David did not send the pull
request early enough, and Linus closed the merge window 2 or 3 days
earlier than on wold expect.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]