[RFC PATCH 0/2] mtd: nand: add manafacturer specific handling

[RFC PATCH 0/2] mtd: nand: add manafacturer specific handling

[Boris BREZILLON]

Hello,

This is a proposal to add support for manufacturer specific handling of
NAND chips.

Currently the only ones allowed to modify how NAND chips are accessed are
NAND controllers (by modifying the nand_chip callbacks) and NAND core code
(by setting default callbacks when these are not specified by the NAND
controller).

This series adds support for manufacturer/vendor specific handling, which is
particularly useful for read retry implementation that are (at least for
the moment) manufacturer specific.

I know there was a debate on how we should support all these READ RETRY
implementations (see http://permalink.gmane.org/gmane.linux.drivers.mtd/50603),
so please tell if I'm wrong.

Best Regards,

Boris


Boris BREZILLON (2):
  mtd: nand: add manufacturer specific init infrastructure
  mtd: nand: add hynix specific initializer

 drivers/mtd/nand/Makefile     |    2 +-
 drivers/mtd/nand/nand_base.c  |    7 ++
 drivers/mtd/nand/nand_hynix.c |  151 +++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/nand_ids.c   |    3 +-
 include/linux/mtd/nand.h      |    6 ++
 5 files changed, 167 insertions(+), 2 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_hynix.c

-- 
1.7.9.5

[RFC PATCH 1/2] mtd: nand: add manufacturer specific init infrastructure

[Boris BREZILLON]

Add new fields in nand_manufacturers and nand_chip struct to provide
manufacturer specific handling like read retries.

Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
---
 drivers/mtd/nand/nand_base.c |    7 +++++++
 include/linux/mtd/nand.h     |    4 ++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 9715a7b..8b16479 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3610,6 +3610,13 @@ ident_done:
 	if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
 		chip->cmdfunc = nand_command_lp;
 
+	if (nand_manuf_ids[maf_idx].init) {
+		int err;
+		err = nand_manuf_ids[maf_idx].init(mtd, id_data);
+		if (err)
+			return ERR_PTR(err);
+	}
+
 	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,
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 32f8612..6189312 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -575,6 +575,9 @@ struct nand_chip {
 	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
 			int feature_addr, uint8_t *subfeature_para);
 	int (*setup_read_retry)(struct mtd_info *mtd, int retry_mode);
+	void (*manuf_cleanup)(struct mtd_info *mtd);
+
+	void *manuf_priv;
 
 	int chip_delay;
 	unsigned int options;
@@ -720,6 +723,7 @@ struct nand_flash_dev {
 struct nand_manufacturers {
 	int id;
 	char *name;
+	int (*init)(struct mtd_info *mtd, const uint8_t *id);
 };
 
 extern struct nand_flash_dev nand_flash_ids[];
-- 
1.7.9.5

[RFC PATCH 2/2] mtd: nand: add hynix specific initializer

[Boris BREZILLON]

Add an hynix initializer to manage read retries on h27uxgt8t2a chips.

Signed-off-by: Boris BREZILLON <b.brezillon.dev@gmail.com>
---
 drivers/mtd/nand/Makefile     |    2 +-
 drivers/mtd/nand/nand_hynix.c |  151 +++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/nand_ids.c   |    3 +-
 include/linux/mtd/nand.h      |    2 +
 4 files changed, 156 insertions(+), 2 deletions(-)
 create mode 100644 drivers/mtd/nand/nand_hynix.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 542b568..c970ce7 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -5,7 +5,7 @@
 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_NAND_IDS)		+= nand_ids.o nand_hynix.o
 obj-$(CONFIG_MTD_SM_COMMON) 		+= sm_common.o
 
 obj-$(CONFIG_MTD_NAND_CAFE)		+= cafe_nand.o
diff --git a/drivers/mtd/nand/nand_hynix.c b/drivers/mtd/nand/nand_hynix.c
new file mode 100644
index 0000000..c1d0017
--- /dev/null
+++ b/drivers/mtd/nand/nand_hynix.c
@@ -0,0 +1,151 @@
+/*
+ * Copyright (C) 2014 Boris BREZILLON <b.brezillon.dev@gmail.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/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/slab.h>
+
+static u8 h27ucg8t2a_read_retry_regs[] = {
+	0xcc, 0xbf, 0xaa, 0xab, 0xcd, 0xad, 0xae, 0xaf
+};
+
+struct hynix_read_retry {
+	u8 *regs;
+	u8 values[64];
+};
+
+struct hynix_nand {
+	struct hynix_read_retry read_retry;
+};
+
+int nand_setup_read_retry_hynix(struct mtd_info *mtd, int retry_mode)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hynix_nand *hynix = chip->manuf_priv;
+	int offset = retry_mode * 8;
+	int status;
+	int i;
+
+	chip->cmdfunc(mtd, 0x36, -1, -1);
+	for (i = 0; i < 8; i++) {
+		int column = hynix->read_retry.regs[i];
+		column |= column << 8;
+		chip->cmdfunc(mtd, NAND_CMD_NONE, column, -1);
+		chip->write_byte(mtd, hynix->read_retry.values[offset + i]);
+	}
+	chip->cmdfunc(mtd, 0x16, -1, -1);
+
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
+	return 0;
+}
+
+static void h27ucg8t2a_cleanup(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	kfree(chip->manuf_priv);
+}
+
+static int h27ucg8t2a_init(struct mtd_info *mtd, const uint8_t *id)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hynix_nand *hynix;
+	u8 buf[1024];
+	int i, j;
+	int ret;
+
+	chip->select_chip(mtd, 0);
+	chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+	chip->cmdfunc(mtd, 0x36, 0xff, -1);
+	chip->write_byte(mtd, 0x40);
+	chip->cmdfunc(mtd, NAND_CMD_NONE, 0xcc, -1);
+	chip->write_byte(mtd, 0x4d);
+	chip->cmdfunc(mtd, 0x16, -1, -1);
+	chip->cmdfunc(mtd, 0x17, -1, -1);
+	chip->cmdfunc(mtd, 0x04, -1, -1);
+	chip->cmdfunc(mtd, 0x19, -1, -1);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, 0x200);
+
+	chip->read_buf(mtd, buf, 2);
+	if (buf[0] != 0x8 || buf[1] != 0x8)
+		return -EINVAL;
+	chip->read_buf(mtd, buf, 1024);
+
+	ret = 0;
+	for (j = 0; j < 8; j++) {
+		for (i = 0; i < 64; i++) {
+			u8 *tmp = buf + (128 * j);
+			if ((tmp[i] | tmp[i + 64]) != 0xff) {
+				ret = -EINVAL;
+				break;
+			}
+		}
+
+		if (ret)
+			break;
+	}
+
+	chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+	chip->cmdfunc(mtd, 0x38, -1, -1);
+	chip->select_chip(mtd, -1);
+
+	if (!ret) {
+		hynix = kzalloc(sizeof(*hynix), GFP_KERNEL);
+		if (!hynix)
+			return -ENOMEM;
+
+		hynix->read_retry.regs = h27ucg8t2a_read_retry_regs;
+		memcpy(hynix->read_retry.values, buf, 64);
+		chip->manuf_priv = hynix;
+		chip->setup_read_retry = nand_setup_read_retry_hynix;
+		chip->read_retries = 8;
+		chip->manuf_cleanup = h27ucg8t2a_cleanup;
+	}
+
+	return ret;
+}
+
+struct hynix_nand_initializer {
+	u8 id[6];
+	int (*init)(struct mtd_info *mtd, const uint8_t *id);
+};
+
+struct hynix_nand_initializer initializers[] = {
+	{
+		.id = {NAND_MFR_HYNIX, 0xde, 0x94, 0xda, 0x74, 0xc4},
+		.init = h27ucg8t2a_init,
+	},
+};
+
+int hynix_nand_init(struct mtd_info *mtd, const uint8_t *id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(initializers); i++) {
+		struct hynix_nand_initializer *initializer = &initializers[i];
+		if (memcmp(id, initializer->id, sizeof(initializer->id)))
+			continue;
+
+		return initializer->init(mtd, id);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(hynix_nand_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Boris BREZILLON <b.brezillon.dev@gmail.com>");
+MODULE_DESCRIPTION("Hynix NAND specific code");
diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index daa2faa..8d373af 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -156,6 +156,7 @@ struct nand_flash_dev nand_flash_ids[] = {
 	{NULL}
 };
 
+
 /* Manufacturer IDs */
 struct nand_manufacturers nand_manuf_ids[] = {
 	{NAND_MFR_TOSHIBA, "Toshiba"},
@@ -164,7 +165,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_init},
 	{NAND_MFR_MICRON, "Micron"},
 	{NAND_MFR_AMD, "AMD/Spansion"},
 	{NAND_MFR_MACRONIX, "Macronix"},
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 6189312..389b3c5 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -729,6 +729,8 @@ struct nand_manufacturers {
 extern struct nand_flash_dev nand_flash_ids[];
 extern struct nand_manufacturers nand_manuf_ids[];
 
+int hynix_nand_init(struct mtd_info *mtd, const uint8_t *id);
+
 extern int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd);
 extern int nand_default_bbt(struct mtd_info *mtd);
 extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);
-- 
1.7.9.5

Re: [RFC PATCH 0/2] mtd: nand: add manafacturer specific handling

[Richard Weinberger]

On Mon, Feb 24, 2014 at 6:29 PM, Boris BREZILLON
<b.brezillon.dev@gmail.com> wrote:
> Hello,
>
> This is a proposal to add support for manufacturer specific handling of
> NAND chips.
>
> Currently the only ones allowed to modify how NAND chips are accessed are
> NAND controllers (by modifying the nand_chip callbacks) and NAND core code
> (by setting default callbacks when these are not specified by the NAND
> controller).
>
> This series adds support for manufacturer/vendor specific handling, which is
> particularly useful for read retry implementation that are (at least for
> the moment) manufacturer specific.
>
> I know there was a debate on how we should support all these READ RETRY
> implementations (see http://permalink.gmane.org/gmane.linux.drivers.mtd/50603),
> so please tell if I'm wrong.

Just realized that this got no feedback at all.
Brian?

-- 
Thanks,
//richard