public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH 0/9] mtd: nand: improve NAND ID detection
@ 2012-09-25  3:40 Brian Norris
  2012-09-25  3:40 ` [PATCH 1/9] mtd: nand: remove unnecessary variable Brian Norris
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

Hi all,

I've been working on some changes to nand_base.c's nand_get_flash_type()
routine for improving its structure and readability as well as to detect some
new chips. Generally, this patch series does the following:

(1) split several of the big blocks of detection code into separate functions,
    each designated with different tasks
(2) add support for several "new" Hynix and Samsung NAND, which have new
    decoding tables
(3) provide a better baseline on which I or others can make future changes
    (esp., the "generic READ ID length calculation" is useful for detecting
    other patterns that will determine BB marker options)

I have tested this patch series for regressions against a few chips:

  Samsung K9GAG08U0D
  Samsung K9LBG08U0E
  Hynix H27U8G8G5D
  Micron MT29F32G08CBACA

Additionally, I used a test harness for nand_get_flash_type() provided Angus
Clark to test for both regressions and proper fixes (contact Angus if you need
the suite; I'm not sure I have permission to redistribute). The test uses some
data from:

  http://www.linux-mtd.infradead.org/nand-data/nanddata.html

I do not have actual samples of most of the new Samsung and Hynix but have
largely relied on datasheets and support from others who have these chips. Any
testing -- with either old or new chips -- is appreciated.

Thanks,
Brian

Brian Norris (9):
  mtd: nand: remove unnecessary variable
  mtd: nand: remove redundant ID read
  mtd: nand: split BB marker options decoding into its own function
  mtd: nand: split extended ID decoding into its own function
  mtd: nand: split simple ID decode into its own function
  mtd: nand: add generic READ ID length calculation functions
  mtd: nand: increase max OOB size to 640
  mtd: nand: decode Hynix MLC, 6-byte ID length
  mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID

 drivers/mtd/nand/nand_base.c | 360 +++++++++++++++++++++++++++++--------------
 include/linux/mtd/nand.h     |   2 +-
 2 files changed, 248 insertions(+), 114 deletions(-)

-- 
1.7.11.3

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/9] mtd: nand: remove unnecessary variable
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 2/9] mtd: nand: remove redundant ID read Brian Norris
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

We don't actually use the 'ret' variable; we set it, test it, and then it dies.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 88f671c..30588eb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2872,7 +2872,6 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 {
 	int i, maf_idx;
 	u8 id_data[8];
-	int ret;
 
 	/* Select the device */
 	chip->select_chip(mtd, 0);
@@ -2919,8 +2918,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	chip->onfi_version = 0;
 	if (!type->name || !type->pagesize) {
 		/* Check is chip is ONFI compliant */
-		ret = nand_flash_detect_onfi(mtd, chip, &busw);
-		if (ret)
+		if (nand_flash_detect_onfi(mtd, chip, &busw))
 			goto ident_done;
 	}
 
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/9] mtd: nand: remove redundant ID read
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
  2012-09-25  3:40 ` [PATCH 1/9] mtd: nand: remove unnecessary variable Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 3/9] mtd: nand: split BB marker options decoding into its own function Brian Norris
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

Instead of reading 2 bytes then later 8 bytes, we can simply read all 8
bytes from the start.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 30588eb..f725247 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2898,7 +2898,8 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 
 	chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
 
-	for (i = 0; i < 2; i++)
+	/* Read entire ID string */
+	for (i = 0; i < 8; i++)
 		id_data[i] = chip->read_byte(mtd);
 
 	if (id_data[0] != *maf_id || id_data[1] != *dev_id) {
@@ -2922,13 +2923,6 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 			goto ident_done;
 	}
 
-	chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
-
-	/* Read entire ID string */
-
-	for (i = 0; i < 8; i++)
-		id_data[i] = chip->read_byte(mtd);
-
 	if (!type->name)
 		return ERR_PTR(-ENODEV);
 
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/9] mtd: nand: split BB marker options decoding into its own function
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
  2012-09-25  3:40 ` [PATCH 1/9] mtd: nand: remove unnecessary variable Brian Norris
  2012-09-25  3:40 ` [PATCH 2/9] mtd: nand: remove redundant ID read Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 4/9] mtd: nand: split extended ID " Brian Norris
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

When detecting NAND parameters, the code gets a little ugly so that the
logic is obscured. Try to remedy that by moving code to separate functions
that have well-defined purposes.

This patch splits the bad block marker options detection into its own function,
away from the other parameters (e.g., chip size, page size, etc.).

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 66 ++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index f725247..c7fc330 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2862,6 +2862,43 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /*
+ * Set the bad block marker/indicator (BBM/BBI) patterns according to some
+ * heuristic patterns using various detected parameters (e.g., manufacturer,
+ * page size, cell-type information).
+ */
+static void nand_decode_bbm_options(struct mtd_info *mtd,
+				    struct nand_chip *chip, u8 id_data[8])
+{
+	int maf_id = id_data[0];
+
+	/* Set the bad block position */
+	if (mtd->writesize > 512 || (chip->options & NAND_BUSWIDTH_16))
+		chip->badblockpos = NAND_LARGE_BADBLOCK_POS;
+	else
+		chip->badblockpos = NAND_SMALL_BADBLOCK_POS;
+
+	/*
+	 * Bad block marker is stored in the last page of each block on Samsung
+	 * and Hynix MLC devices; stored in first two pages of each block on
+	 * Micron devices with 2KiB pages and on SLC Samsung, Hynix, Toshiba,
+	 * AMD/Spansion, and Macronix.  All others scan only the first page.
+	 */
+	if ((chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+			(maf_id == NAND_MFR_SAMSUNG ||
+			 maf_id == NAND_MFR_HYNIX))
+		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
+	else if ((!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+				(maf_id == NAND_MFR_SAMSUNG ||
+				 maf_id == NAND_MFR_HYNIX ||
+				 maf_id == NAND_MFR_TOSHIBA ||
+				 maf_id == NAND_MFR_AMD ||
+				 maf_id == NAND_MFR_MACRONIX)) ||
+			(mtd->writesize == 2048 &&
+			 maf_id == NAND_MFR_MICRON))
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+}
+
+/*
  * Get the flash and manufacturer id and lookup if the type is supported.
  */
 static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
@@ -3043,6 +3080,8 @@ ident_done:
 		return ERR_PTR(-EINVAL);
 	}
 
+	nand_decode_bbm_options(mtd, chip, id_data);
+
 	/* Calculate the address shift from the page size */
 	chip->page_shift = ffs(mtd->writesize) - 1;
 	/* Convert chipsize to number of pages per chip -1 */
@@ -3059,33 +3098,6 @@ ident_done:
 
 	chip->badblockbits = 8;
 
-	/* Set the bad block position */
-	if (mtd->writesize > 512 || (busw & NAND_BUSWIDTH_16))
-		chip->badblockpos = NAND_LARGE_BADBLOCK_POS;
-	else
-		chip->badblockpos = NAND_SMALL_BADBLOCK_POS;
-
-	/*
-	 * Bad block marker is stored in the last page of each block
-	 * on Samsung and Hynix MLC devices; stored in first two pages
-	 * of each block on Micron devices with 2KiB pages and on
-	 * SLC Samsung, Hynix, Toshiba, AMD/Spansion, and Macronix.
-	 * All others scan only the first page.
-	 */
-	if ((chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-			(*maf_id == NAND_MFR_SAMSUNG ||
-			 *maf_id == NAND_MFR_HYNIX))
-		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
-	else if ((!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-				(*maf_id == NAND_MFR_SAMSUNG ||
-				 *maf_id == NAND_MFR_HYNIX ||
-				 *maf_id == NAND_MFR_TOSHIBA ||
-				 *maf_id == NAND_MFR_AMD ||
-				 *maf_id == NAND_MFR_MACRONIX)) ||
-			(mtd->writesize == 2048 &&
-			 *maf_id == NAND_MFR_MICRON))
-		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
-
 	/* Check for AND chips with 4 page planes */
 	if (chip->options & NAND_4PAGE_ARRAY)
 		chip->erase_cmd = multi_erase_cmd;
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/9] mtd: nand: split extended ID decoding into its own function
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (2 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 3/9] mtd: nand: split BB marker options decoding into its own function Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 5/9] mtd: nand: split simple ID decode " Brian Norris
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

When detecting NAND parameters, the code gets a little ugly so that the
logic is obscured. Try to remedy that by moving code to separate functions
that have well-defined purposes.

This patch splits out the extended ID decode functionality, which handles
decoding the 3rd-8th ID bytes to determine NAND device parameters.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 122 ++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 55 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c7fc330..afbe1ce 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2862,6 +2862,71 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /*
+ * Many new NAND share similar device ID codes, which represent the size of the
+ * chip. The rest of the parameters must be decoded according to generic or
+ * manufacturer-specific "extended ID" decoding patterns.
+ */
+static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
+				u8 id_data[8], int *busw)
+{
+	int extid;
+	/* The 3rd id byte holds MLC / multichip data */
+	chip->cellinfo = id_data[2];
+	/* The 4th id byte is the important one */
+	extid = id_data[3];
+
+	/*
+	 * Field definitions are in the following datasheets:
+	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
+	 * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
+	 *
+	 * Check for wraparound + Samsung ID + nonzero 6th byte
+	 * to decide what to do.
+	 */
+	if (id_data[0] == id_data[6] && id_data[1] == id_data[7] &&
+			id_data[0] == NAND_MFR_SAMSUNG &&
+			(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+			id_data[5] != 0x00) {
+		/* Calc pagesize */
+		mtd->writesize = 2048 << (extid & 0x03);
+		extid >>= 2;
+		/* Calc oobsize */
+		switch (extid & 0x03) {
+		case 1:
+			mtd->oobsize = 128;
+			break;
+		case 2:
+			mtd->oobsize = 218;
+			break;
+		case 3:
+			mtd->oobsize = 400;
+			break;
+		default:
+			mtd->oobsize = 436;
+			break;
+		}
+		extid >>= 2;
+		/* Calc blocksize */
+		mtd->erasesize = (128 * 1024) <<
+			(((extid >> 1) & 0x04) | (extid & 0x03));
+		*busw = 0;
+	} else {
+		/* Calc pagesize */
+		mtd->writesize = 1024 << (extid & 0x03);
+		extid >>= 2;
+		/* Calc oobsize */
+		mtd->oobsize = (8 << (extid & 0x01)) *
+			(mtd->writesize >> 9);
+		extid >>= 2;
+		/* Calc blocksize. Blocksize is multiples of 64KiB */
+		mtd->erasesize = (64 * 1024) << (extid & 0x03);
+		extid >>= 2;
+		/* Get buswidth information */
+		*busw = (extid & 0x01) ? NAND_BUSWIDTH_16 : 0;
+	}
+}
+
+/*
  * Set the bad block marker/indicator (BBM/BBI) patterns according to some
  * heuristic patterns using various detected parameters (e.g., manufacturer,
  * page size, cell-type information).
@@ -2972,61 +3037,8 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		/* Set the pagesize, oobsize, erasesize by the driver */
 		busw = chip->init_size(mtd, chip, id_data);
 	} else if (!type->pagesize) {
-		int extid;
-		/* The 3rd id byte holds MLC / multichip data */
-		chip->cellinfo = id_data[2];
-		/* The 4th id byte is the important one */
-		extid = id_data[3];
-
-		/*
-		 * Field definitions are in the following datasheets:
-		 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
-		 * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
-		 *
-		 * Check for wraparound + Samsung ID + nonzero 6th byte
-		 * to decide what to do.
-		 */
-		if (id_data[0] == id_data[6] && id_data[1] == id_data[7] &&
-				id_data[0] == NAND_MFR_SAMSUNG &&
-				(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-				id_data[5] != 0x00) {
-			/* Calc pagesize */
-			mtd->writesize = 2048 << (extid & 0x03);
-			extid >>= 2;
-			/* Calc oobsize */
-			switch (extid & 0x03) {
-			case 1:
-				mtd->oobsize = 128;
-				break;
-			case 2:
-				mtd->oobsize = 218;
-				break;
-			case 3:
-				mtd->oobsize = 400;
-				break;
-			default:
-				mtd->oobsize = 436;
-				break;
-			}
-			extid >>= 2;
-			/* Calc blocksize */
-			mtd->erasesize = (128 * 1024) <<
-				(((extid >> 1) & 0x04) | (extid & 0x03));
-			busw = 0;
-		} else {
-			/* Calc pagesize */
-			mtd->writesize = 1024 << (extid & 0x03);
-			extid >>= 2;
-			/* Calc oobsize */
-			mtd->oobsize = (8 << (extid & 0x01)) *
-				(mtd->writesize >> 9);
-			extid >>= 2;
-			/* Calc blocksize. Blocksize is multiples of 64KiB */
-			mtd->erasesize = (64 * 1024) << (extid & 0x03);
-			extid >>= 2;
-			/* Get buswidth information */
-			busw = (extid & 0x01) ? NAND_BUSWIDTH_16 : 0;
-		}
+		/* Decode parameters from extended ID */
+		nand_decode_ext_id(mtd, chip, id_data, &busw);
 	} else {
 		/*
 		 * Old devices have chip data hardcoded in the device id table.
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/9] mtd: nand: split simple ID decode into its own function
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (3 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 4/9] mtd: nand: split extended ID " Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 6/9] mtd: nand: add generic READ ID length calculation functions Brian Norris
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

When detecting NAND parameters, the code gets a little ugly so that the
logic is obscured. Try to remedy that by moving code to separate functions
that have well-defined purposes.

This patch splits out the simple ID decode functionality, where all the
information regarding NAND size/blocksize/pagesize/oobsize/busw is encoded in
the first two bytes of the ID string.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 51 +++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index afbe1ce..380f783 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2927,6 +2927,36 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /*
+ * Old devices have chip data hardcoded in the device ID table. nand_decode_id
+ * decodes a matching ID table entry and assigns the MTD size parameters for
+ * the chip.
+ */
+static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
+				struct nand_flash_dev *type, u8 id_data[8],
+				int *busw)
+{
+	int maf_id = id_data[0];
+
+	mtd->erasesize = type->erasesize;
+	mtd->writesize = type->pagesize;
+	mtd->oobsize = mtd->writesize / 32;
+	*busw = type->options & NAND_BUSWIDTH_16;
+
+	/*
+	 * Check for Spansion/AMD ID + repeating 5th, 6th byte since
+	 * some Spansion chips have erasesize that conflicts with size
+	 * listed in nand_ids table.
+	 * Data sheet (5 byte ID): Spansion S30ML-P ORNAND (p.39)
+	 */
+	if (maf_id == NAND_MFR_AMD && id_data[4] != 0x00 && id_data[5] == 0x00
+			&& id_data[6] == 0x00 && id_data[7] == 0x00
+			&& mtd->writesize == 512) {
+		mtd->erasesize = 128 * 1024;
+		mtd->erasesize <<= ((id_data[3] & 0x03) << 1);
+	}
+}
+
+/*
  * Set the bad block marker/indicator (BBM/BBI) patterns according to some
  * heuristic patterns using various detected parameters (e.g., manufacturer,
  * page size, cell-type information).
@@ -3040,26 +3070,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 		/* Decode parameters from extended ID */
 		nand_decode_ext_id(mtd, chip, id_data, &busw);
 	} else {
-		/*
-		 * Old devices have chip data hardcoded in the device id table.
-		 */
-		mtd->erasesize = type->erasesize;
-		mtd->writesize = type->pagesize;
-		mtd->oobsize = mtd->writesize / 32;
-		busw = type->options & NAND_BUSWIDTH_16;
-
-		/*
-		 * Check for Spansion/AMD ID + repeating 5th, 6th byte since
-		 * some Spansion chips have erasesize that conflicts with size
-		 * listed in nand_ids table.
-		 * Data sheet (5 byte ID): Spansion S30ML-P ORNAND (p.39)
-		 */
-		if (*maf_id == NAND_MFR_AMD && id_data[4] != 0x00 &&
-				id_data[5] == 0x00 && id_data[6] == 0x00 &&
-				id_data[7] == 0x00 && mtd->writesize == 512) {
-			mtd->erasesize = 128 * 1024;
-			mtd->erasesize <<= ((id_data[3] & 0x03) << 1);
-		}
+		nand_decode_id(mtd, chip, type, id_data, &busw);
 	}
 	/* Get chip options */
 	chip->options |= type->options;
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/9] mtd: nand: add generic READ ID length calculation functions
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (4 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 5/9] mtd: nand: split simple ID decode " Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 7/9] mtd: nand: increase max OOB size to 640 Brian Norris
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

When decoding the extended ID bytes of a NAND chip, we have to calculate the ID
length according to some heuristic patterns (e.g., Does the ID wrap around?
Does it end in trailing zeros?). Currently, these heuristics are built into
complicated if/else blocks that can be hard to understand.

Now, these checks can be done generically in a function, making them more
robust and reusable. In fact, this sort of calculation is needed in future
additions to nand_base.c. And with this advancement, we get the added benefit
of a more readable "extended ID decode".

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 72 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 380f783..fd5c307 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2862,6 +2862,65 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /*
+ * nand_id_has_period - Check if an ID string has a given wraparound period
+ * @id_data: the ID string
+ * @arrlen: the length of the @id_data array
+ * @period: the period of repitition
+ *
+ * Check if an ID string is repeated within a given sequence of bytes at
+ * specific repetition interval period (e.g., {0x20,0x01,0x7F,0x20} has a
+ * period of 2). This is a helper function for nand_id_len(). Returns non-zero
+ * if the repetition has a period of @period; otherwise, returns zero.
+ */
+static int nand_id_has_period(u8 *id_data, int arrlen, int period)
+{
+	int i, j;
+	for (i = 0; i < period; i++)
+		for (j = i + period; j < arrlen; j += period)
+			if (id_data[i] != id_data[j])
+				return 0;
+	return 1;
+}
+
+/*
+ * nand_id_len - Get the length of an ID string returned by CMD_READID
+ * @id_data: the ID string
+ * @arrlen: the length of the @id_data array
+
+ * Returns the length of the ID string, according to known wraparound/trailing
+ * zero patterns. If no pattern exists, returns the length of the array.
+ */
+static int nand_id_len(u8 *id_data, int arrlen)
+{
+	int last_nonzero, period;
+
+	/* Find last non-zero byte */
+	for (last_nonzero = arrlen - 1; last_nonzero >= 0; last_nonzero--)
+		if (id_data[last_nonzero])
+			break;
+
+	/* All zeros */
+	if (last_nonzero < 0)
+		return 0;
+
+	/* Calculate wraparound period */
+	for (period = 1; period < arrlen; period++)
+		if (nand_id_has_period(id_data, arrlen, period))
+			break;
+
+	/* There's a repeated pattern */
+	if (period < arrlen)
+		return period;
+
+	/* There are trailing zeros */
+	if (last_nonzero < arrlen - 1)
+		return last_nonzero + 1;
+
+	/* No pattern detected */
+	return arrlen;
+}
+
+/*
  * Many new NAND share similar device ID codes, which represent the size of the
  * chip. The rest of the parameters must be decoded according to generic or
  * manufacturer-specific "extended ID" decoding patterns.
@@ -2869,24 +2928,23 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 				u8 id_data[8], int *busw)
 {
-	int extid;
+	int extid, id_len;
 	/* The 3rd id byte holds MLC / multichip data */
 	chip->cellinfo = id_data[2];
 	/* The 4th id byte is the important one */
 	extid = id_data[3];
 
+	id_len = nand_id_len(id_data, 8);
+
 	/*
 	 * Field definitions are in the following datasheets:
 	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
 	 * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
 	 *
-	 * Check for wraparound + Samsung ID + nonzero 6th byte
-	 * to decide what to do.
+	 * Check for ID length + Samsung ID to decide what to do.
 	 */
-	if (id_data[0] == id_data[6] && id_data[1] == id_data[7] &&
-			id_data[0] == NAND_MFR_SAMSUNG &&
-			(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-			id_data[5] != 0x00) {
+	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
+			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
 		/* Calc pagesize */
 		mtd->writesize = 2048 << (extid & 0x03);
 		extid >>= 2;
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/9] mtd: nand: increase max OOB size to 640
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (5 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 6/9] mtd: nand: add generic READ ID length calculation functions Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 8/9] mtd: nand: decode Hynix MLC, 6-byte ID length Brian Norris
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

Some Hynix and Samsung MLC NAND have 640B OOB size. Sooner or later, we should
dynamically allocate the buffers that use these macros.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 include/linux/mtd/nand.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index ce22824..817ff5a 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -56,7 +56,7 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
  * is supported now. If you add a chip with bigger oobsize/page
  * adjust this accordingly.
  */
-#define NAND_MAX_OOBSIZE	576
+#define NAND_MAX_OOBSIZE	640
 #define NAND_MAX_PAGESIZE	8192
 
 /*
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/9] mtd: nand: decode Hynix MLC, 6-byte ID length
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (6 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 7/9] mtd: nand: increase max OOB size to 640 Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-09-25  3:40 ` [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID Brian Norris
  2012-09-27 12:23 ` [PATCH 0/9] mtd: nand: improve NAND ID detection Artem Bityutskiy
  9 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

Hynix has introduced a new ID decoding scheme for their newer MLC, some of
which don't support ONFI. The following devices all follow the pattern given in
the datasheet for Hynix H27UBG8T2B, p.22:

Hynix H27UAG8T2A
Hynix H27UBG8T2A
Hynix H27UBG8T2B

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 45 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index fd5c307..7e93d0d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2940,8 +2940,10 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 	 * Field definitions are in the following datasheets:
 	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
 	 * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
+	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
 	 *
-	 * Check for ID length + Samsung ID to decide what to do.
+	 * Check for ID length, cell type, and Hynix/Samsung ID to decide what
+	 * to do.
 	 */
 	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
 			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
@@ -2968,6 +2970,47 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 		mtd->erasesize = (128 * 1024) <<
 			(((extid >> 1) & 0x04) | (extid & 0x03));
 		*busw = 0;
+	} else if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
+			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+		unsigned int tmp;
+
+		/* Calc pagesize */
+		mtd->writesize = 2048 << (extid & 0x03);
+		extid >>= 2;
+		/* Calc oobsize */
+		switch (((extid >> 2) & 0x04) | (extid & 0x03)) {
+		case 0:
+			mtd->oobsize = 128;
+			break;
+		case 1:
+			mtd->oobsize = 224;
+			break;
+		case 2:
+			mtd->oobsize = 448;
+			break;
+		case 3:
+			mtd->oobsize = 64;
+			break;
+		case 4:
+			mtd->oobsize = 32;
+			break;
+		case 5:
+			mtd->oobsize = 16;
+			break;
+		default:
+			mtd->oobsize = 640;
+			break;
+		}
+		extid >>= 2;
+		/* Calc blocksize */
+		tmp = ((extid >> 1) & 0x04) | (extid & 0x03);
+		if (tmp < 0x03)
+			mtd->erasesize = (128 * 1024) << tmp;
+		else if (tmp == 0x03)
+			mtd->erasesize = 768 * 1024;
+		else
+			mtd->erasesize = (64 * 1024) << tmp;
+		*busw = 0;
 	} else {
 		/* Calc pagesize */
 		mtd->writesize = 1024 << (extid & 0x03);
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (7 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 8/9] mtd: nand: decode Hynix MLC, 6-byte ID length Brian Norris
@ 2012-09-25  3:40 ` Brian Norris
  2012-10-05  1:37   ` Marek Vasut
  2012-09-27 12:23 ` [PATCH 0/9] mtd: nand: improve NAND ID detection Artem Bityutskiy
  9 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2012-09-25  3:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: Angus Clark, Mike Dunn, Artem Bityutskiy, Huang Shijie,
	Shmulik Ladkani, Brian Norris, David Woodhouse

Datasheets for the following Samsung NAND parts (both MLC and SLC) describe
extensions to the Samsung 6-byte extended ID decoding table:

K9GBG08U0A (MLC, 6-byte ID)
K9GAG08U0F (MLC, 6-byte ID)
K9FAG08U0M (SLC, 6-byte ID)

The table found in K9GAG08U0F, p.44, contains a superset of the information
found in other previous datasheets.

This patch adds support for all of these chips, with 512B and 640B OOB sizes.
It also changes the detection pattern such that this table applies to all
Samsung 6-byte ID NAND, not just MLC. This is safe, according to the NAND
parameter data I have collected:

Note that nand_base.c does not yet support the bad block marker scheme defined
for these chips (i.e., scan 1st and last page for BB markers).

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7e93d0d..bcb58ce 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2939,19 +2939,18 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 	/*
 	 * Field definitions are in the following datasheets:
 	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
-	 * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
+	 * New style   (6 byte ID): Samsung K9GAG08U0F (p.44)
 	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
 	 *
 	 * Check for ID length, cell type, and Hynix/Samsung ID to decide what
 	 * to do.
 	 */
-	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
-			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
 		/* Calc pagesize */
 		mtd->writesize = 2048 << (extid & 0x03);
 		extid >>= 2;
 		/* Calc oobsize */
-		switch (extid & 0x03) {
+		switch (((extid >> 2) & 0x04) | (extid & 0x03)) {
 		case 1:
 			mtd->oobsize = 128;
 			break;
@@ -2961,9 +2960,16 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 		case 3:
 			mtd->oobsize = 400;
 			break;
-		default:
+		case 4:
 			mtd->oobsize = 436;
 			break;
+		case 5:
+			mtd->oobsize = 512;
+			break;
+		case 6:
+		default: /* Other cases are "reserved" (unknown) */
+			mtd->oobsize = 640;
+			break;
 		}
 		extid >>= 2;
 		/* Calc blocksize */
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/9] mtd: nand: improve NAND ID detection
  2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
                   ` (8 preceding siblings ...)
  2012-09-25  3:40 ` [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID Brian Norris
@ 2012-09-27 12:23 ` Artem Bityutskiy
  9 siblings, 0 replies; 14+ messages in thread
From: Artem Bityutskiy @ 2012-09-27 12:23 UTC (permalink / raw)
  To: Brian Norris
  Cc: Angus Clark, Mike Dunn, Huang Shijie, linux-mtd, Shmulik Ladkani,
	David Woodhouse

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

On Mon, 2012-09-24 at 20:40 -0700, Brian Norris wrote:
> Hi all,
> 
> I've been working on some changes to nand_base.c's nand_get_flash_type()
> routine for improving its structure and readability as well as to detect some
> new chips. Generally, this patch series does the following:

Pushed the series to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
  2012-09-25  3:40 ` [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID Brian Norris
@ 2012-10-05  1:37   ` Marek Vasut
  2012-10-06 20:24     ` Brian Norris
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Vasut @ 2012-10-05  1:37 UTC (permalink / raw)
  To: linux-mtd
  Cc: Fabio Estevam, Angus Clark, Mike Dunn, Artem Bityutskiy,
	Huang Shijie, Shmulik Ladkani, Brian Norris, David Woodhouse

Dear Brian Norris,

> Datasheets for the following Samsung NAND parts (both MLC and SLC) describe
> extensions to the Samsung 6-byte extended ID decoding table:
[...]

This breaks my board with K9F2G08 part:

[    0.860000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xda (Samsung NAND 
256MiB 3,3V 8-bit), page size: 2048, OOB size: 64


> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 7e93d0d..bcb58ce 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2939,19 +2939,18 @@ static void nand_decode_ext_id(struct mtd_info
> *mtd, struct nand_chip *chip, /*
>  	 * Field definitions are in the following datasheets:
>  	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
> -	 * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
> +	 * New style   (6 byte ID): Samsung K9GAG08U0F (p.44)
>  	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
>  	 *
>  	 * Check for ID length, cell type, and Hynix/Samsung ID to decide what
>  	 * to do.
>  	 */
> -	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
> -			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
> +	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
[...]

I believe the above hunk is wrong, reverting it fixes the problem.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
  2012-10-05  1:37   ` Marek Vasut
@ 2012-10-06 20:24     ` Brian Norris
  2012-10-07  3:29       ` Marek Vasut
  0 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2012-10-06 20:24 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Fabio Estevam, Angus Clark, Mike Dunn, Artem Bityutskiy,
	Huang Shijie, linux-mtd, Shmulik Ladkani, David Woodhouse

Hi Marek,

On Thu, Oct 4, 2012 at 6:37 PM, Marek Vasut <marex@denx.de> wrote:
> This breaks my board with K9F2G08 part:
>
> [    0.860000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xda (Samsung NAND
> 256MiB 3,3V 8-bit), page size: 2048, OOB size: 64

Thanks for the report. I really appreciate the testing! Exactly which
part is this, and what is the full 8-byte ID? For instance, I can
reproduce this on K9F2G08U0B, with ID EC DA 10 95 44 00 EC DA.

>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 7e93d0d..bcb58ce 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -2939,19 +2939,18 @@ static void nand_decode_ext_id(struct mtd_info
>> *mtd, struct nand_chip *chip, /*
>>        * Field definitions are in the following datasheets:
>>        * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
>> -      * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
>> +      * New style   (6 byte ID): Samsung K9GAG08U0F (p.44)
>>        * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
>>        *
>>        * Check for ID length, cell type, and Hynix/Samsung ID to decide what
>>        * to do.
>>        */
>> -     if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
>> -                     (chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
>> +     if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
> [...]
>
> I believe the above hunk is wrong, reverting it fixes the problem.

Yeah, that is one fix. But my real issue is in the ID length, not SLC
vs. MLC. Its datasheet *says* it has a 5-byte ID, but it actually
wraps around at the 6-byte mark (i.e., EC DA 10 95 44 00 EC DA ...) so
its ID length appears as 6. Now, the above hunk is intentional, since
I found Samsung SLC NAND with 6-byte ID which follow this same
pattern. And I don't have any recorded Samsung 6-byte ID SLC who
*don't* follow this pattern. See the following doc, plus I have more
NAND data that should be updated here sometime:

http://www.linux-mtd.infradead.org/nand-data/nanddata.html

Anyway, according to my data, the problem is actually in this hunk in patch 6:

mtd: nand: add generic READ ID length calculation functions

-       if (id_data[0] == id_data[6] && id_data[1] == id_data[7] &&
-                       id_data[0] == NAND_MFR_SAMSUNG &&
-                       (chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-                       id_data[5] != 0x00) {
+       if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
+                       (chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {

Notice that I dropped the 'id_data[5] != 0x00' check. So I think if I
squash the following diff (probably mangled) into the series, this
solves your problems and mine. Can you test?

If it works, I'll probably resend this diff with a full, proper
explanation (and Tested-by credit), or else I can send a v2 of patches
6 and 9. (It's up to Artem, which form he'd rather maintain in his
tree.)

Thanks,
Brian

---
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ec6841d..fa1ae76 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2989,7 +2989,8 @@ static void nand_decode_ext_id(struct mtd_info
*mtd, struct nand_chip *chip,
 	 * Check for ID length, cell type, and Hynix/Samsung ID to decide what
 	 * to do.
 	 */
-	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
+	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
+			id_data[5] != 0x00) {
 		/* Calc pagesize */
 		mtd->writesize = 2048 << (extid & 0x03);
 		extid >>= 2;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID
  2012-10-06 20:24     ` Brian Norris
@ 2012-10-07  3:29       ` Marek Vasut
  0 siblings, 0 replies; 14+ messages in thread
From: Marek Vasut @ 2012-10-07  3:29 UTC (permalink / raw)
  To: Brian Norris
  Cc: Fabio Estevam, Angus Clark, Mike Dunn, Artem Bityutskiy,
	Huang Shijie, linux-mtd, Shmulik Ladkani, David Woodhouse

Dear Brian Norris,

> Hi Marek,
> 
> On Thu, Oct 4, 2012 at 6:37 PM, Marek Vasut <marex@denx.de> wrote:
> > This breaks my board with K9F2G08 part:
> > 
> > [    0.860000] NAND device: Manufacturer ID: 0xec, Chip ID: 0xda (Samsung
> > NAND 256MiB 3,3V 8-bit), page size: 2048, OOB size: 64
> 
> Thanks for the report. I really appreciate the testing! Exactly which
> part is this, and what is the full 8-byte ID? For instance, I can
> reproduce this on K9F2G08U0B, with ID EC DA 10 95 44 00 EC DA.

Yep, that's it ...

ec da 10 95 44 00 ec da

> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> >> index 7e93d0d..bcb58ce 100644
> >> --- a/drivers/mtd/nand/nand_base.c
> >> +++ b/drivers/mtd/nand/nand_base.c
> >> @@ -2939,19 +2939,18 @@ static void nand_decode_ext_id(struct mtd_info
> >> *mtd, struct nand_chip *chip, /*
> >> 
> >>        * Field definitions are in the following datasheets:
> >>        * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
> >> 
> >> -      * New style   (6 byte ID): Samsung K9GBG08U0M (p.40)
> >> +      * New style   (6 byte ID): Samsung K9GAG08U0F (p.44)
> >> 
> >>        * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
> >>        *
> >>        * Check for ID length, cell type, and Hynix/Samsung ID to decide
> >>        what * to do.
> >>        */
> >> 
> >> -     if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
> >> -                     (chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
> >> +     if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
> > 
> > [...]
> > 
> > I believe the above hunk is wrong, reverting it fixes the problem.
> 
> Yeah, that is one fix. But my real issue is in the ID length, not SLC
> vs. MLC. Its datasheet *says* it has a 5-byte ID, but it actually
> wraps around at the 6-byte mark (i.e., EC DA 10 95 44 00 EC DA ...) so
> its ID length appears as 6. Now, the above hunk is intentional, since
> I found Samsung SLC NAND with 6-byte ID which follow this same
> pattern. And I don't have any recorded Samsung 6-byte ID SLC who
> *don't* follow this pattern. See the following doc, plus I have more
> NAND data that should be updated here sometime:
> 
> http://www.linux-mtd.infradead.org/nand-data/nanddata.html

Nice!

> Anyway, according to my data, the problem is actually in this hunk in patch
> 6:
> 
> mtd: nand: add generic READ ID length calculation functions
> 
> -       if (id_data[0] == id_data[6] && id_data[1] == id_data[7] &&
> -                       id_data[0] == NAND_MFR_SAMSUNG &&
> -                       (chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
> -                       id_data[5] != 0x00) {
> +       if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
> +                       (chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
> 
> Notice that I dropped the 'id_data[5] != 0x00' check. So I think if I
> squash the following diff (probably mangled) into the series, this
> solves your problems and mine. Can you test?

Yes, that's a good solution.

Tested-by: Marek Vasut <marex@denx.de>

> If it works, I'll probably resend this diff with a full, proper
> explanation (and Tested-by credit), or else I can send a v2 of patches
> 6 and 9. (It's up to Artem, which form he'd rather maintain in his
> tree.)
> 
> Thanks,
> Brian
> 
> ---
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index ec6841d..fa1ae76 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2989,7 +2989,8 @@ static void nand_decode_ext_id(struct mtd_info
> *mtd, struct nand_chip *chip,
>  	 * Check for ID length, cell type, and Hynix/Samsung ID to decide what
>  	 * to do.
>  	 */
> -	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG) {
> +	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
> +			id_data[5] != 0x00) {
>  		/* Calc pagesize */
>  		mtd->writesize = 2048 << (extid & 0x03);
>  		extid >>= 2;

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-10-07  3:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-25  3:40 [PATCH 0/9] mtd: nand: improve NAND ID detection Brian Norris
2012-09-25  3:40 ` [PATCH 1/9] mtd: nand: remove unnecessary variable Brian Norris
2012-09-25  3:40 ` [PATCH 2/9] mtd: nand: remove redundant ID read Brian Norris
2012-09-25  3:40 ` [PATCH 3/9] mtd: nand: split BB marker options decoding into its own function Brian Norris
2012-09-25  3:40 ` [PATCH 4/9] mtd: nand: split extended ID " Brian Norris
2012-09-25  3:40 ` [PATCH 5/9] mtd: nand: split simple ID decode " Brian Norris
2012-09-25  3:40 ` [PATCH 6/9] mtd: nand: add generic READ ID length calculation functions Brian Norris
2012-09-25  3:40 ` [PATCH 7/9] mtd: nand: increase max OOB size to 640 Brian Norris
2012-09-25  3:40 ` [PATCH 8/9] mtd: nand: decode Hynix MLC, 6-byte ID length Brian Norris
2012-09-25  3:40 ` [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID Brian Norris
2012-10-05  1:37   ` Marek Vasut
2012-10-06 20:24     ` Brian Norris
2012-10-07  3:29       ` Marek Vasut
2012-09-27 12:23 ` [PATCH 0/9] mtd: nand: improve NAND ID detection Artem Bityutskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox