Skip to contentSkip to conversation

mtd: nand: improve NAND ID detection

PATCH In review Brian Norris 3 participants 14 messages Sep 25, 2012 – Oct 7, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover mtd: nand: improve NAND ID detection

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(-)
Signature · 1 line
1.7.11.3
#1 lore raw

PATCH 1/9 mtd: nand: remove unnecessary variable

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

---
 drivers/mtd/nand/nand_base.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/mtd/nand/nand_base.c +1 -3
index 88f671c..30588eb 100644
@@ -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;
 	}
 
Signature · 1 line
1.7.11.3
#2 lore raw

PATCH 2/9 mtd: nand: remove redundant ID read

In reply to Brian Norris

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

---
 drivers/mtd/nand/nand_base.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/mtd/nand/nand_base.c +2 -8
index 30588eb..f725247 100644
@@ -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);
 
Signature · 1 line
1.7.11.3
#3 lore raw

PATCH 3/9 mtd: nand: split BB marker options decoding into its own function

In reply to Brian Norris

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.).

---
 drivers/mtd/nand/nand_base.c | 66 ++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 27 deletions(-)
drivers/mtd/nand/nand_base.c +39 -27
index f725247..c7fc330 100644
@@ -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;
Signature · 1 line
1.7.11.3
#4 lore raw

PATCH 4/9 mtd: nand: split extended ID decoding into its own function

In reply to Brian Norris

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.

---
 drivers/mtd/nand/nand_base.c | 122 ++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 55 deletions(-)
drivers/mtd/nand/nand_base.c +67 -55
index c7fc330..afbe1ce 100644
@@ -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.
Signature · 1 line
1.7.11.3
#5 lore raw

PATCH 5/9 mtd: nand: split simple ID decode into its own function

In reply to Brian Norris

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.

---
 drivers/mtd/nand/nand_base.c | 51 +++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 20 deletions(-)
drivers/mtd/nand/nand_base.c +31 -20
index afbe1ce..380f783 100644
@@ -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;
Signature · 1 line
1.7.11.3
#6 lore raw

PATCH 6/9 mtd: nand: add generic READ ID length calculation functions

In reply to Brian Norris

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".

---
 drivers/mtd/nand/nand_base.c | 72 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 65 insertions(+), 7 deletions(-)
drivers/mtd/nand/nand_base.c +65 -7
index 380f783..fd5c307 100644
@@ -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;
Signature · 1 line
1.7.11.3
#7 lore raw

PATCH 7/9 mtd: nand: increase max OOB size to 640

In reply to Brian Norris

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

---
 include/linux/mtd/nand.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/mtd/nand.h +1 -1
index ce22824..817ff5a 100644
@@ -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
 
 /*
Signature · 1 line
1.7.11.3
#8 lore raw

PATCH 8/9 mtd: nand: decode Hynix MLC, 6-byte ID length

In reply to Brian Norris

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

---
 drivers/mtd/nand/nand_base.c | 45 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)
drivers/mtd/nand/nand_base.c +44 -1
index fd5c307..7e93d0d 100644
@@ -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);
Signature · 1 line
1.7.11.3
#9 lore raw

PATCH 9/9 mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID

In reply to Brian Norris

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).

---
 drivers/mtd/nand/nand_base.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/mtd/nand/nand_base.c +11 -5
index 7e93d0d..bcb58ce 100644
@@ -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 */
Signature · 1 line
1.7.11.3
#10 lore raw

Re: [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID

Dear Brian Norris,

Quoted 2 lines

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

Quoted 9 lines
drivers/mtd/nand/nand_base.c +0 -0
index 7e93d0d..bcb58ce 100644
@@ -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)

drivers/mtd/nand/nand_base.c static void nand_decode_ext_id(struct mtd_info · line 2948

Quoted 10 lines
drivers/mtd/nand/nand_base.c +2 -3
-	 * 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

#11 lore raw

Re: [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID

Hi Marek,

Quoted 4 lines · Marek Vasut

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.

Quoted 21 lines
Quoted 19 lines
drivers/mtd/nand/nand_base.c +2 -3
index 7e93d0d..bcb58ce 100644
@@ -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) {

[...]

Quoted 1 line

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

---

drivers/mtd/nand/nand_base.c +0 -0
index ec6841d..fa1ae76 100644
@@ -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;

Tested-by
#12 lore raw

Re: [PATCH 9/9] mtd: nand: detect Samsung K9GBG08U0A, K9GAG08U0F ID

Dear Brian Norris,

Quoted 11 lines

Hi Marek,

Quoted 4 lines · Marek Vasut

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

Quoted 36 lines
Quoted 19 lines
Quoted 19 lines
drivers/mtd/nand/nand_base.c +1 -1
index 7e93d0d..bcb58ce 100644
@@ -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.
       */
 
Quoted 7 lines

drivers/mtd/nand/nand_base.c static void nand_decode_ext_id(struct mtd_info · line 2952

Quoted 3 lines
drivers/mtd/nand/nand_base.c +1 -2
-     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:

Quoted 1 line

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

Nice!

Quoted 12 lines

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

mtd: nand: add generic READ ID length calculation functions

(quoted diff) +2 -4
-       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)) {
Quoted 3 lines

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.

Quoted 24 lines

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

---

drivers/mtd/nand/nand_base.c +2 -1
index ec6841d..fa1ae76 100644
@@ -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;

#13 lore raw
In reply to Brian Norris
Quoted 5 lines · Brian Norris

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!

Signature · 2 lines
Best Regards,
Artem Bityutskiy