[PATCH v6 16/17] mtd: nand: hynix: Rework NAND ID decoding to extract more information

[PATCH v6 16/17] mtd: nand: hynix: Rework NAND ID decoding to extract more information

[Boris Brezillon]

The current NAND ID detection in nand_hynix.c is not handling the
different scheme used by Hynix, thus forcing developers to add new
entries in the nand_ids table each time they want to support a new MLC
NAND.

Enhance the detection logic to handle all known formats. This does not
necessarily mean we are handling all the cases, but if new formats are
discovered, the code should evolve to take them into account instead of
adding more full-id entries in the nand_ids table.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Richard Weinberger <richard@nod.at>
---
 drivers/mtd/nand/nand_hynix.c | 228 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 209 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/nand_hynix.c b/drivers/mtd/nand/nand_hynix.c
index fbd661688158..92b8e435776c 100644
--- a/drivers/mtd/nand/nand_hynix.c
+++ b/drivers/mtd/nand/nand_hynix.c
@@ -16,21 +16,56 @@
  */
 
 #include <linux/mtd/nand.h>
+#include <linux/sizes.h>
 
-static void hynix_nand_decode_id(struct nand_chip *chip)
+static bool hynix_nand_has_valid_jedecid(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 jedecid[6] = { };
+	int i = 0;
+
+	chip->cmdfunc(mtd, NAND_CMD_READID, 0x40, -1);
+	for (i = 0; i < 5; i++)
+		jedecid[i] = chip->read_byte(mtd);
 
-	/* Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22) */
-	if (chip->id.len == 6 && !nand_is_slc(chip)) {
-		u8 tmp, extid = chip->id.data[3];
+	return !strcmp("JEDEC", jedecid);
+}
 
-		/* Extract pagesize */
-		mtd->writesize = 2048 << (extid & 0x03);
-		extid >>= 2;
+static void hynix_nand_extract_oobsize(struct nand_chip *chip,
+				       bool valid_jedecid)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 oobsize;
 
-		/* Extract oobsize */
-		switch (((extid >> 2) & 0x4) | (extid & 0x3)) {
+	oobsize = ((chip->id.data[3] >> 2) & 0x3) |
+		  ((chip->id.data[3] >> 4) & 0x4);
+
+	if (valid_jedecid) {
+		switch (oobsize) {
+		case 0:
+			mtd->oobsize = 2048;
+			break;
+		case 1:
+			mtd->oobsize = 1664;
+			break;
+		case 2:
+			mtd->oobsize = 1024;
+			break;
+		case 3:
+			mtd->oobsize = 640;
+			break;
+		default:
+			/*
+			 * We should never reach this case, but if that
+			 * happens, this probably means Hynix decided to use
+			 * a different extended ID format, and we should find
+			 * a way to support it.
+			 */
+			WARN(1, "Invalid OOB size");
+			break;
+		}
+	} else {
+		switch (oobsize) {
 		case 0:
 			mtd->oobsize = 128;
 			break;
@@ -49,23 +84,178 @@ static void hynix_nand_decode_id(struct nand_chip *chip)
 		case 5:
 			mtd->oobsize = 16;
 			break;
-		default:
+		case 6:
 			mtd->oobsize = 640;
 			break;
+		default:
+			/*
+			 * We should never reach this case, but if that
+			 * happens, this probably means Hynix decided to use
+			 * a different extended ID format, and we should find
+			 * a way to support it.
+			 */
+			WARN(1, "Invalid OOB size");
+			break;
 		}
+	}
+}
+
+static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
+						bool valid_jedecid)
+{
+	u8 ecc_level = (chip->id.data[4] >> 4) & 0x7;
+
+	if (valid_jedecid) {
+		/* Reference: H27UCG8T2E datasheet */
+		chip->ecc_step_ds = 1024;
 
-		/* Extract blocksize */
-		extid >>= 2;
-		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;
+		switch (ecc_level) {
+		case 0:
+			chip->ecc_step_ds = 0;
+			chip->ecc_strength_ds = 0;
+			break;
+		case 1:
+			chip->ecc_strength_ds = 4;
+			break;
+		case 2:
+			chip->ecc_strength_ds = 24;
+			break;
+		case 3:
+			chip->ecc_strength_ds = 32;
+			break;
+		case 4:
+			chip->ecc_strength_ds = 40;
+			break;
+		case 5:
+			chip->ecc_strength_ds = 50;
+			break;
+		case 6:
+			chip->ecc_strength_ds = 60;
+			break;
+		default:
+			/*
+			 * We should never reach this case, but if that
+			 * happens, this probably means Hynix decided to use
+			 * a different extended ID format, and we should find
+			 * a way to support it.
+			 */
+			WARN(1, "Invalid ECC requirements");
+		}
+	} else {
+		/*
+		 * The ECC requirements field meaning depends on the
+		 * NAND technology.
+		 */
+		u8 nand_tech = chip->id.data[5] & 0x3;
+
+		if (nand_tech < 3) {
+			/* > 26nm, reference: H27UBG8T2A datasheet */
+			if (ecc_level < 5) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1 << ecc_level;
+			} else if (ecc_level < 7) {
+				if (ecc_level == 5)
+					chip->ecc_step_ds = 2048;
+				else
+					chip->ecc_step_ds = 2048;
+				chip->ecc_strength_ds = 24;
+			} else {
+				/*
+				 * We should never reach this case, but if that
+				 * happens, this probably means Hynix decided
+				 * to use a different extended ID format, and
+				 * we should find a way to support it.
+				 */
+				WARN(1, "Invalid ECC requirements");
+			}
+		} else {
+			/* <= 26nm, reference: H27UBG8T2B datasheet */
+			if (!ecc_level) {
+				chip->ecc_step_ds = 0;
+				chip->ecc_strength_ds = 0;
+			} else if (ecc_level < 5) {
+				chip->ecc_step_ds = 512;
+				chip->ecc_strength_ds = 1 << (ecc_level - 1);
+			} else {
+				chip->ecc_step_ds = 1024;
+				chip->ecc_strength_ds = 24 +
+							(8 * (ecc_level - 5));
+			}
+		}
+	}
+}
+
+static void hynix_nand_extract_scrambling_requirements(struct nand_chip *chip,
+						       bool valid_jedecid)
+{
+	u8 nand_tech;
+
+	/* We need scrambling on all TLC NANDs*/
+	if (chip->bits_per_cell > 2)
+		chip->options |= NAND_NEED_SCRAMBLING;
+
+	/* And on MLC NANDs with sub-3xnm process */
+	if (valid_jedecid) {
+		nand_tech = chip->id.data[5] >> 4;
+
+		/* < 3xnm */
+		if (nand_tech > 0)
+			chip->options |= NAND_NEED_SCRAMBLING;
 	} else {
+		nand_tech = chip->id.data[5] & 0x3;
+
+		/* < 32nm */
+		if (nand_tech > 2)
+			chip->options |= NAND_NEED_SCRAMBLING;
+	}
+}
+
+static void hynix_nand_decode_id(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	bool valid_jedecid;
+	u8 tmp;
+
+	/*
+	 * Exclude all SLC NANDs from this advanced detection scheme.
+	 * According to the ranges defined in several datasheets, it might
+	 * appear that even SLC NANDs could fall in this extended ID scheme.
+	 * If that the case rework the test to let SLC NANDs go through the
+	 * detection process.
+	 */
+	if (chip->id.len < 6 || nand_is_slc(chip)) {
 		nand_decode_ext_id(chip);
+		return;
 	}
+
+	/* Extract pagesize */
+	mtd->writesize = 2048 << (chip->id.data[3] & 0x03);
+
+	tmp = (chip->id.data[3] >> 4) & 0x3;
+	/*
+	 * When bit7 is set that means we start counting at 1MiB, otherwise
+	 * we start counting at 128KiB and shift this value the content of
+	 * ID[3][4:5].
+	 * The only exception is when ID[3][4:5] == 3 and ID[3][7] == 0, in
+	 * this case the erasesize is set to 768KiB.
+	 */
+	if (chip->id.data[3] & 0x80)
+		mtd->erasesize = SZ_1M << tmp;
+	else if (tmp == 3)
+		mtd->erasesize = SZ_512K + SZ_256K;
+	else
+		mtd->erasesize = SZ_128K << tmp;
+
+	/*
+	 * Modern Toggle DDR NANDs have a valid JEDECID even though they are
+	 * not exposing a valid JEDEC parameter table.
+	 * These NANDs use a different NAND ID scheme.
+	 */
+	valid_jedecid = hynix_nand_has_valid_jedecid(chip);
+
+	hynix_nand_extract_oobsize(chip, valid_jedecid);
+	hynix_nand_extract_ecc_requirements(chip, valid_jedecid);
+	hynix_nand_extract_scrambling_requirements(chip, valid_jedecid);
 }
 
 static int hynix_nand_init(struct nand_chip *chip)
-- 
2.7.4

Re: [PATCH v6 16/17] mtd: nand: hynix: Rework NAND ID decoding to extract more information

[Boris Brezillon]

Hi Julia,

On Mon, 9 Jan 2017 21:21:27 +0100 (CET)
Julia Lawall <julia.lawall@lip6.fr> wrote:

> It looks odd that lines 158 and 160 are the same.

Indeed, it should be 'chip->ecc_step_ds = 1024;' in the else branch.
I'll fix that.

Thanks,

Boris

> 
> julia
> 
> ---------- Forwarded message ----------
> 
> In-Reply-To: <1483956264-3335-17-git-send-email-boris.brezillon@free-electrons.com>
> 
> Hi Boris,
> 
> [auto build test WARNING on mtd/master]
> [also build test WARNING on v4.10-rc3 next-20170106]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Boris-Brezillon/mtd-nand-allow-vendor-specific-detection-initialization/20170110-022221
> base:   git://git.infradead.org/linux-mtd.git master
> :::::: branch date: 2 hours ago
> :::::: commit date: 2 hours ago
> 
> >> drivers/mtd/nand/nand_hynix.c:157:4-6: WARNING: possible condition with no effect (if == else)  
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout ee58e9ecc231f4a40ff46fe95078d7796ebe430b
> vim +157 drivers/mtd/nand/nand_hynix.c
> 
> ee58e9ec Boris Brezillon 2017-01-09  141  			 */
> ee58e9ec Boris Brezillon 2017-01-09  142  			WARN(1, "Invalid ECC requirements");
> 1065fa22 Boris Brezillon 2017-01-09  143  		}
> ee58e9ec Boris Brezillon 2017-01-09  144  	} else {
> ee58e9ec Boris Brezillon 2017-01-09  145  		/*
> ee58e9ec Boris Brezillon 2017-01-09  146  		 * The ECC requirements field meaning depends on the
> ee58e9ec Boris Brezillon 2017-01-09  147  		 * NAND technology.
> ee58e9ec Boris Brezillon 2017-01-09  148  		 */
> ee58e9ec Boris Brezillon 2017-01-09  149  		u8 nand_tech = chip->id.data[5] & 0x3;
> 1065fa22 Boris Brezillon 2017-01-09  150
> ee58e9ec Boris Brezillon 2017-01-09  151  		if (nand_tech < 3) {
> ee58e9ec Boris Brezillon 2017-01-09  152  			/* > 26nm, reference: H27UBG8T2A datasheet */
> ee58e9ec Boris Brezillon 2017-01-09  153  			if (ecc_level < 5) {
> ee58e9ec Boris Brezillon 2017-01-09  154  				chip->ecc_step_ds = 512;
> ee58e9ec Boris Brezillon 2017-01-09  155  				chip->ecc_strength_ds = 1 << ecc_level;
> ee58e9ec Boris Brezillon 2017-01-09  156  			} else if (ecc_level < 7) {
> ee58e9ec Boris Brezillon 2017-01-09 @157  				if (ecc_level == 5)
> ee58e9ec Boris Brezillon 2017-01-09  158  					chip->ecc_step_ds = 2048;
> 1065fa22 Boris Brezillon 2017-01-09  159  				else
> ee58e9ec Boris Brezillon 2017-01-09  160  					chip->ecc_step_ds = 2048;
> ee58e9ec Boris Brezillon 2017-01-09  161  				chip->ecc_strength_ds = 24;
> ee58e9ec Boris Brezillon 2017-01-09  162  			} else {
> ee58e9ec Boris Brezillon 2017-01-09  163  				/*
> ee58e9ec Boris Brezillon 2017-01-09  164  				 * We should never reach this case, but if that
> ee58e9ec Boris Brezillon 2017-01-09  165  				 * happens, this probably means Hynix decided
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation