linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization
@ 2016-06-08 13:00 Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 01/15] mtd: nand: get rid of the mtd parameter in all auto-detection functions Boris Brezillon
                   ` (15 more replies)
  0 siblings, 16 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Hello,

This patch series is a step forward in supporting vendor-specific
functionalities.
This series is mainly moving vendor-specific initialization or
detection code out of the core, but also introduces an infrastructure
allowing support for vendor-specific features.

While those features might seem useless to most users, some of them are
actually required on modern MLC/TLC NANDs (this is the case of read-retry
support, which AFAICT has not been standardized by the JEDEC consortium).

Now, let's detail what's inside this patch-set.

Patches 1 to 4 are simple reworks simplifying auto-detection function
prototypes, and clarifying their purpose.

Patch 5 is introducing the vendor-specific initialization
infrastructure.

Patch 6 is removing the MTD_NAND_IDS Kconfig option to avoid creating
a nand_ids.ko module when MTD_NAND is enabled as a module. This prevents
a future cross-dependency between nand.ko where all vendor specific
code will rely and nand_ids.ko which will reference vendor-specific ops
in its manufacturer table, which in turn is referenced by the core code
linked in nand.ko.

Patches 7 to 12 are moving vendor-specific code into their respective
nand_<vendor>.c files.

Patch 13 is taking a patch proposed by Hans and adding support for ECC
requirements extraction from the samsung extended IDs. It seems to apply
to all Samsung MLCs, but even if it's not the case, the detection code
should be improved to support the new formats.

Patch 14 is adding support for advanced NAND ID decoding to the Hynix
driver (OOB size, ECC and scrambling requirements extraction). Again
this detection code might be incomplete, but I'd like people to extend
it if required rather than adding new full-id entries in the nand_ids
table.

And finally, patch 15 is showing how useful this vendor-specific stuff
can be by implementing read-retry support for Hynix 1x nm MLCs. And
trust me, you don't want to try using such a NAND without read-retry
support ;).

As always, I'm open to any suggestion to improve this vendor-specific
infrastructure, so please review the code :).

Thanks,

Boris

Changes since v1:
- split detection and initialization steps to avoid keeping
  information retrieved by nand_decode_ext_id() if it's not
  appropriate (Aleksei reported a bug where NAND_BUSWIDTH_16
  was set by nand_decode_ext_id() and not cleared by the
  samsung ->init() function).
  The new approach is to call ->detect() if it's provided and
  fallback to nand_decode_ext_id() if it's not. ->detect()
  implementation should can call nand_decode_ext_id() if needed.

Boris Brezillon (14):
  mtd: nand: get rid of the mtd parameter in all auto-detection
    functions
  mtd: nand: store nand ID in struct nand_chip
  mtd: nand: get rid of busw parameter
  mtd: nand: rename nand_get_flash_type() into nand_detect()
  mtd: nand: add manufacturer specific initialization/detection steps
  mtd: nand: kill the MTD_NAND_IDS Kconfig option
  mtd: nand: move Samsung specific init/detection logic in
    nand_samsung.c
  mtd: nand: move Hynix specific init/detection logic in nand_hynix.c
  mtd: nand: move Toshiba specific init/detection logic in
    nand_toshiba.c
  mtd: nand: move Micron specific init logic in nand_micron.c
  mtd: nand: move AMD/Spansion specific init/detection logic in
    nand_amd.c
  mtd: nand: move Macronix specific initialization in nand_macronix.c
  mtd: nand: hynix: rework NAND ID decoding to extract more information
  mtd: nand: hynix: add read-retry support for 1x nm MLC NANDs

Hans de Goede (1):
  mtd: nand: samsung: retrieve ECC requirements from extended ID

 arch/cris/arch-v32/drivers/Kconfig |   1 -
 drivers/mtd/nand/Kconfig           |   4 -
 drivers/mtd/nand/Makefile          |   9 +-
 drivers/mtd/nand/nand_amd.c        |  48 +++
 drivers/mtd/nand/nand_base.c       | 372 +++++++---------------
 drivers/mtd/nand/nand_hynix.c      | 629 +++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/nand_ids.c        |  21 +-
 drivers/mtd/nand/nand_macronix.c   |  27 ++
 drivers/mtd/nand/nand_micron.c     |  84 +++++
 drivers/mtd/nand/nand_samsung.c    | 109 +++++++
 drivers/mtd/nand/nand_toshiba.c    |  48 +++
 include/linux/mtd/nand.h           |  69 ++--
 12 files changed, 1132 insertions(+), 289 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_amd.c
 create mode 100644 drivers/mtd/nand/nand_hynix.c
 create mode 100644 drivers/mtd/nand/nand_macronix.c
 create mode 100644 drivers/mtd/nand/nand_micron.c
 create mode 100644 drivers/mtd/nand/nand_samsung.c
 create mode 100644 drivers/mtd/nand/nand_toshiba.c

-- 
2.7.4

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

* [PATCH v2 01/15] mtd: nand: get rid of the mtd parameter in all auto-detection functions
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 02/15] mtd: nand: store nand ID in struct nand_chip Boris Brezillon
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Now that struct nand_chip embeds an mtd_info object we can get rid of the
mtd parameter and extract it from the chip parameter with the nand_to_mtd()
helper.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 56 ++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7bc37b4..c0d8c43 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3228,9 +3228,10 @@ static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
 }
 
 /* Parse the Extended Parameter Page. */
-static int nand_flash_detect_ext_param_page(struct mtd_info *mtd,
-		struct nand_chip *chip, struct nand_onfi_params *p)
+static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
+					    struct nand_onfi_params *p)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct onfi_ext_param_page *ep;
 	struct onfi_ext_section *s;
 	struct onfi_ext_ecc_info *ecc;
@@ -3325,9 +3326,9 @@ static void nand_onfi_detect_micron(struct nand_chip *chip,
 /*
  * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
  */
-static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
-					int *busw)
+static int nand_flash_detect_onfi(struct nand_chip *chip, int *busw)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_onfi_params *p = &chip->onfi_params;
 	int i, j;
 	int val;
@@ -3414,7 +3415,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 			chip->cmdfunc = nand_command_lp;
 
 		/* The Extended Parameter Page is supported since ONFI 2.1. */
-		if (nand_flash_detect_ext_param_page(mtd, chip, p))
+		if (nand_flash_detect_ext_param_page(chip, p))
 			pr_warn("Failed to detect ONFI extended param page\n");
 	} else {
 		pr_warn("Could not retrieve ONFI ECC requirements\n");
@@ -3429,9 +3430,9 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 /*
  * Check if the NAND chip is JEDEC compliant, returns 1 if it is, 0 otherwise.
  */
-static int nand_flash_detect_jedec(struct mtd_info *mtd, struct nand_chip *chip,
-					int *busw)
+static int nand_flash_detect_jedec(struct nand_chip *chip, int *busw)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_jedec_params *p = &chip->jedec_params;
 	struct jedec_ecc_info *ecc;
 	int val;
@@ -3581,9 +3582,10 @@ static int nand_get_bits_per_cell(u8 cellinfo)
  * 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)
+static void nand_decode_ext_id(struct nand_chip *chip, u8 id_data[8],
+			       int *busw)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int extid, id_len;
 	/* The 3rd id byte holds MLC / multichip data */
 	chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
@@ -3714,10 +3716,10 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
  * 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)
+static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type,
+			   u8 id_data[8], int *busw)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int maf_id = id_data[0];
 
 	mtd->erasesize = type->erasesize;
@@ -3747,9 +3749,9 @@ static void nand_decode_id(struct mtd_info *mtd, struct nand_chip *chip,
  * 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])
+static void nand_decode_bbm_options(struct nand_chip *chip, u8 id_data[8])
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int maf_id = id_data[0];
 
 	/* Set the bad block position */
@@ -3784,9 +3786,12 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
 	return type->id_len;
 }
 
-static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
-		   struct nand_flash_dev *type, u8 *id_data, int *busw)
+static bool find_full_id_nand(struct nand_chip *chip,
+			      struct nand_flash_dev *type, u8 *id_data,
+			      int *busw)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
 	if (!strncmp(type->id, id_data, type->id_len)) {
 		mtd->writesize = type->pagesize;
 		mtd->erasesize = type->erasesize;
@@ -3813,11 +3818,11 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 /*
  * 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,
-						  struct nand_chip *chip,
+static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 						  int *maf_id, int *dev_id,
 						  struct nand_flash_dev *type)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	int busw;
 	int i, maf_idx;
 	u8 id_data[8];
@@ -3862,7 +3867,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 
 	for (; type->name != NULL; type++) {
 		if (is_full_id_nand(type)) {
-			if (find_full_id_nand(mtd, chip, type, id_data, &busw))
+			if (find_full_id_nand(chip, type, id_data, &busw))
 				goto ident_done;
 		} else if (*dev_id == type->dev_id) {
 			break;
@@ -3872,11 +3877,11 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 	chip->onfi_version = 0;
 	if (!type->name || !type->pagesize) {
 		/* Check if the chip is ONFI compliant */
-		if (nand_flash_detect_onfi(mtd, chip, &busw))
+		if (nand_flash_detect_onfi(chip, &busw))
 			goto ident_done;
 
 		/* Check if the chip is JEDEC compliant */
-		if (nand_flash_detect_jedec(mtd, chip, &busw))
+		if (nand_flash_detect_jedec(chip, &busw))
 			goto ident_done;
 	}
 
@@ -3890,9 +3895,9 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 
 	if (!type->pagesize) {
 		/* Decode parameters from extended ID */
-		nand_decode_ext_id(mtd, chip, id_data, &busw);
+		nand_decode_ext_id(chip, id_data, &busw);
 	} else {
-		nand_decode_id(mtd, chip, type, id_data, &busw);
+		nand_decode_id(chip, type, id_data, &busw);
 	}
 	/* Get chip options */
 	chip->options |= type->options;
@@ -3929,7 +3934,7 @@ ident_done:
 		return ERR_PTR(-EINVAL);
 	}
 
-	nand_decode_bbm_options(mtd, chip, id_data);
+	nand_decode_bbm_options(chip, id_data);
 
 	/* Calculate the address shift from the page size */
 	chip->page_shift = ffs(mtd->writesize) - 1;
@@ -4140,8 +4145,7 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 	nand_set_defaults(chip, chip->options & NAND_BUSWIDTH_16);
 
 	/* Read the flash type */
-	type = nand_get_flash_type(mtd, chip, &nand_maf_id,
-				   &nand_dev_id, table);
+	type = nand_get_flash_type(chip, &nand_maf_id, &nand_dev_id, table);
 
 	if (IS_ERR(type)) {
 		if (!(chip->options & NAND_SCAN_SILENT_NODEV))
-- 
2.7.4

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

* [PATCH v2 02/15] mtd: nand: store nand ID in struct nand_chip
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 01/15] mtd: nand: get rid of the mtd parameter in all auto-detection functions Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 03/15] mtd: nand: get rid of busw parameter Boris Brezillon
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Store the NAND ID in struct nand_chip to avoid passing id_data and id_len
as function parameters.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 55 ++++++++++++++++++++++++--------------------
 include/linux/mtd/nand.h     | 13 +++++++++++
 2 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c0d8c43..4959263 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3582,18 +3582,16 @@ static int nand_get_bits_per_cell(u8 cellinfo)
  * 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 nand_chip *chip, u8 id_data[8],
-			       int *busw)
+static void nand_decode_ext_id(struct nand_chip *chip, int *busw)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	int extid, id_len;
+	int extid, id_len = chip->id.len;
+	u8 *id_data = chip->id.data;
 	/* The 3rd id byte holds MLC / multichip data */
 	chip->bits_per_cell = nand_get_bits_per_cell(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)
@@ -3717,9 +3715,10 @@ static void nand_decode_ext_id(struct nand_chip *chip, u8 id_data[8],
  * the chip.
  */
 static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type,
-			   u8 id_data[8], int *busw)
+			   int *busw)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 *id_data = chip->id.data;
 	int maf_id = id_data[0];
 
 	mtd->erasesize = type->erasesize;
@@ -3749,9 +3748,10 @@ static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type,
  * heuristic patterns using various detected parameters (e.g., manufacturer,
  * page size, cell-type information).
  */
-static void nand_decode_bbm_options(struct nand_chip *chip, u8 id_data[8])
+static void nand_decode_bbm_options(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 *id_data = chip->id.data;
 	int maf_id = id_data[0];
 
 	/* Set the bad block position */
@@ -3787,10 +3787,10 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
 }
 
 static bool find_full_id_nand(struct nand_chip *chip,
-			      struct nand_flash_dev *type, u8 *id_data,
-			      int *busw)
+			      struct nand_flash_dev *type, int *busw)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 *id_data = chip->id.data;
 
 	if (!strncmp(type->id, id_data, type->id_len)) {
 		mtd->writesize = type->pagesize;
@@ -3819,13 +3819,13 @@ static bool find_full_id_nand(struct nand_chip *chip,
  * Get the flash and manufacturer id and lookup if the type is supported.
  */
 static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
-						  int *maf_id, int *dev_id,
 						  struct nand_flash_dev *type)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int busw;
 	int i, maf_idx;
-	u8 id_data[8];
+	u8 *id_data = chip->id.data;
+	u8 maf_id, dev_id;
 
 	/* Select the device */
 	chip->select_chip(mtd, 0);
@@ -3840,8 +3840,8 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	chip->cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
 
 	/* Read manufacturer and device IDs */
-	*maf_id = chip->read_byte(mtd);
-	*dev_id = chip->read_byte(mtd);
+	maf_id = chip->read_byte(mtd);
+	dev_id = chip->read_byte(mtd);
 
 	/*
 	 * Try again to make sure, as some systems the bus-hold or other
@@ -3856,20 +3856,22 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	for (i = 0; i < 8; i++)
 		id_data[i] = chip->read_byte(mtd);
 
-	if (id_data[0] != *maf_id || id_data[1] != *dev_id) {
+	if (id_data[0] != maf_id || id_data[1] != dev_id) {
 		pr_info("second ID read did not match %02x,%02x against %02x,%02x\n",
-			*maf_id, *dev_id, id_data[0], id_data[1]);
+			maf_id, dev_id, id_data[0], id_data[1]);
 		return ERR_PTR(-ENODEV);
 	}
 
+	chip->id.len = nand_id_len(id_data, 8);
+
 	if (!type)
 		type = nand_flash_ids;
 
 	for (; type->name != NULL; type++) {
 		if (is_full_id_nand(type)) {
-			if (find_full_id_nand(chip, type, id_data, &busw))
+			if (find_full_id_nand(chip, type, &busw))
 				goto ident_done;
-		} else if (*dev_id == type->dev_id) {
+		} else if (dev_id == type->dev_id) {
 			break;
 		}
 	}
@@ -3895,9 +3897,9 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 
 	if (!type->pagesize) {
 		/* Decode parameters from extended ID */
-		nand_decode_ext_id(chip, id_data, &busw);
+		nand_decode_ext_id(chip, &busw);
 	} else {
-		nand_decode_id(chip, type, id_data, &busw);
+		nand_decode_id(chip, type, &busw);
 	}
 	/* Get chip options */
 	chip->options |= type->options;
@@ -3906,13 +3908,13 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	 * Check if chip is not a Samsung device. Do not clear the
 	 * options for chips which do not have an extended id.
 	 */
-	if (*maf_id != NAND_MFR_SAMSUNG && !type->pagesize)
+	if (maf_id != NAND_MFR_SAMSUNG && !type->pagesize)
 		chip->options &= ~NAND_SAMSUNG_LP_OPTIONS;
 ident_done:
 
 	/* Try to identify manufacturer */
 	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
-		if (nand_manuf_ids[maf_idx].id == *maf_id)
+		if (nand_manuf_ids[maf_idx].id == maf_id)
 			break;
 	}
 
@@ -3926,7 +3928,7 @@ ident_done:
 		 * chip correct!
 		 */
 		pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-			*maf_id, *dev_id);
+			maf_id, dev_id);
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name, mtd->name);
 		pr_warn("bus width %d instead %d bit\n",
 			   (chip->options & NAND_BUSWIDTH_16) ? 16 : 8,
@@ -3934,7 +3936,7 @@ ident_done:
 		return ERR_PTR(-EINVAL);
 	}
 
-	nand_decode_bbm_options(chip, id_data);
+	nand_decode_bbm_options(chip);
 
 	/* Calculate the address shift from the page size */
 	chip->page_shift = ffs(mtd->writesize) - 1;
@@ -3958,7 +3960,7 @@ ident_done:
 		chip->cmdfunc = nand_command_lp;
 
 	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-		*maf_id, *dev_id);
+		maf_id, dev_id);
 
 	if (chip->onfi_version)
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
@@ -4145,7 +4147,7 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 	nand_set_defaults(chip, chip->options & NAND_BUSWIDTH_16);
 
 	/* Read the flash type */
-	type = nand_get_flash_type(chip, &nand_maf_id, &nand_dev_id, table);
+	type = nand_get_flash_type(chip, table);
 
 	if (IS_ERR(type)) {
 		if (!(chip->options & NAND_SCAN_SILENT_NODEV))
@@ -4154,6 +4156,9 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 		return PTR_ERR(type);
 	}
 
+	nand_maf_id = chip->id.data[0];
+	nand_dev_id = chip->id.data[1];
+
 	chip->select_chip(mtd, -1);
 
 	/* Check for a chip array */
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index fbe8e16..3072f5e 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -447,6 +447,17 @@ struct nand_jedec_params {
 } __packed;
 
 /**
+ * struct nand_id - NAND id structure
+ * @data: buffer containing the id bytes. Currently 8 bytes large, but can
+ *	  be extended if required.
+ * @len: ID length.
+ */
+struct nand_id {
+	u8 data[8];
+	int len;
+};
+
+/**
  * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices
  * @lock:               protection lock
  * @active:		the mtd device which holds the controller currently
@@ -639,6 +650,7 @@ struct nand_buffers {
  * @pagebuf_bitflips:	[INTERN] holds the bitflip count for the page which is
  *			currently in data_buf.
  * @subpagesize:	[INTERN] holds the subpagesize
+ * @id:			[INTERN] holds NAND ID
  * @onfi_version:	[INTERN] holds the chip ONFI version (BCD encoded),
  *			non 0 if ONFI supported.
  * @jedec_version:	[INTERN] holds the chip JEDEC version (BCD encoded),
@@ -718,6 +730,7 @@ struct nand_chip {
 	int badblockpos;
 	int badblockbits;
 
+	struct nand_id id;
 	int onfi_version;
 	int jedec_version;
 	union {
-- 
2.7.4

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

* [PATCH v2 03/15] mtd: nand: get rid of busw parameter
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 01/15] mtd: nand: get rid of the mtd parameter in all auto-detection functions Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 02/15] mtd: nand: store nand ID in struct nand_chip Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 04/15] mtd: nand: rename nand_get_flash_type() into nand_detect() Boris Brezillon
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Auto-detection functions are passed a busw parameter to retrieve the actual
NAND bus width and eventually set the correct value in chip->options.
Rework the nand_get_flash_type() function to get rid of this extra
parameter and let detection code directly set the NAND_BUSWIDTH_16 flag in
chip->options if needed.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 68 ++++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 4959263..1e865c0 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3148,8 +3148,10 @@ static void nand_shutdown(struct mtd_info *mtd)
 }
 
 /* Set default functions */
-static void nand_set_defaults(struct nand_chip *chip, int busw)
+static void nand_set_defaults(struct nand_chip *chip)
 {
+	unsigned int busw = chip->options & NAND_BUSWIDTH_16;
+
 	/* check for proper chip_delay setup, set 20us if not */
 	if (!chip->chip_delay)
 		chip->chip_delay = 20;
@@ -3326,7 +3328,7 @@ static void nand_onfi_detect_micron(struct nand_chip *chip,
 /*
  * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
  */
-static int nand_flash_detect_onfi(struct nand_chip *chip, int *busw)
+static int nand_flash_detect_onfi(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_onfi_params *p = &chip->onfi_params;
@@ -3395,9 +3397,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip, int *busw)
 	chip->bits_per_cell = p->bits_per_cell;
 
 	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
-		*busw = NAND_BUSWIDTH_16;
-	else
-		*busw = 0;
+		chip->options |= NAND_BUSWIDTH_16;
 
 	if (p->ecc_bits != 0xff) {
 		chip->ecc_strength_ds = p->ecc_bits;
@@ -3430,7 +3430,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip, int *busw)
 /*
  * Check if the NAND chip is JEDEC compliant, returns 1 if it is, 0 otherwise.
  */
-static int nand_flash_detect_jedec(struct nand_chip *chip, int *busw)
+static int nand_flash_detect_jedec(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_jedec_params *p = &chip->jedec_params;
@@ -3491,9 +3491,7 @@ static int nand_flash_detect_jedec(struct nand_chip *chip, int *busw)
 	chip->bits_per_cell = p->bits_per_cell;
 
 	if (jedec_feature(chip) & JEDEC_FEATURE_16_BIT_BUS)
-		*busw = NAND_BUSWIDTH_16;
-	else
-		*busw = 0;
+		chip->options |= NAND_BUSWIDTH_16;
 
 	/* ECC info */
 	ecc = &p->ecc_info[0];
@@ -3582,7 +3580,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
  * 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 nand_chip *chip, int *busw)
+static void nand_decode_ext_id(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int extid, id_len = chip->id.len;
@@ -3635,7 +3633,6 @@ static void nand_decode_ext_id(struct nand_chip *chip, int *busw)
 		/* Calc blocksize */
 		mtd->erasesize = (128 * 1024) <<
 			(((extid >> 1) & 0x04) | (extid & 0x03));
-		*busw = 0;
 	} else if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
 			!nand_is_slc(chip)) {
 		unsigned int tmp;
@@ -3676,7 +3673,6 @@ static void nand_decode_ext_id(struct nand_chip *chip, int *busw)
 			mtd->erasesize = 768 * 1024;
 		else
 			mtd->erasesize = (64 * 1024) << tmp;
-		*busw = 0;
 	} else {
 		/* Calc pagesize */
 		mtd->writesize = 1024 << (extid & 0x03);
@@ -3689,7 +3685,8 @@ static void nand_decode_ext_id(struct nand_chip *chip, int *busw)
 		mtd->erasesize = (64 * 1024) << (extid & 0x03);
 		extid >>= 2;
 		/* Get buswidth information */
-		*busw = (extid & 0x01) ? NAND_BUSWIDTH_16 : 0;
+		if (extid & 0x1)
+			chip->options |= NAND_BUSWIDTH_16;
 
 		/*
 		 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
@@ -3714,8 +3711,7 @@ static void nand_decode_ext_id(struct nand_chip *chip, int *busw)
  * decodes a matching ID table entry and assigns the MTD size parameters for
  * the chip.
  */
-static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type,
-			   int *busw)
+static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	u8 *id_data = chip->id.data;
@@ -3724,7 +3720,6 @@ static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type,
 	mtd->erasesize = type->erasesize;
 	mtd->writesize = type->pagesize;
 	mtd->oobsize = mtd->writesize / 32;
-	*busw = type->options & NAND_BUSWIDTH_16;
 
 	/* All legacy ID NAND are small-page, SLC */
 	chip->bits_per_cell = 1;
@@ -3787,7 +3782,7 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
 }
 
 static bool find_full_id_nand(struct nand_chip *chip,
-			      struct nand_flash_dev *type, int *busw)
+			      struct nand_flash_dev *type)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	u8 *id_data = chip->id.data;
@@ -3805,8 +3800,6 @@ static bool find_full_id_nand(struct nand_chip *chip,
 		chip->onfi_timing_mode_default =
 					type->onfi_timing_mode_default;
 
-		*busw = type->options & NAND_BUSWIDTH_16;
-
 		if (!mtd->name)
 			mtd->name = type->name;
 
@@ -3867,9 +3860,24 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	if (!type)
 		type = nand_flash_ids;
 
+	/*
+	 * Save the NAND_BUSWIDTH_16 flag before letting auto-detection logic
+	 * override it.
+	 * This is required to make sure initial NAND bus width set by the
+	 * NAND controller driver is coherent with the real NAND bus width
+	 * (extracted by auto-detection code).
+	 */
+	busw = chip->options & NAND_BUSWIDTH_16;
+
+	/*
+	 * The flag is only set (never cleared), reset it to its default value
+	 * before starting auto-detection.
+	 */
+	chip->options &= ~NAND_BUSWIDTH_16;
+
 	for (; type->name != NULL; type++) {
 		if (is_full_id_nand(type)) {
-			if (find_full_id_nand(chip, type, &busw))
+			if (find_full_id_nand(chip, type))
 				goto ident_done;
 		} else if (dev_id == type->dev_id) {
 			break;
@@ -3879,11 +3887,11 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	chip->onfi_version = 0;
 	if (!type->name || !type->pagesize) {
 		/* Check if the chip is ONFI compliant */
-		if (nand_flash_detect_onfi(chip, &busw))
+		if (nand_flash_detect_onfi(chip))
 			goto ident_done;
 
 		/* Check if the chip is JEDEC compliant */
-		if (nand_flash_detect_jedec(chip, &busw))
+		if (nand_flash_detect_jedec(chip))
 			goto ident_done;
 	}
 
@@ -3897,9 +3905,9 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 
 	if (!type->pagesize) {
 		/* Decode parameters from extended ID */
-		nand_decode_ext_id(chip, &busw);
+		nand_decode_ext_id(chip);
 	} else {
-		nand_decode_id(chip, type, &busw);
+		nand_decode_id(chip, type);
 	}
 	/* Get chip options */
 	chip->options |= type->options;
@@ -3919,9 +3927,8 @@ ident_done:
 	}
 
 	if (chip->options & NAND_BUSWIDTH_AUTO) {
-		WARN_ON(chip->options & NAND_BUSWIDTH_16);
-		chip->options |= busw;
-		nand_set_defaults(chip, busw);
+		WARN_ON(busw & NAND_BUSWIDTH_16);
+		nand_set_defaults(chip);
 	} else if (busw != (chip->options & NAND_BUSWIDTH_16)) {
 		/*
 		 * Check, if buswidth is correct. Hardware drivers should set
@@ -3930,9 +3937,8 @@ ident_done:
 		pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
 			maf_id, dev_id);
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name, mtd->name);
-		pr_warn("bus width %d instead %d bit\n",
-			   (chip->options & NAND_BUSWIDTH_16) ? 16 : 8,
-			   busw ? 16 : 8);
+		pr_warn("bus width %d instead %d bit\n", busw ? 16 : 8,
+			(chip->options & NAND_BUSWIDTH_16) ? 16 : 8);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -4144,7 +4150,7 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 		mtd->name = dev_name(mtd->dev.parent);
 
 	/* Set the default functions */
-	nand_set_defaults(chip, chip->options & NAND_BUSWIDTH_16);
+	nand_set_defaults(chip);
 
 	/* Read the flash type */
 	type = nand_get_flash_type(chip, table);
-- 
2.7.4

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

* [PATCH v2 04/15] mtd: nand: rename nand_get_flash_type() into nand_detect()
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (2 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 03/15] mtd: nand: get rid of busw parameter Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps Boris Brezillon
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

The only caller of nand_get_flash_type() (nand_scan_ident()) actually
don't use the returned nand_flash_dev pointer except for converting it to
to an error code.
Rename this function nand_detect() and make it return an integer.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 1e865c0..95e9a8e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3811,8 +3811,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
 /*
  * Get the flash and manufacturer id and lookup if the type is supported.
  */
-static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
-						  struct nand_flash_dev *type)
+static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int busw;
@@ -3852,7 +3851,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	if (id_data[0] != maf_id || id_data[1] != dev_id) {
 		pr_info("second ID read did not match %02x,%02x against %02x,%02x\n",
 			maf_id, dev_id, id_data[0], id_data[1]);
-		return ERR_PTR(-ENODEV);
+		return -ENODEV;
 	}
 
 	chip->id.len = nand_id_len(id_data, 8);
@@ -3896,7 +3895,7 @@ static struct nand_flash_dev *nand_get_flash_type(struct nand_chip *chip,
 	}
 
 	if (!type->name)
-		return ERR_PTR(-ENODEV);
+		return -ENODEV;
 
 	if (!mtd->name)
 		mtd->name = type->name;
@@ -3939,7 +3938,7 @@ ident_done:
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name, mtd->name);
 		pr_warn("bus width %d instead %d bit\n", busw ? 16 : 8,
 			(chip->options & NAND_BUSWIDTH_16) ? 16 : 8);
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 	}
 
 	nand_decode_bbm_options(chip);
@@ -3981,7 +3980,7 @@ ident_done:
 	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
 		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
 		mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
-	return type;
+	return 0;
 }
 
 static const char * const nand_ecc_modes[] = {
@@ -4139,7 +4138,6 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 {
 	int i, nand_maf_id, nand_dev_id;
 	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_flash_dev *type;
 	int ret;
 
 	ret = nand_dt_init(chip);
@@ -4153,13 +4151,12 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 	nand_set_defaults(chip);
 
 	/* Read the flash type */
-	type = nand_get_flash_type(chip, table);
-
-	if (IS_ERR(type)) {
+	ret = nand_detect(chip, table);
+	if (ret) {
 		if (!(chip->options & NAND_SCAN_SILENT_NODEV))
 			pr_warn("No NAND device found\n");
 		chip->select_chip(mtd, -1);
-		return PTR_ERR(type);
+		return ret;
 	}
 
 	nand_maf_id = chip->id.data[0];
-- 
2.7.4

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

* [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (3 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 04/15] mtd: nand: rename nand_get_flash_type() into nand_detect() Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-18  9:23   ` Richard Weinberger
  2016-06-08 13:00 ` [PATCH v2 06/15] mtd: nand: kill the MTD_NAND_IDS Kconfig option Boris Brezillon
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

A lot of NANDs are implementing generic features in a non-generic way,
or are providing advanced auto-detection logic where the NAND ID bytes
meaning changes with the NAND generation.

Providing this vendor specific initialization step will allow us to get
rid of the full ids in the nand_ids table or all the vendor specific
cases added over the time in the generic NAND ID decoding logic.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 42 ++++++++++++++++++++++++++++++++----------
 include/linux/mtd/nand.h     | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 95e9a8e..0a7d1c6 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3580,7 +3580,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
  * 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 nand_chip *chip)
+void nand_decode_ext_id(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int extid, id_len = chip->id.len;
@@ -3705,6 +3705,7 @@ static void nand_decode_ext_id(struct nand_chip *chip)
 
 	}
 }
+EXPORT_SYMBOL_GPL(nand_decode_ext_id);
 
 /*
  * Old devices have chip data hardcoded in the device ID table. nand_decode_id
@@ -3815,7 +3816,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int busw;
-	int i, maf_idx;
+	int i, maf_idx, ret;
 	u8 *id_data = chip->id.data;
 	u8 maf_id, dev_id;
 
@@ -3856,6 +3857,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 
 	chip->id.len = nand_id_len(id_data, 8);
 
+	/* Try to identify manufacturer */
+	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
+		if (nand_manuf_ids[maf_idx].id == maf_id)
+			break;
+	}
+
+	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
+
 	if (!type)
 		type = nand_flash_ids;
 
@@ -3903,8 +3912,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	chip->chipsize = (uint64_t)type->chipsize << 20;
 
 	if (!type->pagesize) {
-		/* Decode parameters from extended ID */
-		nand_decode_ext_id(chip);
+		/*
+		 * Try manufacturer detection if available and use
+		 * nand_decode_ext_id() otherwise.
+		 */
+		if (chip->manufacturer.ops->detect)
+			chip->manufacturer.ops->detect(chip);
+		else
+			nand_decode_ext_id(chip);
 	} else {
 		nand_decode_id(chip, type);
 	}
@@ -3919,12 +3934,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 		chip->options &= ~NAND_SAMSUNG_LP_OPTIONS;
 ident_done:
 
-	/* Try to identify manufacturer */
-	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
-		if (nand_manuf_ids[maf_idx].id == maf_id)
-			break;
-	}
-
 	if (chip->options & NAND_BUSWIDTH_AUTO) {
 		WARN_ON(busw & NAND_BUSWIDTH_16);
 		nand_set_defaults(chip);
@@ -3964,6 +3973,15 @@ ident_done:
 	if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
 		chip->cmdfunc = nand_command_lp;
 
+	/*
+	 * Manufacturer specific initialization.
+	 */
+	if (chip->manufacturer.ops && chip->manufacturer.ops->init) {
+		ret = chip->manufacturer.ops->init(chip);
+		if (ret)
+			return ret;
+	}
+
 	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
 		maf_id, dev_id);
 
@@ -4623,6 +4641,10 @@ void nand_release(struct mtd_info *mtd)
 	if (chip->badblock_pattern && chip->badblock_pattern->options
 			& NAND_BBT_DYNAMICSTRUCT)
 		kfree(chip->badblock_pattern);
+
+	/* Release manufacturer private data */
+	if (chip->manufacturer.ops && chip->manufacturer.ops->cleanup)
+		chip->manufacturer.ops->cleanup(chip);
 }
 EXPORT_SYMBOL_GPL(nand_release);
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3072f5e..c0a1e36 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -577,6 +577,20 @@ struct nand_buffers {
 };
 
 /**
+ * struct nand_manufacturer_ops - NAND Manufacturer operations
+ * @detect: detect the NAND memory organization and capabilities
+ * @init: initialize all vendor specific fields (like the ->read_retry()
+ *	  implementation) if any.
+ * @cleanup: the ->init() function may have allocated resources, ->cleanup()
+ *	     is here to let vendor specific code release those resources.
+ */
+struct nand_manufacturer_ops {
+	void (*detect)(struct nand_chip *chip);
+	int (*init)(struct nand_chip *chip);
+	void (*cleanup)(struct nand_chip *chip);
+};
+
+/**
  * struct nand_chip - NAND Private Flash Chip Data
  * @mtd:		MTD device registered to the MTD framework
  * @IO_ADDR_R:		[BOARDSPECIFIC] address to read the 8 I/O lines of the
@@ -676,6 +690,7 @@ struct nand_buffers {
  *			additional error status checks (determine if errors are
  *			correctable).
  * @write_page:		[REPLACEABLE] High-level page write function
+ * @manufacturer:	[INTERN] Contains manufacturer data
  */
 
 struct nand_chip {
@@ -756,6 +771,11 @@ struct nand_chip {
 	struct nand_bbt_descr *badblock_pattern;
 
 	void *priv;
+
+	struct {
+		const struct nand_manufacturer_ops *ops;
+		void *priv;
+	} manufacturer;
 };
 
 extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
@@ -792,6 +812,17 @@ static inline void nand_set_controller_data(struct nand_chip *chip, void *priv)
 	chip->priv = priv;
 }
 
+static inline void nand_set_manufacturer_data(struct nand_chip *chip,
+					      void *priv)
+{
+	chip->manufacturer.priv = priv;
+}
+
+static inline void *nand_get_manufacturer_data(struct nand_chip *chip)
+{
+	return chip->manufacturer.priv;
+}
+
 /*
  * NAND Flash Manufacturer ID Codes
  */
@@ -896,10 +927,12 @@ struct nand_flash_dev {
  * struct nand_manufacturers - NAND Flash Manufacturer ID Structure
  * @name:	Manufacturer name
  * @id:		manufacturer ID code of device.
+ * @ops:	manufacturer operations
 */
 struct nand_manufacturers {
 	int id;
 	char *name;
+	const struct nand_manufacturer_ops *ops;
 };
 
 extern struct nand_flash_dev nand_flash_ids[];
@@ -1105,4 +1138,7 @@ int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
 /* Default read_oob syndrome implementation */
 int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 			   int page);
+
+/* Default extended ID decoding function */
+void nand_decode_ext_id(struct nand_chip *chip);
 #endif /* __LINUX_MTD_NAND_H */
-- 
2.7.4

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

* [PATCH v2 06/15] mtd: nand: kill the MTD_NAND_IDS Kconfig option
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (4 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 07/15] mtd: nand: move Samsung specific init/detection logic in nand_samsung.c Boris Brezillon
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

MTD_NAND_IDS is selected by MTD_NAND, which makes it useless. Remove the
Kconfig option and link nand_ids.o into the nand.o object file.
Doing that also prevents adding an extra nand_ids.ko module when MTD_NAND
is activated as a module.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 arch/cris/arch-v32/drivers/Kconfig | 1 -
 drivers/mtd/nand/Kconfig           | 4 ----
 drivers/mtd/nand/Makefile          | 3 +--
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/Kconfig b/arch/cris/arch-v32/drivers/Kconfig
index 2735eb7..b7cd6b9 100644
--- a/arch/cris/arch-v32/drivers/Kconfig
+++ b/arch/cris/arch-v32/drivers/Kconfig
@@ -136,7 +136,6 @@ config ETRAX_NANDFLASH
 	bool "NAND flash support"
 	depends on ETRAX_ARCH_V32
 	select MTD_NAND
-	select MTD_NAND_IDS
 	help
 	  This option enables MTD mapping of NAND flash devices.  Needed to use
 	  NAND flash memories.  If unsure, say Y.
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f05e0e9..16d7dff 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -13,7 +13,6 @@ config MTD_NAND_ECC_SMC
 menuconfig MTD_NAND
 	tristate "NAND Device Support"
 	depends on MTD
-	select MTD_NAND_IDS
 	select MTD_NAND_ECC
 	help
 	  This enables support for accessing all type of NAND flash
@@ -109,9 +108,6 @@ config MTD_NAND_OMAP_BCH
 config MTD_NAND_OMAP_BCH_BUILD
 	def_tristate MTD_NAND_OMAP2 && MTD_NAND_OMAP_BCH
 
-config MTD_NAND_IDS
-	tristate
-
 config MTD_NAND_RICOH
 	tristate "Ricoh xD card reader"
 	default n
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index f553353..d303dee 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -5,7 +5,6 @@
 obj-$(CONFIG_MTD_NAND)			+= nand.o
 obj-$(CONFIG_MTD_NAND_ECC)		+= nand_ecc.o
 obj-$(CONFIG_MTD_NAND_BCH)		+= nand_bch.o
-obj-$(CONFIG_MTD_NAND_IDS)		+= nand_ids.o
 obj-$(CONFIG_MTD_SM_COMMON) 		+= sm_common.o
 
 obj-$(CONFIG_MTD_NAND_CAFE)		+= cafe_nand.o
@@ -58,4 +57,4 @@ obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
-nand-objs := nand_base.o nand_bbt.o nand_timings.o
+nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
-- 
2.7.4

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

* [PATCH v2 07/15] mtd: nand: move Samsung specific init/detection logic in nand_samsung.c
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (5 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 06/15] mtd: nand: kill the MTD_NAND_IDS Kconfig option Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 08/15] mtd: nand: move Hynix specific init/detection logic in nand_hynix.c Boris Brezillon
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Move Samsung specific initialization and detection logic into
nand_samsung.c. This is part of the "separate vendor specific code from
core" cleanup process.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/Makefile       |  1 +
 drivers/mtd/nand/nand_base.c    | 52 ++----------------------
 drivers/mtd/nand/nand_ids.c     |  6 ++-
 drivers/mtd/nand/nand_samsung.c | 89 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+), 50 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_samsung.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index d303dee..55a55d3 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
+nand-objs += nand_samsung.o
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0a7d1c6..a7c7ef3 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3593,48 +3593,13 @@ void nand_decode_ext_id(struct nand_chip *chip)
 	/*
 	 * Field definitions are in the following datasheets:
 	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
-	 * New Samsung (6 byte ID): Samsung K9GAG08U0F (p.44)
 	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
 	 *
 	 * Check for ID length, non-zero 6th byte, cell type, and Hynix/Samsung
 	 * ID to decide what to do.
 	 */
-	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
-			!nand_is_slc(chip) && id_data[5] != 0x00) {
-		/* Calc pagesize */
-		mtd->writesize = 2048 << (extid & 0x03);
-		extid >>= 2;
-		/* Calc oobsize */
-		switch (((extid >> 2) & 0x04) | (extid & 0x03)) {
-		case 1:
-			mtd->oobsize = 128;
-			break;
-		case 2:
-			mtd->oobsize = 218;
-			break;
-		case 3:
-			mtd->oobsize = 400;
-			break;
-		case 4:
-			mtd->oobsize = 436;
-			break;
-		case 5:
-			mtd->oobsize = 512;
-			break;
-		case 6:
-			mtd->oobsize = 640;
-			break;
-		case 7:
-		default: /* Other cases are "reserved" (unknown) */
-			mtd->oobsize = 1024;
-			break;
-		}
-		extid >>= 2;
-		/* Calc blocksize */
-		mtd->erasesize = (128 * 1024) <<
-			(((extid >> 1) & 0x04) | (extid & 0x03));
-	} else if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
-			!nand_is_slc(chip)) {
+	if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
+	    !nand_is_slc(chip)) {
 		unsigned int tmp;
 
 		/* Calc pagesize */
@@ -3762,13 +3727,10 @@ static void nand_decode_bbm_options(struct nand_chip *chip)
 	 * Micron devices with 2KiB pages and on SLC Samsung, Hynix, Toshiba,
 	 * AMD/Spansion, and Macronix.  All others scan only the first page.
 	 */
-	if (!nand_is_slc(chip) &&
-			(maf_id == NAND_MFR_SAMSUNG ||
-			 maf_id == NAND_MFR_HYNIX))
+	if (!nand_is_slc(chip) && maf_id == NAND_MFR_HYNIX)
 		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
 	else if ((nand_is_slc(chip) &&
-				(maf_id == NAND_MFR_SAMSUNG ||
-				 maf_id == NAND_MFR_HYNIX ||
+				(maf_id == NAND_MFR_HYNIX ||
 				 maf_id == NAND_MFR_TOSHIBA ||
 				 maf_id == NAND_MFR_AMD ||
 				 maf_id == NAND_MFR_MACRONIX)) ||
@@ -3926,12 +3888,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	/* Get chip options */
 	chip->options |= type->options;
 
-	/*
-	 * Check if chip is not a Samsung device. Do not clear the
-	 * options for chips which do not have an extended id.
-	 */
-	if (maf_id != NAND_MFR_SAMSUNG && !type->pagesize)
-		chip->options &= ~NAND_SAMSUNG_LP_OPTIONS;
 ident_done:
 
 	if (chip->options & NAND_BUSWIDTH_AUTO) {
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index ccc05f5..5d2bef5 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -10,7 +10,7 @@
 #include <linux/mtd/nand.h>
 #include <linux/sizes.h>
 
-#define LP_OPTIONS NAND_SAMSUNG_LP_OPTIONS
+#define LP_OPTIONS 0
 #define LP_OPTIONS16 (LP_OPTIONS | NAND_BUSWIDTH_16)
 
 #define SP_OPTIONS NAND_NEED_READRDY
@@ -166,9 +166,11 @@ struct nand_flash_dev nand_flash_ids[] = {
 };
 
 /* Manufacturer IDs */
+extern const struct nand_manufacturer_ops samsung_nand_manuf_ops;
+
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba"},
-	{NAND_MFR_SAMSUNG, "Samsung"},
+	{NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
 	{NAND_MFR_FUJITSU, "Fujitsu"},
 	{NAND_MFR_NATIONAL, "National"},
 	{NAND_MFR_RENESAS, "Renesas"},
diff --git a/drivers/mtd/nand/nand_samsung.c b/drivers/mtd/nand/nand_samsung.c
new file mode 100644
index 0000000..863be01
--- /dev/null
+++ b/drivers/mtd/nand/nand_samsung.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) 2013 Boris Brezillon <boris.brezillon@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mtd/nand.h>
+
+static void samsung_nand_decode_id(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	/* New Samsung (6 byte ID): Samsung K9GAG08U0F (p.44) */
+	if (chip->id.len == 6 && !nand_is_slc(chip) &&
+	    chip->id.data[5] != 0x00) {
+		u8 extid = chip->id.data[3];
+
+		/* Get pagesize */
+		mtd->writesize = 2048 << (extid & 0x03);
+
+		extid >>= 2;
+
+		/* Get oobsize */
+		switch (((extid >> 2) & 0x4) | (extid & 0x3)) {
+		case 1:
+			mtd->oobsize = 128;
+			break;
+		case 2:
+			mtd->oobsize = 218;
+			break;
+		case 3:
+			mtd->oobsize = 400;
+			break;
+		case 4:
+			mtd->oobsize = 436;
+			break;
+		case 5:
+			mtd->oobsize = 512;
+			break;
+		case 6:
+			mtd->oobsize = 640;
+			break;
+		default:
+			/*
+			 * We should never reach this case, but if that
+			 * happens, this probably means Samsung decided to use
+			 * a different extended ID format, and we should find
+			 * a way to support it.
+			 */
+			WARN(1, "Invalid OOB size value");
+			break;
+		}
+
+		/* Get blocksize */
+		extid >>= 2;
+		mtd->erasesize = (128 * 1024) <<
+				 (((extid >> 1) & 0x04) | (extid & 0x03));
+	} else {
+		nand_decode_ext_id(chip);
+	}
+}
+
+static int samsung_nand_init(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	if (mtd->writesize > 512)
+		chip->options |= NAND_SAMSUNG_LP_OPTIONS;
+
+	if (!nand_is_slc(chip))
+		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
+	else
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	return 0;
+}
+
+const struct nand_manufacturer_ops samsung_nand_manuf_ops = {
+	.detect = samsung_nand_decode_id,
+	.init = samsung_nand_init,
+};
-- 
2.7.4

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

* [PATCH v2 08/15] mtd: nand: move Hynix specific init/detection logic in nand_hynix.c
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (6 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 07/15] mtd: nand: move Samsung specific init/detection logic in nand_samsung.c Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 09/15] mtd: nand: move Toshiba specific init/detection logic in nand_toshiba.c Boris Brezillon
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Move Hynix specific initialization and detection logic into
nand_hynix.c. This is part of the "separate vendor specific code from
core" cleanup process.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/Makefile     |   1 +
 drivers/mtd/nand/nand_base.c  | 108 ++++++++++--------------------------------
 drivers/mtd/nand/nand_hynix.c |  81 +++++++++++++++++++++++++++++++
 drivers/mtd/nand/nand_ids.c   |   3 +-
 4 files changed, 110 insertions(+), 83 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_hynix.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 55a55d3..d79338f 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -58,4 +58,5 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
+nand-objs += nand_hynix.o
 nand-objs += nand_samsung.o
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a7c7ef3..69812fd 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3590,85 +3590,32 @@ void nand_decode_ext_id(struct nand_chip *chip)
 	/* The 4th id byte is the important one */
 	extid = id_data[3];
 
+	/* 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 */
+	if (extid & 0x1)
+		chip->options |= NAND_BUSWIDTH_16;
+
 	/*
-	 * Field definitions are in the following datasheets:
-	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
-	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
-	 *
-	 * Check for ID length, non-zero 6th byte, cell type, and Hynix/Samsung
-	 * ID to decide what to do.
+	 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
+	 * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
+	 * follows:
+	 * - ID byte 6, bits[2:0]: 100b -> 43nm, 101b -> 32nm,
+	 *                         110b -> 24nm
+	 * - ID byte 5, bit[7]:    1 -> BENAND, 0 -> raw SLC
 	 */
-	if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
-	    !nand_is_slc(chip)) {
-		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;
-	} 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 */
-		if (extid & 0x1)
-			chip->options |= NAND_BUSWIDTH_16;
-
-		/*
-		 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
-		 * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
-		 * follows:
-		 * - ID byte 6, bits[2:0]: 100b -> 43nm, 101b -> 32nm,
-		 *                         110b -> 24nm
-		 * - ID byte 5, bit[7]:    1 -> BENAND, 0 -> raw SLC
-		 */
-		if (id_len >= 6 && id_data[0] == NAND_MFR_TOSHIBA &&
-				nand_is_slc(chip) &&
-				(id_data[5] & 0x7) == 0x6 /* 24nm */ &&
-				!(id_data[4] & 0x80) /* !BENAND */) {
-			mtd->oobsize = 32 * mtd->writesize >> 9;
-		}
-
-	}
+	if (id_len >= 6 && id_data[0] == NAND_MFR_TOSHIBA &&
+	    nand_is_slc(chip) &&
+	    (id_data[5] & 0x7) == 0x6 /* 24nm */ &&
+	     !(id_data[4] & 0x80) /* !BENAND */)
+		mtd->oobsize = 32 * mtd->writesize >> 9;
 }
 EXPORT_SYMBOL_GPL(nand_decode_ext_id);
 
@@ -3727,11 +3674,8 @@ static void nand_decode_bbm_options(struct nand_chip *chip)
 	 * Micron devices with 2KiB pages and on SLC Samsung, Hynix, Toshiba,
 	 * AMD/Spansion, and Macronix.  All others scan only the first page.
 	 */
-	if (!nand_is_slc(chip) && maf_id == NAND_MFR_HYNIX)
-		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
-	else if ((nand_is_slc(chip) &&
-				(maf_id == NAND_MFR_HYNIX ||
-				 maf_id == NAND_MFR_TOSHIBA ||
+	if ((nand_is_slc(chip) &&
+				(maf_id == NAND_MFR_TOSHIBA ||
 				 maf_id == NAND_MFR_AMD ||
 				 maf_id == NAND_MFR_MACRONIX)) ||
 			(mtd->writesize == 2048 &&
diff --git a/drivers/mtd/nand/nand_hynix.c b/drivers/mtd/nand/nand_hynix.c
new file mode 100644
index 0000000..06c8e8b
--- /dev/null
+++ b/drivers/mtd/nand/nand_hynix.c
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2013 Boris Brezillon <boris.brezillon@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mtd/nand.h>
+
+static void hynix_nand_decode_id(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	/* 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];
+
+		/* Extract pagesize */
+		mtd->writesize = 2048 << (extid & 0x03);
+		extid >>= 2;
+
+		/* Extract oobsize */
+		switch (((extid >> 2) & 0x4) | (extid & 0x3)) {
+		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;
+		}
+
+		/* 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;
+	} else {
+		nand_decode_ext_id(chip);
+	}
+}
+
+static int hynix_nand_init(struct nand_chip *chip)
+{
+	if (!nand_is_slc(chip))
+		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
+	else
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	return 0;
+}
+
+const struct nand_manufacturer_ops hynix_nand_manuf_ops = {
+	.detect = hynix_nand_decode_id,
+	.init = hynix_nand_init,
+};
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 5d2bef5..7bf93b8 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -167,6 +167,7 @@ struct nand_flash_dev nand_flash_ids[] = {
 
 /* Manufacturer IDs */
 extern const struct nand_manufacturer_ops samsung_nand_manuf_ops;
+extern const struct nand_manufacturer_ops hynix_nand_manuf_ops;
 
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba"},
@@ -175,7 +176,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_NATIONAL, "National"},
 	{NAND_MFR_RENESAS, "Renesas"},
 	{NAND_MFR_STMICRO, "ST Micro"},
-	{NAND_MFR_HYNIX, "Hynix"},
+	{NAND_MFR_HYNIX, "Hynix", &hynix_nand_manuf_ops},
 	{NAND_MFR_MICRON, "Micron"},
 	{NAND_MFR_AMD, "AMD/Spansion"},
 	{NAND_MFR_MACRONIX, "Macronix"},
-- 
2.7.4

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

* [PATCH v2 09/15] mtd: nand: move Toshiba specific init/detection logic in nand_toshiba.c
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (7 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 08/15] mtd: nand: move Hynix specific init/detection logic in nand_hynix.c Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 10/15] mtd: nand: move Micron specific init logic in nand_micron.c Boris Brezillon
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Move Toshiba specific initialization and detection logic into
nand_toshiba.c. This is part of the "separate vendor specific code from
core" cleanup process.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/Makefile       |  1 +
 drivers/mtd/nand/nand_base.c    | 19 ++--------------
 drivers/mtd/nand/nand_ids.c     |  3 ++-
 drivers/mtd/nand/nand_toshiba.c | 48 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 18 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_toshiba.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index d79338f..403e9a7 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
 nand-objs += nand_hynix.o
 nand-objs += nand_samsung.o
+nand-objs += nand_toshiba.o
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 69812fd..931aafb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3583,7 +3583,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
 void nand_decode_ext_id(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	int extid, id_len = chip->id.len;
+	int extid;
 	u8 *id_data = chip->id.data;
 	/* The 3rd id byte holds MLC / multichip data */
 	chip->bits_per_cell = nand_get_bits_per_cell(id_data[2]);
@@ -3602,20 +3602,6 @@ void nand_decode_ext_id(struct nand_chip *chip)
 	/* Get buswidth information */
 	if (extid & 0x1)
 		chip->options |= NAND_BUSWIDTH_16;
-
-	/*
-	 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
-	 * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
-	 * follows:
-	 * - ID byte 6, bits[2:0]: 100b -> 43nm, 101b -> 32nm,
-	 *                         110b -> 24nm
-	 * - ID byte 5, bit[7]:    1 -> BENAND, 0 -> raw SLC
-	 */
-	if (id_len >= 6 && id_data[0] == NAND_MFR_TOSHIBA &&
-	    nand_is_slc(chip) &&
-	    (id_data[5] & 0x7) == 0x6 /* 24nm */ &&
-	     !(id_data[4] & 0x80) /* !BENAND */)
-		mtd->oobsize = 32 * mtd->writesize >> 9;
 }
 EXPORT_SYMBOL_GPL(nand_decode_ext_id);
 
@@ -3675,8 +3661,7 @@ static void nand_decode_bbm_options(struct nand_chip *chip)
 	 * AMD/Spansion, and Macronix.  All others scan only the first page.
 	 */
 	if ((nand_is_slc(chip) &&
-				(maf_id == NAND_MFR_TOSHIBA ||
-				 maf_id == NAND_MFR_AMD ||
+				(maf_id == NAND_MFR_AMD ||
 				 maf_id == NAND_MFR_MACRONIX)) ||
 			(mtd->writesize == 2048 &&
 			 maf_id == NAND_MFR_MICRON))
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 7bf93b8..2cc9d32 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -166,11 +166,12 @@ struct nand_flash_dev nand_flash_ids[] = {
 };
 
 /* Manufacturer IDs */
+extern const struct nand_manufacturer_ops toshiba_nand_manuf_ops;
 extern const struct nand_manufacturer_ops samsung_nand_manuf_ops;
 extern const struct nand_manufacturer_ops hynix_nand_manuf_ops;
 
 struct nand_manufacturers nand_manuf_ids[] = {
-	{NAND_MFR_TOSHIBA, "Toshiba"},
+	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
 	{NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
 	{NAND_MFR_FUJITSU, "Fujitsu"},
 	{NAND_MFR_NATIONAL, "National"},
diff --git a/drivers/mtd/nand/nand_toshiba.c b/drivers/mtd/nand/nand_toshiba.c
new file mode 100644
index 0000000..0e19f9d
--- /dev/null
+++ b/drivers/mtd/nand/nand_toshiba.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2013 Boris Brezillon <boris.brezillon@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mtd/nand.h>
+
+static void toshiba_nand_decode_id(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	nand_decode_ext_id(chip);
+
+	/*
+	 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
+	 * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
+	 * follows:
+	 * - ID byte 6, bits[2:0]: 100b -> 43nm, 101b -> 32nm,
+	 *                         110b -> 24nm
+	 * - ID byte 5, bit[7]:    1 -> BENAND, 0 -> raw SLC
+	 */
+	if (chip->id.len >= 6 && nand_is_slc(chip) &&
+	    (chip->id.data[5] & 0x7) == 0x6 /* 24nm */ &&
+	    !(chip->id.data[4] & 0x80) /* !BENAND */)
+		mtd->oobsize = 32 * mtd->writesize >> 9;
+}
+
+static int toshiba_nand_init(struct nand_chip *chip)
+{
+	if (nand_is_slc(chip))
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	return 0;
+}
+
+const struct nand_manufacturer_ops toshiba_nand_manuf_ops = {
+	.detect = toshiba_nand_decode_id,
+	.init = toshiba_nand_init,
+};
-- 
2.7.4

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

* [PATCH v2 10/15] mtd: nand: move Micron specific init logic in nand_micron.c
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (8 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 09/15] mtd: nand: move Toshiba specific init/detection logic in nand_toshiba.c Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 11/15] mtd: nand: move AMD/Spansion specific init/detection logic in nand_amd.c Boris Brezillon
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Move Micron specific initialization logic into nand_micron.c. This is
part of the "separate vendor specific code from core" cleanup process.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/Makefile      |  1 +
 drivers/mtd/nand/nand_base.c   | 31 +---------------
 drivers/mtd/nand/nand_ids.c    |  3 +-
 drivers/mtd/nand/nand_micron.c | 84 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h       | 20 ----------
 5 files changed, 88 insertions(+), 51 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_micron.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 403e9a7..f1bd008 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -59,5 +59,6 @@ obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
 nand-objs += nand_hynix.o
+nand-objs += nand_micron.o
 nand-objs += nand_samsung.o
 nand-objs += nand_toshiba.o
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 931aafb..17fb6f6 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3301,30 +3301,6 @@ ext_out:
 	return ret;
 }
 
-static int nand_setup_read_retry_micron(struct mtd_info *mtd, int retry_mode)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {retry_mode};
-
-	return chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
-			feature);
-}
-
-/*
- * Configure chip properties from Micron vendor-specific ONFI table
- */
-static void nand_onfi_detect_micron(struct nand_chip *chip,
-		struct nand_onfi_params *p)
-{
-	struct nand_onfi_vendor_micron *micron = (void *)p->vendor;
-
-	if (le16_to_cpu(p->vendor_revision) < 1)
-		return;
-
-	chip->read_retries = micron->read_retry_options;
-	chip->setup_read_retry = nand_setup_read_retry_micron;
-}
-
 /*
  * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
  */
@@ -3421,9 +3397,6 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 		pr_warn("Could not retrieve ONFI ECC requirements\n");
 	}
 
-	if (p->jedec_id == NAND_MFR_MICRON)
-		nand_onfi_detect_micron(chip, p);
-
 	return 1;
 }
 
@@ -3662,9 +3635,7 @@ static void nand_decode_bbm_options(struct nand_chip *chip)
 	 */
 	if ((nand_is_slc(chip) &&
 				(maf_id == NAND_MFR_AMD ||
-				 maf_id == NAND_MFR_MACRONIX)) ||
-			(mtd->writesize == 2048 &&
-			 maf_id == NAND_MFR_MICRON))
+				 maf_id == NAND_MFR_MACRONIX)))
 		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
 }
 
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 2cc9d32..5d02787 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -169,6 +169,7 @@ struct nand_flash_dev nand_flash_ids[] = {
 extern const struct nand_manufacturer_ops toshiba_nand_manuf_ops;
 extern const struct nand_manufacturer_ops samsung_nand_manuf_ops;
 extern const struct nand_manufacturer_ops hynix_nand_manuf_ops;
+extern const struct nand_manufacturer_ops micron_nand_manuf_ops;
 
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
@@ -178,7 +179,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_RENESAS, "Renesas"},
 	{NAND_MFR_STMICRO, "ST Micro"},
 	{NAND_MFR_HYNIX, "Hynix", &hynix_nand_manuf_ops},
-	{NAND_MFR_MICRON, "Micron"},
+	{NAND_MFR_MICRON, "Micron", &micron_nand_manuf_ops},
 	{NAND_MFR_AMD, "AMD/Spansion"},
 	{NAND_MFR_MACRONIX, "Macronix"},
 	{NAND_MFR_EON, "Eon"},
diff --git a/drivers/mtd/nand/nand_micron.c b/drivers/mtd/nand/nand_micron.c
new file mode 100644
index 0000000..2303e45
--- /dev/null
+++ b/drivers/mtd/nand/nand_micron.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2013 Boris Brezillon <boris.brezillon@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mtd/nand.h>
+
+struct nand_onfi_vendor_micron {
+	u8 two_plane_read;
+	u8 read_cache;
+	u8 read_unique_id;
+	u8 dq_imped;
+	u8 dq_imped_num_settings;
+	u8 dq_imped_feat_addr;
+	u8 rb_pulldown_strength;
+	u8 rb_pulldown_strength_feat_addr;
+	u8 rb_pulldown_strength_num_settings;
+	u8 otp_mode;
+	u8 otp_page_start;
+	u8 otp_data_prot_addr;
+	u8 otp_num_pages;
+	u8 otp_feat_addr;
+	u8 read_retry_options;
+	u8 reserved[72];
+	u8 param_revision;
+} __packed;
+
+
+static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {retry_mode};
+
+	return chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
+				       feature);
+}
+
+/*
+ * Configure chip properties from Micron vendor-specific ONFI table
+ */
+static int micron_nand_onfi_init(struct nand_chip *chip)
+{
+	struct nand_onfi_params *p = &chip->onfi_params;
+	struct nand_onfi_vendor_micron *micron = (void *)p->vendor;
+
+	if (!chip->onfi_version)
+		return 0;
+
+	if (le16_to_cpu(p->vendor_revision) < 1)
+		return 0;
+
+	chip->read_retries = micron->read_retry_options;
+	chip->setup_read_retry = micron_nand_setup_read_retry;
+
+	return 0;
+}
+
+static int micron_nand_init(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int ret;
+
+	ret = micron_nand_onfi_init(chip);
+	if (ret)
+		return ret;
+
+	if (mtd->writesize == 2048)
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	return 0;
+}
+
+const struct nand_manufacturer_ops micron_nand_manuf_ops = {
+	.init = micron_nand_init,
+};
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index c0a1e36..0bdea71 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -348,26 +348,6 @@ struct onfi_ext_param_page {
 	 */
 } __packed;
 
-struct nand_onfi_vendor_micron {
-	u8 two_plane_read;
-	u8 read_cache;
-	u8 read_unique_id;
-	u8 dq_imped;
-	u8 dq_imped_num_settings;
-	u8 dq_imped_feat_addr;
-	u8 rb_pulldown_strength;
-	u8 rb_pulldown_strength_feat_addr;
-	u8 rb_pulldown_strength_num_settings;
-	u8 otp_mode;
-	u8 otp_page_start;
-	u8 otp_data_prot_addr;
-	u8 otp_num_pages;
-	u8 otp_feat_addr;
-	u8 read_retry_options;
-	u8 reserved[72];
-	u8 param_revision;
-} __packed;
-
 struct jedec_ecc_info {
 	u8 ecc_bits;
 	u8 codeword_size;
-- 
2.7.4

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

* [PATCH v2 11/15] mtd: nand: move AMD/Spansion specific init/detection logic in nand_amd.c
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (9 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 10/15] mtd: nand: move Micron specific init logic in nand_micron.c Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 12/15] mtd: nand: move Macronix specific initialization in nand_macronix.c Boris Brezillon
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Moving AMD/Spansion specific initialization/detection into nand_amd.c.
This is part of the "separate vendor specific code from core" cleanup
process.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/Makefile    |  1 +
 drivers/mtd/nand/nand_amd.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/nand_base.c | 19 +-----------------
 drivers/mtd/nand/nand_ids.c  |  3 ++-
 4 files changed, 52 insertions(+), 19 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_amd.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index f1bd008..19d0cb8 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
+nand-objs += nand_amd.o
 nand-objs += nand_hynix.o
 nand-objs += nand_micron.o
 nand-objs += nand_samsung.o
diff --git a/drivers/mtd/nand/nand_amd.c b/drivers/mtd/nand/nand_amd.c
new file mode 100644
index 0000000..3d7cc36
--- /dev/null
+++ b/drivers/mtd/nand/nand_amd.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2013 Boris Brezillon <boris.brezillon@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mtd/nand.h>
+
+static void amd_nand_decode_id(struct nand_chip *chip)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	nand_decode_ext_id(chip);
+
+	/*
+	 * 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 (chip->id.data[4] != 0x00 && chip->id.data[5] == 0x00 &&
+	     chip->id.data[6] == 0x00 && chip->id.data[7] == 0x00 &&
+	     mtd->writesize == 512) {
+		mtd->erasesize = 128 * 1024;
+		mtd->erasesize <<= ((chip->id.data[3] & 0x03) << 1);
+	}
+}
+
+static int amd_nand_init(struct nand_chip *chip)
+{
+	if (nand_is_slc(chip))
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	return 0;
+}
+
+const struct nand_manufacturer_ops amd_nand_manuf_ops = {
+	.detect = amd_nand_decode_id,
+	.init = amd_nand_init,
+};
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 17fb6f6..5fd8487 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3586,8 +3586,6 @@ EXPORT_SYMBOL_GPL(nand_decode_ext_id);
 static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	u8 *id_data = chip->id.data;
-	int maf_id = id_data[0];
 
 	mtd->erasesize = type->erasesize;
 	mtd->writesize = type->pagesize;
@@ -3595,19 +3593,6 @@ static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
 
 	/* All legacy ID NAND are small-page, SLC */
 	chip->bits_per_cell = 1;
-
-	/*
-	 * 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);
-	}
 }
 
 /*
@@ -3633,9 +3618,7 @@ static void nand_decode_bbm_options(struct nand_chip *chip)
 	 * Micron devices with 2KiB pages and on SLC Samsung, Hynix, Toshiba,
 	 * AMD/Spansion, and Macronix.  All others scan only the first page.
 	 */
-	if ((nand_is_slc(chip) &&
-				(maf_id == NAND_MFR_AMD ||
-				 maf_id == NAND_MFR_MACRONIX)))
+	if (nand_is_slc(chip) && maf_id == NAND_MFR_MACRONIX)
 		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
 }
 
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 5d02787..299b2b1 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -170,6 +170,7 @@ extern const struct nand_manufacturer_ops toshiba_nand_manuf_ops;
 extern const struct nand_manufacturer_ops samsung_nand_manuf_ops;
 extern const struct nand_manufacturer_ops hynix_nand_manuf_ops;
 extern const struct nand_manufacturer_ops micron_nand_manuf_ops;
+extern const struct nand_manufacturer_ops amd_nand_manuf_ops;
 
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
@@ -180,7 +181,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_STMICRO, "ST Micro"},
 	{NAND_MFR_HYNIX, "Hynix", &hynix_nand_manuf_ops},
 	{NAND_MFR_MICRON, "Micron", &micron_nand_manuf_ops},
-	{NAND_MFR_AMD, "AMD/Spansion"},
+	{NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
 	{NAND_MFR_MACRONIX, "Macronix"},
 	{NAND_MFR_EON, "Eon"},
 	{NAND_MFR_SANDISK, "SanDisk"},
-- 
2.7.4

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

* [PATCH v2 12/15] mtd: nand: move Macronix specific initialization in nand_macronix.c
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (10 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 11/15] mtd: nand: move AMD/Spansion specific init/detection logic in nand_amd.c Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 13/15] mtd: nand: samsung: retrieve ECC requirements from extended ID Boris Brezillon
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Move Macronix specific initialization into nand_macronix.c. This is part
of the "separate vendor specific code from core" cleanup process.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/Makefile        |  1 +
 drivers/mtd/nand/nand_base.c     | 11 -----------
 drivers/mtd/nand/nand_ids.c      |  3 ++-
 drivers/mtd/nand/nand_macronix.c | 27 +++++++++++++++++++++++++++
 4 files changed, 30 insertions(+), 12 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_macronix.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 19d0cb8..19150d3 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
 nand-objs += nand_amd.o
 nand-objs += nand_hynix.o
+nand-objs += nand_macronix.o
 nand-objs += nand_micron.o
 nand-objs += nand_samsung.o
 nand-objs += nand_toshiba.o
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 5fd8487..06b5abf 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3603,23 +3603,12 @@ static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev *type)
 static void nand_decode_bbm_options(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	u8 *id_data = chip->id.data;
-	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 (nand_is_slc(chip) && maf_id == NAND_MFR_MACRONIX)
-		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
 }
 
 static inline bool is_full_id_nand(struct nand_flash_dev *type)
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 299b2b1..5bc347c 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -171,6 +171,7 @@ extern const struct nand_manufacturer_ops samsung_nand_manuf_ops;
 extern const struct nand_manufacturer_ops hynix_nand_manuf_ops;
 extern const struct nand_manufacturer_ops micron_nand_manuf_ops;
 extern const struct nand_manufacturer_ops amd_nand_manuf_ops;
+extern const struct nand_manufacturer_ops macronix_nand_manuf_ops;
 
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
@@ -182,7 +183,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_HYNIX, "Hynix", &hynix_nand_manuf_ops},
 	{NAND_MFR_MICRON, "Micron", &micron_nand_manuf_ops},
 	{NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
-	{NAND_MFR_MACRONIX, "Macronix"},
+	{NAND_MFR_MACRONIX, "Macronix", &macronix_nand_manuf_ops},
 	{NAND_MFR_EON, "Eon"},
 	{NAND_MFR_SANDISK, "SanDisk"},
 	{NAND_MFR_INTEL, "Intel"},
diff --git a/drivers/mtd/nand/nand_macronix.c b/drivers/mtd/nand/nand_macronix.c
new file mode 100644
index 0000000..960ddc0
--- /dev/null
+++ b/drivers/mtd/nand/nand_macronix.c
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2013 Boris Brezillon <boris.brezillon@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/mtd/nand.h>
+
+static int macronix_nand_init(struct nand_chip *chip)
+{
+	if (nand_is_slc(chip))
+		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
+
+	return 0;
+}
+
+const struct nand_manufacturer_ops macronix_nand_manuf_ops = {
+	.init = macronix_nand_init,
+};
-- 
2.7.4

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

* [PATCH v2 13/15] mtd: nand: samsung: retrieve ECC requirements from extended ID
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (11 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 12/15] mtd: nand: move Macronix specific initialization in nand_macronix.c Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 13:00 ` [PATCH v2 14/15] mtd: nand: hynix: rework NAND ID decoding to extract more information Boris Brezillon
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

From: Hans de Goede <hdegoede@redhat.com>

On some nand controllers with hw-ecc the controller code wants to know
the ecc strength and size and having these as 0, 0 is not accepted.

Specifying these in devicetree is possible but undesirable as the nand
may be different in different production runs of the same board, so it
is better to get this info from the nand id where possible.

This commit adds code to read the ecc strength and size from the nand
for Samsung extended-id nands. This code is based on the info for the 5th
id byte in the datasheets for the following Samsung nands: K9GAG08U0E,
K9GAG08U0F, K9GAG08X0D, K9GBG08U0A, K9GBG08U0B. These all use these bits
in the exact same way.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_samsung.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mtd/nand/nand_samsung.c b/drivers/mtd/nand/nand_samsung.c
index 863be01..5d973d4 100644
--- a/drivers/mtd/nand/nand_samsung.c
+++ b/drivers/mtd/nand/nand_samsung.c
@@ -63,6 +63,26 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 		extid >>= 2;
 		mtd->erasesize = (128 * 1024) <<
 				 (((extid >> 1) & 0x04) | (extid & 0x03));
+
+		/* Extract ECC requirements from 5th id byte*/
+		extid = (chip->id.data[4] >> 4) & 0x07;
+		if (extid < 5) {
+			chip->ecc_step_ds = 512;
+			chip->ecc_strength_ds = 1 << extid;
+		} else {
+			chip->ecc_step_ds = 1024;
+			switch (extid) {
+			case 5:
+				chip->ecc_strength_ds = 24;
+				break;
+			case 6:
+				chip->ecc_strength_ds = 40;
+				break;
+			case 7:
+				chip->ecc_strength_ds = 60;
+				break;
+			}
+		}
 	} else {
 		nand_decode_ext_id(chip);
 	}
-- 
2.7.4

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

* [PATCH v2 14/15] mtd: nand: hynix: rework NAND ID decoding to extract more information
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (12 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 13/15] mtd: nand: samsung: retrieve ECC requirements from extended ID Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-08 14:34   ` kbuild test robot
  2016-06-08 13:00 ` [PATCH v2 15/15] mtd: nand: hynix: add read-retry support for 1x nm MLC NANDs Boris Brezillon
  2016-06-18 22:09 ` [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Richard Weinberger
  15 siblings, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

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>
---
 drivers/mtd/nand/nand_hynix.c | 227 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 208 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/nand_hynix.c b/drivers/mtd/nand/nand_hynix.c
index 06c8e8b..f73b99f 100644
--- a/drivers/mtd/nand/nand_hynix.c
+++ b/drivers/mtd/nand/nand_hynix.c
@@ -14,20 +14,54 @@
 
 #include <linux/mtd/nand.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 Samsung 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;
@@ -46,23 +80,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 Samsung 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 Samsung 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 Samsung 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

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

* [PATCH v2 15/15] mtd: nand: hynix: add read-retry support for 1x nm MLC NANDs
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (13 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 14/15] mtd: nand: hynix: rework NAND ID decoding to extract more information Boris Brezillon
@ 2016-06-08 13:00 ` Boris Brezillon
  2016-06-18 22:09 ` [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Richard Weinberger
  15 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-08 13:00 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, linux-mtd, Boris Brezillon,
	Richard Weinberger
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

All Hynix MLC NANDs using the produced with the 1X nm process support
read-retry.
This read retry implementation should also be reusable for other Hynix
NANDs, but the method to retrieve the read-retry parameters from the
read-retry OTP area might change a bit (some NANDs are even using a fixed
set of values instead of retrieving those information from the OTP area).

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_hynix.c | 361 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 360 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_hynix.c b/drivers/mtd/nand/nand_hynix.c
index f73b99f..7f5b446 100644
--- a/drivers/mtd/nand/nand_hynix.c
+++ b/drivers/mtd/nand/nand_hynix.c
@@ -13,6 +13,53 @@
  */
 
 #include <linux/mtd/nand.h>
+#include <linux/slab.h>
+
+#define NAND_HYNIX_CMD_SET_PARAMS	0x36
+#define NAND_HYNIX_CMD_APPLY_PARAMS	0x16
+
+#define NAND_HYNIX_1XNM_RR_REPEAT	8
+
+/**
+ * struct hynix_read_retry - read-retry data
+ * @nregs: number of register to set when applying a new read-retry mode
+ * @regs: register offsets (NAND chip dependent)
+ * @values: array of values to set in registers. The array size is equal to
+ *	    (nregs * nmodes)
+ */
+struct hynix_read_retry {
+	int nregs;
+	const u8 *regs;
+	u8 values[0];
+};
+
+/**
+ * struct hynix_nand - private Hynix NAND struct
+ * @nand_technology: manufacturing process expressed in picometer
+ * @read_retry: read-retry information
+ */
+struct hynix_nand {
+	const struct hynix_read_retry *read_retry;
+};
+
+/**
+ * struct hynix_read_retry_otp - structure describing how the read-retry OTP
+ *				 area
+ * @nregs: number of hynix private registers to set before reading the reading
+ *	   the OTP area
+ * @regs: registers that should be configured
+ * @values: values that should be set in regs
+ * @page: the address to pass to the READ_PAGE command. Depends on the NAND
+ *	  chip
+ * @size: size of the read-retry OTP section
+ */
+struct hynix_read_retry_otp {
+	int nregs;
+	const u8 *regs;
+	const u8 *values;
+	int page;
+	int size;
+};
 
 static bool hynix_nand_has_valid_jedecid(struct nand_chip *chip)
 {
@@ -27,6 +74,292 @@ static bool hynix_nand_has_valid_jedecid(struct nand_chip *chip)
 	return !strcmp("JEDEC", jedecid);
 }
 
+static int hynix_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct hynix_nand *hynix = nand_get_manufacturer_data(chip);
+	const u8 *values;
+	int status;
+	int i;
+
+	values = hynix->read_retry->values +
+		 (retry_mode * hynix->read_retry->nregs);
+
+	/* Enter 'Set Hynix Parameters' mode */
+	chip->cmdfunc(mtd, NAND_HYNIX_CMD_SET_PARAMS, -1, -1);
+
+	/*
+	 * Configure the NAND in the requested read-retry mode.
+	 * This is done by setting pre-defined values in internal NAND
+	 * registers.
+	 *
+	 * The set of registers is NAND specific, and the values are either
+	 * predefined or extracted from an OTP area on the NAND (values are
+	 * probably tweaked at production in this case).
+	 */
+	for (i = 0; i < hynix->read_retry->nregs; i++) {
+		int column = hynix->read_retry->regs[i];
+
+		column |= column << 8;
+		chip->cmdfunc(mtd, NAND_CMD_NONE, column, -1);
+		chip->write_byte(mtd, values[i]);
+	}
+
+	/* Apply the new settings. */
+	chip->cmdfunc(mtd, NAND_HYNIX_CMD_APPLY_PARAMS, -1, -1);
+
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
+	return 0;
+}
+
+/**
+ * hynix_get_majority - get the value that is occurring the most in a given
+ *			set of values
+ * @in: the array of values to test
+ * @repeat: the size of the in array
+ * @out: pointer used to store the output value
+ *
+ * This function implements the 'majority check' logic that is supposed to
+ * overcome the unreliability of MLC NANDs when reading the OTP area storing
+ * the read-retry parameters.
+ *
+ * It's based on a pretty simple assumption: if we repeat the same value
+ * several times and then take the one that is occurring the most, we should
+ * find the correct value.
+ * Let's hope this dummy algorithm prevents us from losing the read-retry
+ * parameters.
+ */
+static int hynix_get_majority(const u8 *in, int repeat, u8 *out)
+{
+	int i, j, half = repeat / 2;
+
+	/*
+	 * We only test the first half of the in array because we must ensure
+	 * that the value is at least occurring repeat / 2 times.
+	 *
+	 * This loop is suboptimal since we may count the occurrences of the
+	 * same value several time, but we are doing that on small sets, which
+	 * makes it acceptable.
+	 */
+	for (i = 0; i < half; i++) {
+		int cnt = 0;
+		u8 val = in[i];
+
+		/* Count all values that are matching the one at index i. */
+		for (j = i + 1; j < repeat; j++) {
+			if (in[j] == val)
+				cnt++;
+		}
+
+		/* We found a value occurring more than repeat / 2. */
+		if (cnt > half) {
+			*out = val;
+			return 0;
+		}
+	}
+
+	return -EIO;
+}
+
+static int hynix_read_rr_otp(struct nand_chip *chip,
+			     const struct hynix_read_retry_otp *info,
+			     void *buf)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int i;
+
+	chip->select_chip(mtd, 0);
+	chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+
+	chip->cmdfunc(mtd, NAND_HYNIX_CMD_SET_PARAMS, -1, -1);
+
+	for (i = 0; i < info->nregs; i++) {
+		int column = info->regs[i];
+
+		column |= column << 8;
+		chip->cmdfunc(mtd, NAND_CMD_NONE, column, -1);
+		chip->write_byte(mtd, info->values[i]);
+	}
+
+	chip->cmdfunc(mtd, NAND_HYNIX_CMD_APPLY_PARAMS, -1, -1);
+
+	/* Sequence to enter OTP mode? */
+	chip->cmdfunc(mtd, 0x17, -1, -1);
+	chip->cmdfunc(mtd, 0x04, -1, -1);
+	chip->cmdfunc(mtd, 0x19, -1, -1);
+
+	/* Now read the page */
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, info->page);
+	chip->read_buf(mtd, buf, info->size);
+
+	/* Put everything back to normal */
+	chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+	chip->cmdfunc(mtd, NAND_HYNIX_CMD_SET_PARAMS, 0x38, -1);
+	chip->write_byte(mtd, 0x0);
+	chip->cmdfunc(mtd, NAND_HYNIX_CMD_APPLY_PARAMS, -1, -1);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, -1);
+	chip->select_chip(mtd, -1);
+
+	return 0;
+}
+
+#define NAND_HYNIX_1XNM_RR_COUNT_OFFS				0
+#define NAND_HYNIX_1XNM_RR_REG_COUNT_OFFS			8
+#define NAND_HYNIX_1XNM_RR_SET_OFFS(x, setsize, inv)		\
+	(16 + ((((x) * 2) + ((inv) ? 1 : 0)) * (setsize)))
+
+static int hynix_mlc_1xnm_rr_value(const u8 *buf, int nmodes, int nregs,
+				   int mode, int reg, bool inv, u8 *val)
+{
+	u8 tmp[NAND_HYNIX_1XNM_RR_REPEAT];
+	int val_offs = (mode * nregs) + reg;
+	int set_size = nmodes * nregs;
+	int i, ret;
+
+	for (i = 0; i < NAND_HYNIX_1XNM_RR_REPEAT; i++) {
+		int set_offs = NAND_HYNIX_1XNM_RR_SET_OFFS(i, set_size, inv);
+
+		tmp[i] = buf[val_offs + set_offs];
+	}
+
+
+	ret = hynix_get_majority(tmp, NAND_HYNIX_1XNM_RR_REPEAT, val);
+	if (ret)
+		return ret;
+
+	if (inv)
+		*val = ~*val;
+
+	return 0;
+}
+
+static u8 hynix_1xnm_mlc_read_retry_regs[] = {
+	0xcc, 0xbf, 0xaa, 0xab, 0xcd, 0xad, 0xae, 0xaf
+};
+
+static int hynix_mlc_1xnm_rr_init(struct nand_chip *chip,
+				  const struct hynix_read_retry_otp *info)
+{
+	struct hynix_nand *hynix = nand_get_manufacturer_data(chip);
+	struct hynix_read_retry *rr = NULL;
+	int ret, i, j;
+	u8 nregs, nmodes;
+	u8 *buf;
+
+	buf = kmalloc(info->size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = hynix_read_rr_otp(chip, info, buf);
+	if (ret)
+		goto out;
+
+	ret = hynix_get_majority(buf, NAND_HYNIX_1XNM_RR_REPEAT,
+				 &nmodes);
+	if (ret)
+		goto out;
+
+	ret = hynix_get_majority(buf + NAND_HYNIX_1XNM_RR_REPEAT,
+				 NAND_HYNIX_1XNM_RR_REPEAT,
+				 &nregs);
+	if (ret)
+		goto out;
+
+	rr = kzalloc(sizeof(*rr) + (nregs * nmodes), GFP_KERNEL);
+	if (!rr)
+		goto out;
+
+	for (i = 0; i < nmodes; i++) {
+		for (j = 0; j < nregs; j++) {
+			u8 *val = rr->values + (i * nregs);
+
+			ret = hynix_mlc_1xnm_rr_value(buf, nmodes, nregs, i, j,
+						      false, val);
+			if (!ret)
+				continue;
+
+			ret = hynix_mlc_1xnm_rr_value(buf, nmodes, nregs, i, j,
+						      true, val);
+			if (ret)
+				goto out;
+		}
+	}
+
+	rr->nregs = nregs;
+	rr->regs = hynix_1xnm_mlc_read_retry_regs;
+	hynix->read_retry = rr;
+	chip->setup_read_retry = hynix_nand_setup_read_retry;
+	chip->read_retries = nmodes;
+
+out:
+	kfree(buf);
+
+	if (ret)
+		kfree(rr);
+
+	return ret;
+}
+
+static const u8 hynix_mlc_1xnm_rr_otp_regs[] = { 0x38 };
+static const u8 hynix_mlc_1xnm_rr_otp_values[] = { 0x52 };
+
+static const struct hynix_read_retry_otp hynix_mlc_1xnm_rr_otps[] = {
+	{
+		.nregs = ARRAY_SIZE(hynix_mlc_1xnm_rr_otp_regs),
+		.regs = hynix_mlc_1xnm_rr_otp_regs,
+		.values = hynix_mlc_1xnm_rr_otp_values,
+		.page = 0x21f,
+		.size = 784
+	},
+	{
+		.nregs = ARRAY_SIZE(hynix_mlc_1xnm_rr_otp_regs),
+		.regs = hynix_mlc_1xnm_rr_otp_regs,
+		.values = hynix_mlc_1xnm_rr_otp_values,
+		.page = 0x200,
+		.size = 528,
+	},
+};
+
+static int hynix_nand_rr_init(struct nand_chip *chip)
+{
+	int i, ret = 0;
+	bool valid_jedecid;
+
+	valid_jedecid = hynix_nand_has_valid_jedecid(chip);
+
+	/*
+	 * We only support read-retry for 1xnm NANDs, and those NANDs all
+	 * expose a valid JEDEC ID.
+	 */
+	if (valid_jedecid) {
+		u8 nand_tech = chip->id.data[5] >> 4;
+
+		/* 1xnm technology */
+		if (nand_tech == 4) {
+			for (i = 0; i < ARRAY_SIZE(hynix_mlc_1xnm_rr_otps);
+			     i++) {
+
+				/*
+				 * FIXME: Hynix recommend to copy the
+				 * read-retry OTP area into a normal page.
+				 */
+				ret = hynix_mlc_1xnm_rr_init(chip,
+						hynix_mlc_1xnm_rr_otps);
+				if (!ret)
+					break;
+			}
+		}
+	}
+
+	if (ret)
+		pr_warn("failed to initialize read-retry infrastructure");
+
+	return 0;
+}
+
 static void hynix_nand_extract_oobsize(struct nand_chip *chip,
 				       bool valid_jedecid)
 {
@@ -254,17 +587,43 @@ static void hynix_nand_decode_id(struct nand_chip *chip)
 	hynix_nand_extract_scrambling_requirements(chip, valid_jedecid);
 }
 
+static void hynix_nand_cleanup(struct nand_chip *chip)
+{
+	struct hynix_nand *hynix = nand_get_manufacturer_data(chip);
+
+	if (!hynix)
+		return;
+
+	kfree(hynix->read_retry);
+	kfree(hynix);
+	nand_set_manufacturer_data(chip, NULL);
+}
+
 static int hynix_nand_init(struct nand_chip *chip)
 {
+	struct hynix_nand *hynix;
+	int ret;
+
 	if (!nand_is_slc(chip))
 		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
 	else
 		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
 
-	return 0;
+	hynix = kzalloc(sizeof(*hynix), GFP_KERNEL);
+	if (!hynix)
+		return -ENOMEM;
+
+	nand_set_manufacturer_data(chip, hynix);
+
+	ret = hynix_nand_rr_init(chip);
+	if (ret)
+		hynix_nand_cleanup(chip);
+
+	return ret;
 }
 
 const struct nand_manufacturer_ops hynix_nand_manuf_ops = {
 	.detect = hynix_nand_decode_id,
 	.init = hynix_nand_init,
+	.cleanup = hynix_nand_cleanup,
 };
-- 
2.7.4

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

* Re: [PATCH v2 14/15] mtd: nand: hynix: rework NAND ID decoding to extract more information
  2016-06-08 13:00 ` [PATCH v2 14/15] mtd: nand: hynix: rework NAND ID decoding to extract more information Boris Brezillon
@ 2016-06-08 14:34   ` kbuild test robot
  0 siblings, 0 replies; 24+ messages in thread
From: kbuild test robot @ 2016-06-08 14:34 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: kbuild-all, David Woodhouse, Brian Norris, linux-mtd,
	Boris Brezillon, Richard Weinberger, Hans de Goede, linux-kernel,
	Aleksei Mamlin

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

Hi,

[auto build test ERROR on mtd/master]
[also build test ERROR on v4.7-rc2 next-20160608]
[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/20160608-210755
base:   git://git.infradead.org/linux-mtd.git master
config: x86_64-randconfig-s3-06081945 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/mtd/nand/nand_hynix.c: In function 'hynix_nand_decode_id':
>> drivers/mtd/nand/nand_hynix.c:239:20: error: 'SZ_1M' undeclared (first use in this function)
      mtd->erasesize = SZ_1M << tmp;
                       ^~~~~
   drivers/mtd/nand/nand_hynix.c:239:20: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/mtd/nand/nand_hynix.c:241:20: error: 'SZ_512K' undeclared (first use in this function)
      mtd->erasesize = SZ_512K + SZ_256K;
                       ^~~~~~~
>> drivers/mtd/nand/nand_hynix.c:241:30: error: 'SZ_256K' undeclared (first use in this function)
      mtd->erasesize = SZ_512K + SZ_256K;
                                 ^~~~~~~
>> drivers/mtd/nand/nand_hynix.c:243:20: error: 'SZ_128K' undeclared (first use in this function)
      mtd->erasesize = SZ_128K << tmp;
                       ^~~~~~~

vim +/SZ_1M +239 drivers/mtd/nand/nand_hynix.c

   233		 * we start counting at 128KiB and shift this value the content of
   234		 * ID[3][4:5].
   235		 * The only exception is when ID[3][4:5] == 3 and ID[3][7] == 0, in
   236		 * this case the erasesize is set to 768KiB.
   237		 */
   238		if (chip->id.data[3] & 0x80)
 > 239			mtd->erasesize = SZ_1M << tmp;
   240		else if (tmp == 3)
 > 241			mtd->erasesize = SZ_512K + SZ_256K;
   242		else
 > 243			mtd->erasesize = SZ_128K << tmp;
   244	
   245		/*
   246		 * Modern Toggle DDR NANDs have a valid JEDECID even though they are

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 26572 bytes --]

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

* Re: [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps
  2016-06-08 13:00 ` [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps Boris Brezillon
@ 2016-06-18  9:23   ` Richard Weinberger
  2016-06-18 11:31     ` Boris Brezillon
  2016-06-18 11:34     ` Boris Brezillon
  0 siblings, 2 replies; 24+ messages in thread
From: Richard Weinberger @ 2016-06-18  9:23 UTC (permalink / raw)
  To: Boris Brezillon, David Woodhouse, Brian Norris, linux-mtd
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin

Boris,

Am 08.06.2016 um 15:00 schrieb Boris Brezillon:
> A lot of NANDs are implementing generic features in a non-generic way,
> or are providing advanced auto-detection logic where the NAND ID bytes
> meaning changes with the NAND generation.
> 
> Providing this vendor specific initialization step will allow us to get
> rid of the full ids in the nand_ids table or all the vendor specific
> cases added over the time in the generic NAND ID decoding logic.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/nand/nand_base.c | 42 ++++++++++++++++++++++++++++++++----------
>  include/linux/mtd/nand.h     | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 95e9a8e..0a7d1c6 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3580,7 +3580,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
>   * 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 nand_chip *chip)
> +void nand_decode_ext_id(struct nand_chip *chip)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	int extid, id_len = chip->id.len;
> @@ -3705,6 +3705,7 @@ static void nand_decode_ext_id(struct nand_chip *chip)
>  
>  	}
>  }
> +EXPORT_SYMBOL_GPL(nand_decode_ext_id);
>  
>  /*
>   * Old devices have chip data hardcoded in the device ID table. nand_decode_id
> @@ -3815,7 +3816,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	int busw;
> -	int i, maf_idx;
> +	int i, maf_idx, ret;
>  	u8 *id_data = chip->id.data;
>  	u8 maf_id, dev_id;
>  
> @@ -3856,6 +3857,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
>  
>  	chip->id.len = nand_id_len(id_data, 8);
>  
> +	/* Try to identify manufacturer */
> +	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
> +		if (nand_manuf_ids[maf_idx].id == maf_id)
> +			break;
> +	}
> +
> +	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
> +

Instead of checking on every access whether chip->manufacturer.ops is present, just
assign a nop field.
i.e.

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 55f3ab8..aadebe7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3703,6 +3703,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)

 	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;

+	if (!chip->manufacturer.ops)
+		/* assign no operations */
+		chip->manufacturer.ops = nand_manuf_ids[0].ops;
+
 	if (!type)
 		type = nand_flash_ids;

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index f1476ff..cdd26af 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -173,6 +173,8 @@ extern const struct nand_manufacturer_ops micron_nand_manuf_ops;
 extern const struct nand_manufacturer_ops amd_nand_manuf_ops;
 extern const struct nand_manufacturer_ops macronix_nand_manuf_ops;

+static const struct nand_manufacturer_ops no_ops;
+
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
 	{NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
@@ -188,7 +190,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_SANDISK, "SanDisk"},
 	{NAND_MFR_INTEL, "Intel"},
 	{NAND_MFR_ATO, "ATO"},
-	{0x0, "Unknown"}
+	{0x0, "Unknown", &no_ops}
 };

 EXPORT_SYMBOL(nand_manuf_ids);


>  	if (!type)
>  		type = nand_flash_ids;
>  
> @@ -3903,8 +3912,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
>  	chip->chipsize = (uint64_t)type->chipsize << 20;
>  
>  	if (!type->pagesize) {
> -		/* Decode parameters from extended ID */
> -		nand_decode_ext_id(chip);
> +		/*
> +		 * Try manufacturer detection if available and use
> +		 * nand_decode_ext_id() otherwise.
> +		 */
> +		if (chip->manufacturer.ops->detect)

You need to check here for chip->manufacturer.ops first.

Thanks,
//richard

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

* Re: [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps
  2016-06-18  9:23   ` Richard Weinberger
@ 2016-06-18 11:31     ` Boris Brezillon
  2016-06-18 11:40       ` Boris Brezillon
  2016-06-18 11:34     ` Boris Brezillon
  1 sibling, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2016-06-18 11:31 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: David Woodhouse, Brian Norris, linux-mtd, Hans de Goede,
	linux-kernel, Aleksei Mamlin

On Sat, 18 Jun 2016 11:23:01 +0200
Richard Weinberger <richard@nod.at> wrote:

> Boris,
> 
> Am 08.06.2016 um 15:00 schrieb Boris Brezillon:
> > A lot of NANDs are implementing generic features in a non-generic way,
> > or are providing advanced auto-detection logic where the NAND ID bytes
> > meaning changes with the NAND generation.
> > 
> > Providing this vendor specific initialization step will allow us to get
> > rid of the full ids in the nand_ids table or all the vendor specific
> > cases added over the time in the generic NAND ID decoding logic.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  drivers/mtd/nand/nand_base.c | 42 ++++++++++++++++++++++++++++++++----------
> >  include/linux/mtd/nand.h     | 36 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 68 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 95e9a8e..0a7d1c6 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -3580,7 +3580,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
> >   * 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 nand_chip *chip)
> > +void nand_decode_ext_id(struct nand_chip *chip)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> >  	int extid, id_len = chip->id.len;
> > @@ -3705,6 +3705,7 @@ static void nand_decode_ext_id(struct nand_chip *chip)
> >  
> >  	}
> >  }
> > +EXPORT_SYMBOL_GPL(nand_decode_ext_id);
> >  
> >  /*
> >   * Old devices have chip data hardcoded in the device ID table. nand_decode_id
> > @@ -3815,7 +3816,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> >  	int busw;
> > -	int i, maf_idx;
> > +	int i, maf_idx, ret;
> >  	u8 *id_data = chip->id.data;
> >  	u8 maf_id, dev_id;
> >  
> > @@ -3856,6 +3857,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> >  
> >  	chip->id.len = nand_id_len(id_data, 8);
> >  
> > +	/* Try to identify manufacturer */
> > +	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
> > +		if (nand_manuf_ids[maf_idx].id == maf_id)
> > +			break;
> > +	}
> > +
> > +	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
> > +  
> 
> Instead of checking on every access whether chip->manufacturer.ops is present, just
> assign a nop field.
> i.e.
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 55f3ab8..aadebe7 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3703,6 +3703,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> 
>  	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
> 
> +	if (!chip->manufacturer.ops)
> +		/* assign no operations */
> +		chip->manufacturer.ops = nand_manuf_ids[0].ops;
> +
>  	if (!type)
>  		type = nand_flash_ids;
> 
> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> index f1476ff..cdd26af 100644
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -173,6 +173,8 @@ extern const struct nand_manufacturer_ops micron_nand_manuf_ops;
>  extern const struct nand_manufacturer_ops amd_nand_manuf_ops;
>  extern const struct nand_manufacturer_ops macronix_nand_manuf_ops;
> 
> +static const struct nand_manufacturer_ops no_ops;
> +
>  struct nand_manufacturers nand_manuf_ids[] = {
>  	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
>  	{NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
> @@ -188,7 +190,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
>  	{NAND_MFR_SANDISK, "SanDisk"},
>  	{NAND_MFR_INTEL, "Intel"},
>  	{NAND_MFR_ATO, "ATO"},
> -	{0x0, "Unknown"}
> +	{0x0, "Unknown", &no_ops}
>  };
> 
>  EXPORT_SYMBOL(nand_manuf_ids);
> 

Sorry, but I don't see any added value in adding this no_ops instance.
The NULL value is supposed to represent no_ops.
That's not like this path was critical: it's only call once during NAND
initialization.


How about adding the following helpers instead:

static inline bool nand_has_manufacturer_ops(struct nand_chip * chip)
{
	return chip->manufacturer.ops != NULL;
}

static inline void nand_manufacturer_detect(struct nand_chip * chip)
{
	if (!chip->manufacturer.ops || chip->manufacturer.ops->detect)
		return;

	chip->manufacturer.ops->detect(chip);
}

static inline int nand_manufacturer_init(struct nand_chip * chip)
{
	if (!chip->manufacturer.ops || chip->manufacturer.ops->init)
		return 0;

	return chip->manufacturer.ops->init(chip);
}

> 
> >  	if (!type)
> >  		type = nand_flash_ids;
> >  
> > @@ -3903,8 +3912,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> >  	chip->chipsize = (uint64_t)type->chipsize << 20;
> >  
> >  	if (!type->pagesize) {
> > -		/* Decode parameters from extended ID */
> > -		nand_decode_ext_id(chip);
> > +		/*
> > +		 * Try manufacturer detection if available and use
> > +		 * nand_decode_ext_id() otherwise.
> > +		 */
> > +		if (chip->manufacturer.ops->detect)  
> 
> You need to check here for chip->manufacturer.ops first.

Correct.

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

* Re: [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps
  2016-06-18  9:23   ` Richard Weinberger
  2016-06-18 11:31     ` Boris Brezillon
@ 2016-06-18 11:34     ` Boris Brezillon
  2016-06-18 12:34       ` Richard Weinberger
  1 sibling, 1 reply; 24+ messages in thread
From: Boris Brezillon @ 2016-06-18 11:34 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: David Woodhouse, Brian Norris, linux-mtd, Hans de Goede,
	linux-kernel, Aleksei Mamlin

On Sat, 18 Jun 2016 11:23:01 +0200
Richard Weinberger <richard@nod.at> wrote:

> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 55f3ab8..aadebe7 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3703,6 +3703,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> 
>  	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
> 
> +	if (!chip->manufacturer.ops)
> +		/* assign no operations */
> +		chip->manufacturer.ops = nand_manuf_ids[0].ops;
> +

BTW, this is wrong, the manufacturer id code is not the index of the
nand_manuf_ids[] table ;). If we go for this option, we should probably
declare no_ops in nand_base.c and assign it here:

		chip->manufacturer.ops = nand_manuf_no_ops;

>  	if (!type)
>  		type = nand_flash_ids;
> 

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

* Re: [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps
  2016-06-18 11:31     ` Boris Brezillon
@ 2016-06-18 11:40       ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-18 11:40 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: David Woodhouse, Brian Norris, linux-mtd, Hans de Goede,
	linux-kernel, Aleksei Mamlin

On Sat, 18 Jun 2016 13:31:21 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Sat, 18 Jun 2016 11:23:01 +0200
> Richard Weinberger <richard@nod.at> wrote:
> 
> > Boris,
> > 
> > Am 08.06.2016 um 15:00 schrieb Boris Brezillon:  
> > > A lot of NANDs are implementing generic features in a non-generic way,
> > > or are providing advanced auto-detection logic where the NAND ID bytes
> > > meaning changes with the NAND generation.
> > > 
> > > Providing this vendor specific initialization step will allow us to get
> > > rid of the full ids in the nand_ids table or all the vendor specific
> > > cases added over the time in the generic NAND ID decoding logic.
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > ---
> > >  drivers/mtd/nand/nand_base.c | 42 ++++++++++++++++++++++++++++++++----------
> > >  include/linux/mtd/nand.h     | 36 ++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 68 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > > index 95e9a8e..0a7d1c6 100644
> > > --- a/drivers/mtd/nand/nand_base.c
> > > +++ b/drivers/mtd/nand/nand_base.c
> > > @@ -3580,7 +3580,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
> > >   * 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 nand_chip *chip)
> > > +void nand_decode_ext_id(struct nand_chip *chip)
> > >  {
> > >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > >  	int extid, id_len = chip->id.len;
> > > @@ -3705,6 +3705,7 @@ static void nand_decode_ext_id(struct nand_chip *chip)
> > >  
> > >  	}
> > >  }
> > > +EXPORT_SYMBOL_GPL(nand_decode_ext_id);
> > >  
> > >  /*
> > >   * Old devices have chip data hardcoded in the device ID table. nand_decode_id
> > > @@ -3815,7 +3816,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> > >  {
> > >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > >  	int busw;
> > > -	int i, maf_idx;
> > > +	int i, maf_idx, ret;
> > >  	u8 *id_data = chip->id.data;
> > >  	u8 maf_id, dev_id;
> > >  
> > > @@ -3856,6 +3857,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> > >  
> > >  	chip->id.len = nand_id_len(id_data, 8);
> > >  
> > > +	/* Try to identify manufacturer */
> > > +	for (maf_idx = 0; nand_manuf_ids[maf_idx].id != 0x0; maf_idx++) {
> > > +		if (nand_manuf_ids[maf_idx].id == maf_id)
> > > +			break;
> > > +	}
> > > +
> > > +	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
> > > +    
> > 
> > Instead of checking on every access whether chip->manufacturer.ops is present, just
> > assign a nop field.
> > i.e.
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 55f3ab8..aadebe7 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -3703,6 +3703,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> > 
> >  	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
> > 
> > +	if (!chip->manufacturer.ops)
> > +		/* assign no operations */
> > +		chip->manufacturer.ops = nand_manuf_ids[0].ops;
> > +
> >  	if (!type)
> >  		type = nand_flash_ids;
> > 
> > diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> > index f1476ff..cdd26af 100644
> > --- a/drivers/mtd/nand/nand_ids.c
> > +++ b/drivers/mtd/nand/nand_ids.c
> > @@ -173,6 +173,8 @@ extern const struct nand_manufacturer_ops micron_nand_manuf_ops;
> >  extern const struct nand_manufacturer_ops amd_nand_manuf_ops;
> >  extern const struct nand_manufacturer_ops macronix_nand_manuf_ops;
> > 
> > +static const struct nand_manufacturer_ops no_ops;
> > +
> >  struct nand_manufacturers nand_manuf_ids[] = {
> >  	{NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
> >  	{NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
> > @@ -188,7 +190,7 @@ struct nand_manufacturers nand_manuf_ids[] = {
> >  	{NAND_MFR_SANDISK, "SanDisk"},
> >  	{NAND_MFR_INTEL, "Intel"},
> >  	{NAND_MFR_ATO, "ATO"},
> > -	{0x0, "Unknown"}
> > +	{0x0, "Unknown", &no_ops}
> >  };
> > 
> >  EXPORT_SYMBOL(nand_manuf_ids);
> >   
> 
> Sorry, but I don't see any added value in adding this no_ops instance.
> The NULL value is supposed to represent no_ops.
> That's not like this path was critical: it's only call once during NAND
> initialization.
> 
> 
> How about adding the following helpers instead:
> 
> static inline bool nand_has_manufacturer_ops(struct nand_chip * chip)
> {
> 	return chip->manufacturer.ops != NULL;
> }

I meant

static inline bool nand_has_manufacturer_detect(struct nand_chip * chip)
{
	return chip->manufacturer.ops && chip->manufacturer.ops->detect;
}

> 
> static inline void nand_manufacturer_detect(struct nand_chip * chip)
> {
> 	if (!chip->manufacturer.ops || chip->manufacturer.ops->detect)
> 		return;
> 
> 	chip->manufacturer.ops->detect(chip);
> }
> 
> static inline int nand_manufacturer_init(struct nand_chip * chip)
> {
> 	if (!chip->manufacturer.ops || chip->manufacturer.ops->init)
> 		return 0;
> 
> 	return chip->manufacturer.ops->init(chip);
> }
> 
> >   
> > >  	if (!type)
> > >  		type = nand_flash_ids;
> > >  
> > > @@ -3903,8 +3912,14 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> > >  	chip->chipsize = (uint64_t)type->chipsize << 20;
> > >  
> > >  	if (!type->pagesize) {
> > > -		/* Decode parameters from extended ID */
> > > -		nand_decode_ext_id(chip);
> > > +		/*
> > > +		 * Try manufacturer detection if available and use
> > > +		 * nand_decode_ext_id() otherwise.
> > > +		 */
> > > +		if (chip->manufacturer.ops->detect)    
> > 
> > You need to check here for chip->manufacturer.ops first.  
> 
> Correct.

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

* Re: [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps
  2016-06-18 11:34     ` Boris Brezillon
@ 2016-06-18 12:34       ` Richard Weinberger
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Weinberger @ 2016-06-18 12:34 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Brian Norris, linux-mtd, Hans de Goede,
	linux-kernel, Aleksei Mamlin



Am 18.06.2016 um 13:34 schrieb Boris Brezillon:
> On Sat, 18 Jun 2016 11:23:01 +0200
> Richard Weinberger <richard@nod.at> wrote:
> 
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 55f3ab8..aadebe7 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -3703,6 +3703,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
>>
>>  	chip->manufacturer.ops = nand_manuf_ids[maf_idx].ops;
>>
>> +	if (!chip->manufacturer.ops)
>> +		/* assign no operations */
>> +		chip->manufacturer.ops = nand_manuf_ids[0].ops;
>> +
> 
> BTW, this is wrong, the manufacturer id code is not the index of the
> nand_manuf_ids[] table ;). If we go for this option, we should probably
> declare no_ops in nand_base.c and assign it here:
> 

Oh, yes. You are right.
Anyway, I think having a wrapper is also a good solution.

Thanks,
//richard

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

* Re: [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization
  2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
                   ` (14 preceding siblings ...)
  2016-06-08 13:00 ` [PATCH v2 15/15] mtd: nand: hynix: add read-retry support for 1x nm MLC NANDs Boris Brezillon
@ 2016-06-18 22:09 ` Richard Weinberger
  2016-06-19  8:34   ` Boris Brezillon
  15 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2016-06-18 22:09 UTC (permalink / raw)
  To: Boris Brezillon, David Woodhouse, Brian Norris, linux-mtd
  Cc: Hans de Goede, linux-kernel, Aleksei Mamlin



Am 08.06.2016 um 15:00 schrieb Boris Brezillon:
> Hello,
> 
> This patch series is a step forward in supporting vendor-specific
> functionalities.
> This series is mainly moving vendor-specific initialization or
> detection code out of the core, but also introduces an infrastructure
> allowing support for vendor-specific features.
> 
> While those features might seem useless to most users, some of them are
> actually required on modern MLC/TLC NANDs (this is the case of read-retry
> support, which AFAICT has not been standardized by the JEDEC consortium).
> 
> Now, let's detail what's inside this patch-set.
> 
> Patches 1 to 4 are simple reworks simplifying auto-detection function
> prototypes, and clarifying their purpose.
> 
> Patch 5 is introducing the vendor-specific initialization
> infrastructure.
> 
> Patch 6 is removing the MTD_NAND_IDS Kconfig option to avoid creating
> a nand_ids.ko module when MTD_NAND is enabled as a module. This prevents
> a future cross-dependency between nand.ko where all vendor specific
> code will rely and nand_ids.ko which will reference vendor-specific ops
> in its manufacturer table, which in turn is referenced by the core code
> linked in nand.ko.
> 
> Patches 7 to 12 are moving vendor-specific code into their respective
> nand_<vendor>.c files.
> 
> Patch 13 is taking a patch proposed by Hans and adding support for ECC
> requirements extraction from the samsung extended IDs. It seems to apply
> to all Samsung MLCs, but even if it's not the case, the detection code
> should be improved to support the new formats.
> 
> Patch 14 is adding support for advanced NAND ID decoding to the Hynix
> driver (OOB size, ECC and scrambling requirements extraction). Again
> this detection code might be incomplete, but I'd like people to extend
> it if required rather than adding new full-id entries in the nand_ids
> table.
> 
> And finally, patch 15 is showing how useful this vendor-specific stuff
> can be by implementing read-retry support for Hynix 1x nm MLCs. And
> trust me, you don't want to try using such a NAND without read-retry
> support ;).
> 
> As always, I'm open to any suggestion to improve this vendor-specific
> infrastructure, so please review the code :).

Series looks good to me. :-)

BTW: I wonder whether this work can also be used to support
Micron On-Die-ECC and Toshiba BENAND in a proper way.

Thanks,
//richard

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

* Re: [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization
  2016-06-18 22:09 ` [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Richard Weinberger
@ 2016-06-19  8:34   ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2016-06-19  8:34 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: David Woodhouse, Brian Norris, linux-mtd, Hans de Goede,
	linux-kernel, Aleksei Mamlin

On Sun, 19 Jun 2016 00:09:42 +0200
Richard Weinberger <richard@nod.at> wrote:

> Am 08.06.2016 um 15:00 schrieb Boris Brezillon:
> > Hello,
> > 
> > This patch series is a step forward in supporting vendor-specific
> > functionalities.
> > This series is mainly moving vendor-specific initialization or
> > detection code out of the core, but also introduces an infrastructure
> > allowing support for vendor-specific features.
> > 
> > While those features might seem useless to most users, some of them are
> > actually required on modern MLC/TLC NANDs (this is the case of read-retry
> > support, which AFAICT has not been standardized by the JEDEC consortium).
> > 
> > Now, let's detail what's inside this patch-set.
> > 
> > Patches 1 to 4 are simple reworks simplifying auto-detection function
> > prototypes, and clarifying their purpose.
> > 
> > Patch 5 is introducing the vendor-specific initialization
> > infrastructure.
> > 
> > Patch 6 is removing the MTD_NAND_IDS Kconfig option to avoid creating
> > a nand_ids.ko module when MTD_NAND is enabled as a module. This prevents
> > a future cross-dependency between nand.ko where all vendor specific
> > code will rely and nand_ids.ko which will reference vendor-specific ops
> > in its manufacturer table, which in turn is referenced by the core code
> > linked in nand.ko.
> > 
> > Patches 7 to 12 are moving vendor-specific code into their respective
> > nand_<vendor>.c files.
> > 
> > Patch 13 is taking a patch proposed by Hans and adding support for ECC
> > requirements extraction from the samsung extended IDs. It seems to apply
> > to all Samsung MLCs, but even if it's not the case, the detection code
> > should be improved to support the new formats.
> > 
> > Patch 14 is adding support for advanced NAND ID decoding to the Hynix
> > driver (OOB size, ECC and scrambling requirements extraction). Again
> > this detection code might be incomplete, but I'd like people to extend
> > it if required rather than adding new full-id entries in the nand_ids
> > table.
> > 
> > And finally, patch 15 is showing how useful this vendor-specific stuff
> > can be by implementing read-retry support for Hynix 1x nm MLCs. And
> > trust me, you don't want to try using such a NAND without read-retry
> > support ;).
> > 
> > As always, I'm open to any suggestion to improve this vendor-specific
> > infrastructure, so please review the code :).  
> 
> Series looks good to me. :-)
> 
> BTW: I wonder whether this work can also be used to support
> Micron On-Die-ECC and Toshiba BENAND in a proper way.

Hehe, I was almost sure someone would ask this question, and yes this
infrastructure would partly solve the problem, but this would still
require patching NAND controller drivers.

Here are the aspects I can remember (there might be others):

1/ Some drivers are just supporting a limited set of NAND operations in
   their ->cmdfunc() implementation, which means they're unlikely to
   support the private ENABLE/DISABLE_ECC commands.
2/ We'd have to add a new ECC mode (NAND_ECC_ON_DIE?), and patch NAND
   controller drivers to not blindly set NAND_ECC_HW.
3/ NAND controller drivers should not change any of the chip->ecc.xxx
   fields if NAND_ECC_ON_DIE has been selected.

So, nothing impossible here, but this clearly requires some work.
Moreover, I'm planning to rework this whole ECC thing at some point, to
let the core select the mode (and appropriate implementation) instead of
leaving this responsibility to the NAND controller driver. Note that
this should not dissuade you from adding on-die ECC support before this
change, I'm just sharing my plans.

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

end of thread, other threads:[~2016-06-19  8:34 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 13:00 [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 01/15] mtd: nand: get rid of the mtd parameter in all auto-detection functions Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 02/15] mtd: nand: store nand ID in struct nand_chip Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 03/15] mtd: nand: get rid of busw parameter Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 04/15] mtd: nand: rename nand_get_flash_type() into nand_detect() Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 05/15] mtd: nand: add manufacturer specific initialization/detection steps Boris Brezillon
2016-06-18  9:23   ` Richard Weinberger
2016-06-18 11:31     ` Boris Brezillon
2016-06-18 11:40       ` Boris Brezillon
2016-06-18 11:34     ` Boris Brezillon
2016-06-18 12:34       ` Richard Weinberger
2016-06-08 13:00 ` [PATCH v2 06/15] mtd: nand: kill the MTD_NAND_IDS Kconfig option Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 07/15] mtd: nand: move Samsung specific init/detection logic in nand_samsung.c Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 08/15] mtd: nand: move Hynix specific init/detection logic in nand_hynix.c Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 09/15] mtd: nand: move Toshiba specific init/detection logic in nand_toshiba.c Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 10/15] mtd: nand: move Micron specific init logic in nand_micron.c Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 11/15] mtd: nand: move AMD/Spansion specific init/detection logic in nand_amd.c Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 12/15] mtd: nand: move Macronix specific initialization in nand_macronix.c Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 13/15] mtd: nand: samsung: retrieve ECC requirements from extended ID Boris Brezillon
2016-06-08 13:00 ` [PATCH v2 14/15] mtd: nand: hynix: rework NAND ID decoding to extract more information Boris Brezillon
2016-06-08 14:34   ` kbuild test robot
2016-06-08 13:00 ` [PATCH v2 15/15] mtd: nand: hynix: add read-retry support for 1x nm MLC NANDs Boris Brezillon
2016-06-18 22:09 ` [PATCH v2 00/15] mtd: nand: allow vendor specific detection/initialization Richard Weinberger
2016-06-19  8:34   ` Boris Brezillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).