[RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Brian Norris]

This (somewhat) reverts commit 1696e6bc2ae83734e64e206ac99766ea19e9a14e.

In the patch "mtd: nand: kill NAND_NO_READRDY", I overlooked a few
things.

The original documentation for NAND_NO_READRDY included "True for all
large page devices, as they do not support autoincrement." I was
conflating "not support autoincrement" with the NAND_NO_AUTOINCR option,
which was in fact doing nothing. So, when I dropped NAND_NO_AUTOINCR, I
concluded that I then could harmlessly drop NAND_NO_READRDY. But of
course the fact the NAND_NO_AUTOINCR was doing nothing didn't mean
NAND_NO_READRDY was doing nothing...

So, NAND_NO_READRDY is re-introduced as NAND_NEED_READRDY and applied
only to those few remaining small-page NAND which needed it in the first
place.

This is probably a candidate for stable, but there will certainly be
conflicts, as drivers/mtd/nand/nand_ids.c has changed significantly.

Compile-tested only; I don't have a setup that requires this.

Reported-by: Alexander Shiyan <shc_work@mail.ru>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---

This is an attempt at a version 2 of Alexander's RFC patch. I feel like the
original (per NAND chip type) option makes more sense than per driver (where
Alexander sets this option only for the diskonchip driver). But it is certainly
more invasive. And apparently, no one else needs this option (or at least, no
one complains). Perhaps everyone has just moved beyond small-page NAND?

 drivers/mtd/nand/nand_base.c |   16 +++++++++++
 drivers/mtd/nand/nand_ids.c  |   60 +++++++++++++++++++++---------------------
 include/linux/mtd/nand.h     |    8 ++++++
 3 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 72eada2..7e64dca 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1550,6 +1550,14 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 					oobreadlen -= toread;
 				}
 			}
+
+			if (chip->options & NAND_NEED_READRDY) {
+				/* Apply delay or wait for ready/busy pin */
+				if (!chip->dev_ready)
+					udelay(chip->chip_delay);
+				else
+					nand_wait_ready(mtd);
+			}
 		} else {
 			memcpy(buf, chip->buffers->databuf + col, bytes);
 			buf += bytes;
@@ -1814,6 +1822,14 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
 		len = min(len, readlen);
 		buf = nand_transfer_oob(chip, buf, ops, len);
 
+		if (chip->options & NAND_NEED_READRDY) {
+			/* Apply delay or wait for ready/busy pin */
+			if (!chip->dev_ready)
+				udelay(chip->chip_delay);
+			else
+				nand_wait_ready(mtd);
+		}
+
 		readlen -= len;
 		if (!readlen)
 			break;
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 625bc89..2bf2395 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -22,36 +22,36 @@
  * extended chip ID.
  */
 struct nand_flash_dev nand_flash_ids[] = {
-	LEGACY_ID_NAND("NAND 4MiB 5V 8-bit",   0x6B, 512, 4, 0x2000, 0),
-	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 512, 4, 0x2000, 0),
-	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE5, 512, 4, 0x2000, 0),
-	LEGACY_ID_NAND("NAND 8MiB 3,3V 8-bit", 0xD6, 512, 8, 0x2000, 0),
-	LEGACY_ID_NAND("NAND 8MiB 3,3V 8-bit", 0xE6, 512, 8, 0x2000, 0),
-
-	LEGACY_ID_NAND("NAND 16MiB 1,8V 8-bit",  0x33, 512, 16, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 16MiB 3,3V 8-bit",  0x73, 512, 16, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 16MiB 1,8V 16-bit", 0x43, 512, 16, 0x4000, NAND_BUSWIDTH_16),
-	LEGACY_ID_NAND("NAND 16MiB 3,3V 16-bit", 0x53, 512, 16, 0x4000, NAND_BUSWIDTH_16),
-
-	LEGACY_ID_NAND("NAND 32MiB 1,8V 8-bit",  0x35, 512, 32, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 32MiB 3,3V 8-bit",  0x75, 512, 32, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 32MiB 1,8V 16-bit", 0x45, 512, 32, 0x4000, NAND_BUSWIDTH_16),
-	LEGACY_ID_NAND("NAND 32MiB 3,3V 16-bit", 0x55, 512, 32, 0x4000, NAND_BUSWIDTH_16),
-
-	LEGACY_ID_NAND("NAND 64MiB 1,8V 8-bit",  0x36, 512, 64, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 64MiB 3,3V 8-bit",  0x76, 512, 64, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 64MiB 1,8V 16-bit", 0x46, 512, 64, 0x4000, NAND_BUSWIDTH_16),
-	LEGACY_ID_NAND("NAND 64MiB 3,3V 16-bit", 0x56, 512, 64, 0x4000, NAND_BUSWIDTH_16),
-
-	LEGACY_ID_NAND("NAND 128MiB 1,8V 8-bit",  0x78, 512, 128, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 128MiB 1,8V 8-bit",  0x39, 512, 128, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 128MiB 3,3V 8-bit",  0x79, 512, 128, 0x4000, 0),
-	LEGACY_ID_NAND("NAND 128MiB 1,8V 16-bit", 0x72, 512, 128, 0x4000, NAND_BUSWIDTH_16),
-	LEGACY_ID_NAND("NAND 128MiB 1,8V 16-bit", 0x49, 512, 128, 0x4000, NAND_BUSWIDTH_16),
-	LEGACY_ID_NAND("NAND 128MiB 3,3V 16-bit", 0x74, 512, 128, 0x4000, NAND_BUSWIDTH_16),
-	LEGACY_ID_NAND("NAND 128MiB 3,3V 16-bit", 0x59, 512, 128, 0x4000, NAND_BUSWIDTH_16),
-
-	LEGACY_ID_NAND("NAND 256MiB 3,3V 8-bit", 0x71, 512, 256, 0x4000, 0),
+	LEGACY_ID_NAND("NAND 4MiB 5V 8-bit",   0x6B, 512, 4, 0x2000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 512, 4, 0x2000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE5, 512, 4, 0x2000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 8MiB 3,3V 8-bit", 0xD6, 512, 8, 0x2000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 8MiB 3,3V 8-bit", 0xE6, 512, 8, 0x2000, NAND_NEED_READRDY),
+
+	LEGACY_ID_NAND("NAND 16MiB 1,8V 8-bit",  0x33, 512, 16, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 16MiB 3,3V 8-bit",  0x73, 512, 16, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 16MiB 1,8V 16-bit", 0x43, 512, 16, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+	LEGACY_ID_NAND("NAND 16MiB 3,3V 16-bit", 0x53, 512, 16, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+
+	LEGACY_ID_NAND("NAND 32MiB 1,8V 8-bit",  0x35, 512, 32, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 32MiB 3,3V 8-bit",  0x75, 512, 32, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 32MiB 1,8V 16-bit", 0x45, 512, 32, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+	LEGACY_ID_NAND("NAND 32MiB 3,3V 16-bit", 0x55, 512, 32, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+
+	LEGACY_ID_NAND("NAND 64MiB 1,8V 8-bit",  0x36, 512, 64, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 64MiB 3,3V 8-bit",  0x76, 512, 64, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 64MiB 1,8V 16-bit", 0x46, 512, 64, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+	LEGACY_ID_NAND("NAND 64MiB 3,3V 16-bit", 0x56, 512, 64, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+
+	LEGACY_ID_NAND("NAND 128MiB 1,8V 8-bit",  0x78, 512, 128, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 128MiB 1,8V 8-bit",  0x39, 512, 128, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 128MiB 3,3V 8-bit",  0x79, 512, 128, 0x4000, NAND_NEED_READRDY),
+	LEGACY_ID_NAND("NAND 128MiB 1,8V 16-bit", 0x72, 512, 128, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+	LEGACY_ID_NAND("NAND 128MiB 1,8V 16-bit", 0x49, 512, 128, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+	LEGACY_ID_NAND("NAND 128MiB 3,3V 16-bit", 0x74, 512, 128, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+	LEGACY_ID_NAND("NAND 128MiB 3,3V 16-bit", 0x59, 512, 128, 0x4000, NAND_NEED_READRDY | NAND_BUSWIDTH_16),
+
+	LEGACY_ID_NAND("NAND 256MiB 3,3V 8-bit", 0x71, 512, 256, 0x4000, NAND_NEED_READRDY),
 
 	/*
 	 * These are the new chips with large page size. Their page size and
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 0c40beb..f992b38 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -147,6 +147,14 @@ typedef enum {
 #define NAND_BUSWIDTH_16	0x00000002
 /* Chip has cache program function */
 #define NAND_CACHEPRG		0x00000008
+
+/*
+ * Chip requires ready check on read (for auto-incremented sequential read).
+ * True only for small page devices; large page devices do not support
+ * autoincrement.
+ */
+#define NAND_NEED_READRDY	0x00000100
+
 /* Chip does not allow subpage writes */
 #define NAND_NO_SUBPAGE_WRITE	0x00000200
 
-- 
1.7.9.5

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Brian Norris]

On Wed, Mar 13, 2013 at 9:51 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> This is an attempt at a version 2 of Alexander's RFC patch. I feel like the
> original (per NAND chip type) option makes more sense than per driver (where
> Alexander sets this option only for the diskonchip driver). But it is certainly
> more invasive. And apparently, no one else needs this option (or at least, no
> one complains). Perhaps everyone has just moved beyond small-page NAND?

I'm sorry, I meant to send this is a reply to Alexander's patch. See
his RFC and RFC RESEND:

http://lists.infradead.org/pipermail/linux-mtd/2013-March/046024.html
http://lists.infradead.org/pipermail/linux-mtd/2013-March/046159.html

Brian

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Alexander Shiyan]

Hello.

> This (somewhat) reverts commit 1696e6bc2ae83734e64e206ac99766ea19e9a14e.
> 
> In the patch "mtd: nand: kill NAND_NO_READRDY", I overlooked a few
> things.
> 
> The original documentation for NAND_NO_READRDY included "True for all
> large page devices, as they do not support autoincrement." I was
> conflating "not support autoincrement" with the NAND_NO_AUTOINCR option,
> which was in fact doing nothing. So, when I dropped NAND_NO_AUTOINCR, I
> concluded that I then could harmlessly drop NAND_NO_READRDY. But of
> course the fact the NAND_NO_AUTOINCR was doing nothing didn't mean
> NAND_NO_READRDY was doing nothing...
> 
> So, NAND_NO_READRDY is re-introduced as NAND_NEED_READRDY and applied
> only to those few remaining small-page NAND which needed it in the first
> place.
> 
> This is probably a candidate for stable, but there will certainly be
> conflicts, as drivers/mtd/nand/nand_ids.c has changed significantly.
> 
> Compile-tested only; I don't have a setup that requires this.
> 
> Reported-by: Alexander Shiyan <shc_work@mail.ru>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Thanks. I will check this version tomorrow.

---

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Alexander Shiyan]

> This (somewhat) reverts commit 1696e6bc2ae83734e64e206ac99766ea19e9a14e.
> 
> In the patch "mtd: nand: kill NAND_NO_READRDY", I overlooked a few
> things.
> 
> The original documentation for NAND_NO_READRDY included "True for all
> large page devices, as they do not support autoincrement." I was
> conflating "not support autoincrement" with the NAND_NO_AUTOINCR option,
> which was in fact doing nothing. So, when I dropped NAND_NO_AUTOINCR, I
> concluded that I then could harmlessly drop NAND_NO_READRDY. But of
> course the fact the NAND_NO_AUTOINCR was doing nothing didn't mean
> NAND_NO_READRDY was doing nothing...
> 
> So, NAND_NO_READRDY is re-introduced as NAND_NEED_READRDY and applied
> only to those few remaining small-page NAND which needed it in the first
> place.
> 
> This is probably a candidate for stable, but there will certainly be
> conflicts, as drivers/mtd/nand/nand_ids.c has changed significantly.
> 
> Compile-tested only; I don't have a setup that requires this.
> 
> Reported-by: Alexander Shiyan <shc_work@mail.ru>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> 
> This is an attempt at a version 2 of Alexander's RFC patch. I feel like the
> original (per NAND chip type) option makes more sense than per driver (where
> Alexander sets this option only for the diskonchip driver). But it is certainly
> more invasive. And apparently, no one else needs this option (or at least, no
> one complains). Perhaps everyone has just moved beyond small-page NAND?
...

Tested today with 3 DiskOnChip devices which contain following chips:
NAND device: Manufacturer ID: 0x98, Chip ID: 0x75 (Toshiba NAND 32MiB 3,3V 8-bit), 32MiB, page size: 512, OOB size: 16
NAND device: Manufacturer ID: 0xec, Chip ID: 0x75 (Samsung NAND 32MiB 3,3V 8-bit), 32MiB, page size: 512, OOB size: 16
NAND device: Manufacturer ID: 0x98, Chip ID: 0xe6 (Toshiba NAND 8MiB 3,3V 8-bit), 8MiB, page size: 512, OOB size: 16

All OK. Thanks.

---

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[David Woodhouse]

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

On Thu, 2013-03-14 at 11:18 +0400, Alexander Shiyan wrote:
> > This (somewhat) reverts commit 1696e6bc2ae83734e64e206ac99766ea19e9a14e.
> > 
> > In the patch "mtd: nand: kill NAND_NO_READRDY", I overlooked a few
> > things...
> 
> Tested today with 3 DiskOnChip devices which contain following chips:
> NAND device: Manufacturer ID: 0x98, Chip ID: 0x75 (Toshiba NAND 32MiB 3,3V 8-bit), 32MiB, page size: 512, OOB size: 16
> NAND device: Manufacturer ID: 0xec, Chip ID: 0x75 (Samsung NAND 32MiB 3,3V 8-bit), 32MiB, page size: 512, OOB size: 16
> NAND device: Manufacturer ID: 0x98, Chip ID: 0xe6 (Toshiba NAND 8MiB 3,3V 8-bit), 8MiB, page size: 512, OOB size: 16
> 
> All OK. Thanks.

For 3.9 (and cc stable) then?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Artem Bityutskiy]

On Wed, 2013-03-13 at 09:51 -0700, Brian Norris wrote:
> +			if (chip->options & NAND_NEED_READRDY) {
> +				/* Apply delay or wait for ready/busy pin */
> +				if (!chip->dev_ready)
> +					udelay(chip->chip_delay);
> +				else
> +					nand_wait_ready(mtd);
> +			}

Am I right that this is a small page NAND-specific thing? If yes, can we
just this magic to all small page NANDs without introducing the option?

Also, irrespectively of the final solution, let's do it this way:

1. Create a patch against the 3.9-rc2, not l2-mtd.git, and test it.
2. Add Cc stable, just like David asked.
2. Bug David to merge it.
3. I rebase the l2 tree on top of that.

-- 
Best Regards,
Artem Bityutskiy

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[David Woodhouse]

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

On Thu, 2013-03-14 at 12:41 +0200, Artem Bityutskiy wrote:
> On Wed, 2013-03-13 at 09:51 -0700, Brian Norris wrote:
> > +			if (chip->options & NAND_NEED_READRDY) {
> > +				/* Apply delay or wait for ready/busy pin */
> > +				if (!chip->dev_ready)
> > +					udelay(chip->chip_delay);
> > +				else
> > +					nand_wait_ready(mtd);
> > +			}
> 
> Am I right that this is a small page NAND-specific thing? If yes, can we
> just this magic to all small page NANDs without introducing the option?

Probably, but the option flag is the cleaner way of doing it. Yes, it's
a slightly more intrusive patch right now, but it's nicer.

> Also, irrespectively of the final solution, let's do it this way:
> 
> 1. Create a patch against the 3.9-rc2, not l2-mtd.git, and test it.
> 2. Add Cc stable, just like David asked.
> 2. Bug David to merge it.
> 3. I rebase the l2 tree on top of that.

I've got a couple of (unrelated) patches in linux-mtd.git which I was
about to send to Linus for 3.9. Let's put it on top of that and I'll
send the whole lot together.

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[David Woodhouse]

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

On Thu, 2013-03-14 at 10:51 +0000, David Woodhouse wrote:
> 
> I've got a couple of (unrelated) patches in linux-mtd.git which I was
> about to send to Linus for 3.9. Let's put it on top of that and I'll
> send the whole lot together.

Alexander, please could you retest the version I've just pushed out to
linux-mtd.git?

Thanks.

http://git.infradead.org/linux-mtd.git/commitdiff/5bc7c33ca


-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Alexander Shiyan]

> On Thu, 2013-03-14 at 10:51 +0000, David Woodhouse wrote:
> > 
> > I've got a couple of (unrelated) patches in linux-mtd.git which I was
> > about to send to Linus for 3.9. Let's put it on top of that and I'll
> > send the whole lot together.
> 
> Alexander, please could you retest the version I've just pushed out to
> linux-mtd.git?
> 
> Thanks.
> 
> http://git.infradead.org/linux-mtd.git/commitdiff/5bc7c33ca

All OK.

Tested-by: Alexander Shiyan <shc_work@mail.ru>

---

Re: [RFC "v2"] mtd: nand: reintroduce NAND_NO_READRDY as NAND_NEED_READRDY

[Brian Norris]

On 03/14/2013 05:52 AM, David Woodhouse wrote:
> On Thu, 2013-03-14 at 10:51 +0000, David Woodhouse wrote:
>>
>> I've got a couple of (unrelated) patches in linux-mtd.git which I was
>> about to send to Linus for 3.9. Let's put it on top of that and I'll
>> send the whole lot together.
>
> Alexander, please could you retest the version I've just pushed out to
> linux-mtd.git?
>
> Thanks.
>
> http://git.infradead.org/linux-mtd.git/commitdiff/5bc7c33ca

Your modified versions looks good to me. Thanks for taking this.

Brian