[PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Ezequiel Garcia]

This commit uses the recently introduced SPI NAND framework to support
Micron MT29F and Gigadevice GD5F serial NAND devices. These two families
of devices are fairly similar and so they are supported with only minimal
differences.

The current support includes:

  * Page read and page program operations (using on-die ECC)
  * Page out-of-band read
  * Erase
  * Reset
  * Device status retrieval
  * Device ID retrieval

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/mtd/spi-nand/Kconfig           |  12 +
 drivers/mtd/spi-nand/Makefile          |   1 +
 drivers/mtd/spi-nand/spi-nand-device.c | 500 +++++++++++++++++++++++++++++++++
 3 files changed, 513 insertions(+)
 create mode 100644 drivers/mtd/spi-nand/spi-nand-device.c

diff --git a/drivers/mtd/spi-nand/Kconfig b/drivers/mtd/spi-nand/Kconfig
index 3868477..df29abe 100644
--- a/drivers/mtd/spi-nand/Kconfig
+++ b/drivers/mtd/spi-nand/Kconfig
@@ -4,3 +4,15 @@ menuconfig MTD_SPI_NAND
 	select MTD_NAND
 	help
 	  This is the framework for the SPI NAND.
+
+if MTD_SPI_NAND
+
+config MTD_SPI_NAND_DEVICES
+	tristate "Support for SPI NAND devices (MT29F, GD5F)"
+	default y
+	depends on MTD_SPI_NAND
+	help
+	  Select this option if you require support for the most common SPI NAND
+	  devices such as mt29f and gd5f.
+
+endif # MTD_SPI_NAND
diff --git a/drivers/mtd/spi-nand/Makefile b/drivers/mtd/spi-nand/Makefile
index d454c52..f4f95b7 100644
--- a/drivers/mtd/spi-nand/Makefile
+++ b/drivers/mtd/spi-nand/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_MTD_SPI_NAND)		+= spi-nand-base.o
+obj-$(CONFIG_MTD_SPI_NAND_DEVICES)	+= spi-nand-device.o
diff --git a/drivers/mtd/spi-nand/spi-nand-device.c b/drivers/mtd/spi-nand/spi-nand-device.c
new file mode 100644
index 0000000..73050f3
--- /dev/null
+++ b/drivers/mtd/spi-nand/spi-nand-device.c
@@ -0,0 +1,500 @@
+/*
+ * Copyright (C) 2014 Imagination Technologies Ltd.
+ *
+ * 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; version 2 of the License.
+ *
+ * Notes:
+ * 1. We avoid using a stack-allocated buffer for SPI messages. Using
+ *    a kmalloced buffer is probably better, given we shouldn't assume
+ *    any particular usage by SPI core.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nand.h>
+#include <linux/sizes.h>
+#include <linux/spi/spi.h>
+
+/* SPI NAND commands */
+#define	SPI_NAND_WRITE_ENABLE		0x06
+#define	SPI_NAND_WRITE_DISABLE		0x04
+#define	SPI_NAND_GET_FEATURE		0x0f
+#define	SPI_NAND_SET_FEATURE		0x1f
+#define	SPI_NAND_PAGE_READ		0x13
+#define	SPI_NAND_READ_CACHE		0x03
+#define	SPI_NAND_FAST_READ_CACHE	0x0b
+#define	SPI_NAND_READ_CACHE_X2		0x3b
+#define	SPI_NAND_READ_CACHE_X4		0x6b
+#define	SPI_NAND_READ_CACHE_DUAL_IO	0xbb
+#define	SPI_NAND_READ_CACHE_QUAD_IO	0xeb
+#define	SPI_NAND_READ_ID		0x9f
+#define	SPI_NAND_PROGRAM_LOAD		0x02
+#define	SPI_NAND_PROGRAM_LOAD4		0x32
+#define	SPI_NAND_PROGRAM_EXEC		0x10
+#define	SPI_NAND_PROGRAM_LOAD_RANDOM	0x84
+#define	SPI_NAND_PROGRAM_LOAD_RANDOM4	0xc4
+#define	SPI_NAND_BLOCK_ERASE		0xd8
+#define	SPI_NAND_RESET			0xff
+
+#define SPI_NAND_GD5F_READID_LEN	2
+#define SPI_NAND_MT29F_READID_LEN	2
+
+#define SPI_NAND_GD5F_ECC_MASK		(BIT(0) | BIT(1) | BIT(2))
+#define SPI_NAND_GD5F_ECC_UNCORR	(BIT(0) | BIT(1) | BIT(2))
+#define SPI_NAND_GD5F_ECC_SHIFT		4
+
+#define SPI_NAND_MT29F_ECC_MASK		(BIT(0) | BIT(1))
+#define SPI_NAND_MT29F_ECC_UNCORR	(BIT(1))
+#define SPI_NAND_MT29F_ECC_SHIFT		4
+
+static struct nand_ecclayout ecc_layout_gd5f = {
+	.eccbytes = 128,
+	.eccpos = {
+		128, 129, 130, 131, 132, 133, 134, 135,
+		136, 137, 138, 139, 140, 141, 142, 143,
+		144, 145, 146, 147, 148, 149, 150, 151,
+		152, 153, 154, 155, 156, 157, 158, 159,
+		160, 161, 162, 163, 164, 165, 166, 167,
+		168, 169, 170, 171, 172, 173, 174, 175,
+		176, 177, 178, 179, 180, 181, 182, 183,
+		184, 185, 186, 187, 188, 189, 190, 191,
+		192, 193, 194, 195, 196, 197, 198, 199,
+		200, 201, 202, 203, 204, 205, 206, 207,
+		208, 209, 210, 211, 212, 213, 214, 215,
+		216, 217, 218, 219, 220, 221, 222, 223,
+		224, 225, 226, 227, 228, 229, 230, 231,
+		232, 233, 234, 235, 236, 237, 238, 239,
+		240, 241, 242, 243, 244, 245, 246, 247,
+		248, 249, 250, 251, 252, 253, 254, 255
+	},
+	.oobfree = { {1, 127} }
+};
+
+static struct nand_ecclayout ecc_layout_mt29f = {
+	.eccbytes = 32,
+	.eccpos = {
+		8, 9, 10, 11, 12, 13, 14, 15,
+		24, 25, 26, 27, 28, 29, 30, 31,
+		40, 41, 42, 43, 44, 45, 46, 47,
+		56, 57, 58, 59, 60, 61, 62, 63,
+	 },
+};
+
+static struct nand_flash_dev spi_nand_flash_ids[] = {
+	{
+		.name = "SPI NAND 512MiB 3,3V",
+		.id = { NAND_MFR_GIGADEVICE, 0xb4 },
+		.chipsize = 512,
+		.pagesize = SZ_4K,
+		.erasesize = SZ_256K,
+		.id_len = 2,
+		.oobsize = 256,
+		.ecc.strength_ds = 8,
+		.ecc.step_ds = 512,
+		.ecc.layout = &ecc_layout_gd5f,
+	},
+	{
+		.name = "SPI NAND 512MiB 1,8V",
+		.id = { NAND_MFR_GIGADEVICE, 0xa4 },
+		.chipsize = 512,
+		.pagesize = SZ_4K,
+		.erasesize = SZ_256K,
+		.id_len = 2,
+		.oobsize = 256,
+		.ecc.strength_ds = 8,
+		.ecc.step_ds = 512,
+		.ecc.layout = &ecc_layout_gd5f,
+	},
+	{
+		.name = "SPI NAND 512MiB 3,3V",
+		.id = { NAND_MFR_MICRON, 0x32 },
+		.chipsize = 512,
+		.pagesize = SZ_2K,
+		.erasesize = SZ_128K,
+		.id_len = 2,
+		.oobsize = 64,
+		.ecc.strength_ds = 4,
+		.ecc.step_ds = 512,
+		.ecc.layout = &ecc_layout_mt29f,
+	},
+	{
+		.name = "SPI NAND 256MiB 3,3V",
+		.id = { NAND_MFR_MICRON, 0x22 },
+		.chipsize = 256,
+		.pagesize = SZ_2K,
+		.erasesize = SZ_128K,
+		.id_len = 2,
+		.oobsize = 64,
+		.ecc.strength_ds = 4,
+		.ecc.step_ds = 512,
+		.ecc.layout = &ecc_layout_mt29f,
+	},
+};
+
+enum spi_nand_device_variant {
+	SPI_NAND_GENERIC,
+	SPI_NAND_MT29F,
+	SPI_NAND_GD5F,
+};
+
+struct spi_nand_device_cmd {
+	u8 cmd;
+	u32 n_addr;
+	u8 addr[3];
+	u32 n_tx;
+	u8 *tx_buf;
+	u32 n_rx;
+	u8 *rx_buf;
+};
+
+struct spi_nand_device {
+	struct spi_nand	spi_nand;
+	struct spi_device *spi;
+
+	struct spi_nand_device_cmd cmd;
+};
+
+static int spi_nand_send_command(struct spi_device *spi, int command,
+				 struct spi_nand_device_cmd *cmd)
+{
+	struct spi_message message;
+	struct spi_transfer x[4];
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof(x));
+
+	/* Command */
+	cmd->cmd = command;
+	x[0].len = 1;
+	x[0].tx_buf = &cmd->cmd;
+	spi_message_add_tail(&x[0], &message);
+
+	/* Address */
+	if (cmd->n_addr) {
+		x[1].len = cmd->n_addr;
+		x[1].tx_buf = cmd->addr;
+		spi_message_add_tail(&x[1], &message);
+	}
+
+	/* Data to be transmitted */
+	if (cmd->n_tx) {
+		x[3].len = cmd->n_tx;
+		x[3].tx_buf = cmd->tx_buf;
+		spi_message_add_tail(&x[3], &message);
+	}
+
+	/* Data to be received */
+	if (cmd->n_rx) {
+		x[3].len = cmd->n_rx;
+		x[3].rx_buf = cmd->rx_buf;
+		spi_message_add_tail(&x[3], &message);
+	}
+
+	return spi_sync(spi, &message);
+}
+
+static int spi_nand_device_reset(struct spi_nand *snand)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+
+	dev_dbg(snand->dev, "%s\n", __func__);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_RESET, cmd);
+}
+
+static int spi_nand_device_read_reg(struct spi_nand *snand, u8 opcode, u8 *buf)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 1;
+	cmd->addr[0] = opcode;
+	cmd->n_rx = 1;
+	cmd->rx_buf = buf;
+
+	dev_dbg(snand->dev, "%s: reg 0%x\n", __func__, opcode);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_GET_FEATURE, cmd);
+}
+
+static int spi_nand_device_write_reg(struct spi_nand *snand, u8 opcode, u8 *buf)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 1;
+	cmd->addr[0] = opcode;
+	cmd->n_tx = 1;
+	cmd->tx_buf = buf;
+
+	dev_dbg(snand->dev, "%s: reg 0%x\n", __func__, opcode);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_SET_FEATURE, cmd);
+}
+
+static int spi_nand_device_write_enable(struct spi_nand *snand)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+
+	dev_dbg(snand->dev, "%s\n", __func__);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_WRITE_ENABLE, cmd);
+}
+
+static int spi_nand_device_write_disable(struct spi_nand *snand)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+
+	dev_dbg(snand->dev, "%s\n", __func__);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_WRITE_DISABLE, cmd);
+}
+
+static int spi_nand_device_write_page(struct spi_nand *snand, unsigned int page_addr)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 3;
+	cmd->addr[0] = (u8)((page_addr & 0xff0000) >> 16);
+	cmd->addr[1] = (u8)((page_addr & 0xff00) >> 8);
+	cmd->addr[2] = (u8)(page_addr & 0xff);
+
+	dev_dbg(snand->dev, "%s: page 0x%x\n", __func__, page_addr);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_PROGRAM_EXEC, cmd);
+}
+
+static int spi_nand_device_store_cache(struct spi_nand *snand,
+				       unsigned int page_offset, size_t length,
+				       u8 *write_buf)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 2;
+	cmd->addr[0] = (u8)((page_offset & 0xff00) >> 8);
+	cmd->addr[1] = (u8)(page_offset & 0xff);
+	cmd->n_tx = length;
+	cmd->tx_buf = write_buf;
+
+	dev_dbg(snand->dev, "%s: offset 0x%x\n", __func__, page_offset);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_PROGRAM_LOAD, cmd);
+}
+
+static int spi_nand_device_load_page(struct spi_nand *snand, unsigned int page_addr)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 3;
+	cmd->addr[0] = (u8)((page_addr & 0xff0000) >> 16);
+	cmd->addr[1] = (u8)((page_addr & 0xff00) >> 8);
+	cmd->addr[2] = (u8)(page_addr & 0xff);
+
+	dev_dbg(snand->dev, "%s: page 0x%x\n", __func__, page_addr);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_PAGE_READ, cmd);
+}
+
+static int spi_nand_device_read_cache(struct spi_nand *snand,
+				      unsigned int page_offset, size_t length,
+				      u8 *read_buf)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 3;
+	cmd->addr[0] = 0;
+	cmd->addr[1] = (u8)((page_offset & 0xff00) >> 8);
+	cmd->addr[2] = (u8)(page_offset & 0xff);
+	cmd->n_rx = length;
+	cmd->rx_buf = read_buf;
+
+	dev_dbg(snand->dev, "%s: offset 0x%x\n", __func__, page_offset);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_READ_CACHE, cmd);
+
+	return 0;
+}
+
+static int spi_nand_device_block_erase(struct spi_nand *snand, unsigned int page_addr)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_addr = 3;
+	cmd->addr[0] = 0;
+	cmd->addr[1] = (u8)((page_addr & 0xff00) >> 8);
+	cmd->addr[2] = (u8)(page_addr & 0xff);
+
+	dev_dbg(snand->dev, "%s: block 0x%x\n", __func__, page_addr);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_BLOCK_ERASE, cmd);
+}
+
+static int spi_nand_gd5f_read_id(struct spi_nand *snand, u8 *buf)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_rx = SPI_NAND_GD5F_READID_LEN;
+	cmd->rx_buf = buf;
+
+	dev_dbg(snand->dev, "%s\n", __func__);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_READ_ID, cmd);
+}
+
+static int spi_nand_mt29f_read_id(struct spi_nand *snand, u8 *buf)
+{
+	struct spi_nand_device *snand_dev = snand->priv;
+	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
+
+	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
+	cmd->n_rx = SPI_NAND_MT29F_READID_LEN;
+	cmd->rx_buf = buf;
+
+	dev_dbg(snand->dev, "%s\n", __func__);
+
+	return spi_nand_send_command(snand_dev->spi, SPI_NAND_READ_ID, cmd);
+}
+
+static void spi_nand_mt29f_ecc_status(unsigned int status,
+				      unsigned int *corrected,
+				      unsigned int *ecc_error)
+{
+	unsigned int ecc_status = (status >> SPI_NAND_MT29F_ECC_SHIFT) &
+					     SPI_NAND_MT29F_ECC_MASK;
+
+	*ecc_error = (ecc_status == SPI_NAND_MT29F_ECC_UNCORR) ? 1 : 0;
+	if (*ecc_error == 0)
+		*corrected = ecc_status;
+}
+
+static void spi_nand_gd5f_ecc_status(unsigned int status,
+				     unsigned int *corrected,
+				     unsigned int *ecc_error)
+{
+	unsigned int ecc_status = (status >> SPI_NAND_GD5F_ECC_SHIFT) &
+					     SPI_NAND_GD5F_ECC_MASK;
+
+	*ecc_error = (ecc_status == SPI_NAND_GD5F_ECC_UNCORR) ? 1 : 0;
+	if (*ecc_error == 0)
+		*corrected = 2 + ecc_status;
+}
+
+static int spi_nand_device_probe(struct spi_device *spi)
+{
+	enum spi_nand_device_variant variant;
+	struct spi_nand_device *priv;
+	struct spi_nand *snand;
+	int ret;
+
+	priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	snand = &priv->spi_nand;
+
+	snand->read_cache = spi_nand_device_read_cache;
+	snand->load_page = spi_nand_device_load_page;
+	snand->store_cache = spi_nand_device_store_cache;
+	snand->write_page = spi_nand_device_write_page;
+	snand->write_reg = spi_nand_device_write_reg;
+	snand->read_reg = spi_nand_device_read_reg;
+	snand->block_erase = spi_nand_device_block_erase;
+	snand->reset = spi_nand_device_reset;
+	snand->write_enable = spi_nand_device_write_enable;
+	snand->write_disable = spi_nand_device_write_disable;
+	snand->dev = &spi->dev;
+	snand->priv = priv;
+
+	/*
+	 * gd5f reads three ID bytes, and mt29f reads one dummy address byte
+	 * and two ID bytes. Therefore, we could detect both in the same
+	 * read_id implementation by reading _with_ and _without_ a dummy byte,
+	 * until a proper manufacturer is found.
+	 *
+	 * This'll mean we won't need to specify any specific compatible string
+	 * for a given device, and instead just support spi-nand.
+	 */
+	variant = spi_get_device_id(spi)->driver_data;
+	switch (variant) {
+	case SPI_NAND_MT29F:
+		snand->read_id = spi_nand_mt29f_read_id;
+		snand->get_ecc_status = spi_nand_mt29f_ecc_status;
+		break;
+	case SPI_NAND_GD5F:
+		snand->read_id = spi_nand_gd5f_read_id;
+		snand->get_ecc_status = spi_nand_gd5f_ecc_status;
+		break;
+	default:
+		dev_err(snand->dev, "unknown device\n");
+		return -ENODEV;
+	}
+
+	spi_set_drvdata(spi, snand);
+	priv->spi = spi;
+
+	ret = spi_nand_register(snand, spi_nand_flash_ids);
+	if (ret)
+		return ret;
+	return 0;
+}
+
+static int spi_nand_device_remove(struct spi_device *spi)
+{
+	struct spi_nand *snand = spi_get_drvdata(spi);
+
+	spi_nand_unregister(snand);
+
+	return 0;
+}
+
+const struct spi_device_id spi_nand_id_table[] = {
+	{ "spi-nand", SPI_NAND_GENERIC },
+	{ "mt29f", SPI_NAND_MT29F },
+	{ "gd5f", SPI_NAND_GD5F },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, spi_nand_id_table);
+
+static struct spi_driver spi_nand_device_driver = {
+	.driver = {
+		.name	= "spi_nand_device",
+		.owner	= THIS_MODULE,
+	},
+	.id_table = spi_nand_id_table,
+	.probe	= spi_nand_device_probe,
+	.remove	= spi_nand_device_remove,
+};
+module_spi_driver(spi_nand_device_driver);
+
+MODULE_AUTHOR("Ezequiel Garcia <ezequiel.garcia@imgtec.com>");
+MODULE_DESCRIPTION("SPI NAND device support");
+MODULE_LICENSE("GPL v2");
-- 
2.1.0

[PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Qi Wang 王起 (qiwang)]

Hi Ezequiel, Brian

> +
> +static struct nand_ecclayout ecc_layout_gd5f = {
> +	.eccbytes = 128,
> +	.eccpos = {
> +		128, 129, 130, 131, 132, 133, 134, 135,
> +		136, 137, 138, 139, 140, 141, 142, 143,
> +		144, 145, 146, 147, 148, 149, 150, 151,
> +		152, 153, 154, 155, 156, 157, 158, 159,
> +		160, 161, 162, 163, 164, 165, 166, 167,
> +		168, 169, 170, 171, 172, 173, 174, 175,
> +		176, 177, 178, 179, 180, 181, 182, 183,
> +		184, 185, 186, 187, 188, 189, 190, 191,
> +		192, 193, 194, 195, 196, 197, 198, 199,
> +		200, 201, 202, 203, 204, 205, 206, 207,
> +		208, 209, 210, 211, 212, 213, 214, 215,
> +		216, 217, 218, 219, 220, 221, 222, 223,
> +		224, 225, 226, 227, 228, 229, 230, 231,
> +		232, 233, 234, 235, 236, 237, 238, 239,
> +		240, 241, 242, 243, 244, 245, 246, 247,
> +		248, 249, 250, 251, 252, 253, 254, 255
> +	},
> +	.oobfree = { {1, 127} }
> +};
> +
> +static struct nand_ecclayout ecc_layout_mt29f = {
> +	.eccbytes = 32,
> +	.eccpos = {
> +		8, 9, 10, 11, 12, 13, 14, 15,
> +		24, 25, 26, 27, 28, 29, 30, 31,
> +		40, 41, 42, 43, 44, 45, 46, 47,
> +		56, 57, 58, 59, 60, 61, 62, 63,
> +	 },
> +};
> +

As they are using On die ecc, oob layout definition is defined by SPI NAND vendors. 
Part of oob is protected by ecc, and the rest is unprotect.
I am thinking if we can define a variable to indicate the range, what is protect, 
and what is unprotect. 
This would be very useful for filesystem, which want to write data into OOB.

For example, Jffs2 program clean marker data to oob first when format operation, 
then will program main data into NAND flash main area when normal write operation. 
That mean JFFS2 can only store clean marker into unprotect area of oob. 
But for YAFFS2, it program main data and spare area data together into NAND flash, then
YAFFS2 can store data into protect area of OOB.
How do you think?

Thanks
 

Re: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Ezequiel Garcia]

On 12/03/2014 06:52 AM, Qi Wang 王起 (qiwang) wrote:
>> +
>> +static struct nand_ecclayout ecc_layout_gd5f = {
>> +	.eccbytes = 128,
>> +	.eccpos = {
>> +		128, 129, 130, 131, 132, 133, 134, 135,
>> +		136, 137, 138, 139, 140, 141, 142, 143,
>> +		144, 145, 146, 147, 148, 149, 150, 151,
>> +		152, 153, 154, 155, 156, 157, 158, 159,
>> +		160, 161, 162, 163, 164, 165, 166, 167,
>> +		168, 169, 170, 171, 172, 173, 174, 175,
>> +		176, 177, 178, 179, 180, 181, 182, 183,
>> +		184, 185, 186, 187, 188, 189, 190, 191,
>> +		192, 193, 194, 195, 196, 197, 198, 199,
>> +		200, 201, 202, 203, 204, 205, 206, 207,
>> +		208, 209, 210, 211, 212, 213, 214, 215,
>> +		216, 217, 218, 219, 220, 221, 222, 223,
>> +		224, 225, 226, 227, 228, 229, 230, 231,
>> +		232, 233, 234, 235, 236, 237, 238, 239,
>> +		240, 241, 242, 243, 244, 245, 246, 247,
>> +		248, 249, 250, 251, 252, 253, 254, 255
>> +	},
>> +	.oobfree = { {1, 127} }
>> +};
>> +
>> +static struct nand_ecclayout ecc_layout_mt29f = {
>> +	.eccbytes = 32,
>> +	.eccpos = {
>> +		8, 9, 10, 11, 12, 13, 14, 15,
>> +		24, 25, 26, 27, 28, 29, 30, 31,
>> +		40, 41, 42, 43, 44, 45, 46, 47,
>> +		56, 57, 58, 59, 60, 61, 62, 63,
>> +	 },
>> +};
>> +
> 
> As they are using On die ecc, oob layout definition is defined by SPI NAND vendors. 
> Part of oob is protected by ecc, and the rest is unprotect.
> I am thinking if we can define a variable to indicate the range, what is protect, 
> and what is unprotect. 

Yes, we should add the free out-of-band regions.

> This would be very useful for filesystem, which want to write data into OOB.
> 
> For example, Jffs2 program clean marker data to oob first when format operation, 
> then will program main data into NAND flash main area when normal write operation. 
> That mean JFFS2 can only store clean marker into unprotect area of oob. 

Yeah, but are these devices capable of writing to the OOB region only
(without messing with the data) ?

If I understand this correctly, supporting JFFS2 would mean implementing
a sort of read-modify-write operation to be able to write the OOB
without touching the main area, and viceversa.

I'd say JFFS2 is not the target filesystem for these SPI NAND devices,
and UBI-based filesystems should be used instead (UBI don't use OOB).

> But for YAFFS2, it program main data and spare area data together into NAND flash, then
> YAFFS2 can store data into protect area of OOB.

Well, frankly, I'm not too worried about YAFFS2. It was never mainlined,
and probably it will never be.

Do you have any reason for *not* using a UBI-based FS?
-- 
Ezequiel

RE: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Qi Wang 王起 (qiwang)]

On 12/03/2014 07:14 AM, Ezequiel Garcia wrote:
> 
> On 12/03/2014 06:52 AM, Qi Wang 王起 (qiwang) wrote:
> >> +
> >> +static struct nand_ecclayout ecc_layout_gd5f = {
> >> +	.eccbytes = 128,
> >> +	.eccpos = {
> >> +		128, 129, 130, 131, 132, 133, 134, 135,
> >> +		136, 137, 138, 139, 140, 141, 142, 143,
> >> +		144, 145, 146, 147, 148, 149, 150, 151,
> >> +		152, 153, 154, 155, 156, 157, 158, 159,
> >> +		160, 161, 162, 163, 164, 165, 166, 167,
> >> +		168, 169, 170, 171, 172, 173, 174, 175,
> >> +		176, 177, 178, 179, 180, 181, 182, 183,
> >> +		184, 185, 186, 187, 188, 189, 190, 191,
> >> +		192, 193, 194, 195, 196, 197, 198, 199,
> >> +		200, 201, 202, 203, 204, 205, 206, 207,
> >> +		208, 209, 210, 211, 212, 213, 214, 215,
> >> +		216, 217, 218, 219, 220, 221, 222, 223,
> >> +		224, 225, 226, 227, 228, 229, 230, 231,
> >> +		232, 233, 234, 235, 236, 237, 238, 239,
> >> +		240, 241, 242, 243, 244, 245, 246, 247,
> >> +		248, 249, 250, 251, 252, 253, 254, 255
> >> +	},
> >> +	.oobfree = { {1, 127} }
> >> +};
> >> +
> >> +static struct nand_ecclayout ecc_layout_mt29f = {
> >> +	.eccbytes = 32,
> >> +	.eccpos = {
> >> +		8, 9, 10, 11, 12, 13, 14, 15,
> >> +		24, 25, 26, 27, 28, 29, 30, 31,
> >> +		40, 41, 42, 43, 44, 45, 46, 47,
> >> +		56, 57, 58, 59, 60, 61, 62, 63,
> >> +	 },
> >> +};
> >> +
> >
> > As they are using On die ecc, oob layout definition is defined by SPI
> NAND vendors.
> > Part of oob is protected by ecc, and the rest is unprotect.
> > I am thinking if we can define a variable to indicate the range, what
> > is protect, and what is unprotect.
> 
> Yes, we should add the free out-of-band regions.

Sorry, probably I didn't explain clearly. 
I mean if we can define oob free area like this:
.oobfree_unprotect = { {1, 63} },
.oobfree_protect = { {64, 127} },
To differentiate the which OOB area is protected, which isn't protected.

> > This would be very useful for filesystem, which want to write data into
> OOB.
> >
> > For example, Jffs2 program clean marker data to oob first when format
> > operation, then will program main data into NAND flash main area when
> normal write operation.
> > That mean JFFS2 can only store clean marker into unprotect area of oob.
> 
> Yeah, but are these devices capable of writing to the OOB region only
> (without messing with the data) ?

Yes, SPI NAND have this capability. SPI NAND also have definition for NOP,
just similar with Parallel NAND, fill 0xFF to the area that don't want to 
write anything is ok.

> If I understand this correctly, supporting JFFS2 would mean implementing a
> sort of read-modify-write operation to be able to write the OOB without
> touching the main area, and viceversa.

Yes, just like this operation, but JFFS2 write OOB area first, then main area.
So JFFS2 should write data into unprotect area in OOB, then write main data,
Right? Because if write data into OOB protect area, on die ecc will generate
a ecc code and write into eccpos area. When JFFS2 write main data, a new ecc 
code will generate, and will cause conflict with old one.  

> I'd say JFFS2 is not the target filesystem for these SPI NAND devices, and
> UBI-based filesystems should be used instead (UBI don't use OOB).
> 
> > But for YAFFS2, it program main data and spare area data together into
> > NAND flash, then
> > YAFFS2 can store data into protect area of OOB.
> 
> Well, frankly, I'm not too worried about YAFFS2. It was never mainlined,
> and probably it will never be.
> 
> Do you have any reason for *not* using a UBI-based FS?

I agree with you, I just considered some legacy users customer still using JFFS2.

Thanks   

Re: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Ezequiel Garcia]

On 12/03/2014 09:08 AM, Qi Wang 王起 (qiwang) wrote:
[..]
>>> For example, Jffs2 program clean marker data to oob first when format
>>> operation, then will program main data into NAND flash main area when
>> normal write operation.
>>> That mean JFFS2 can only store clean marker into unprotect area of oob.
>>
>> Yeah, but are these devices capable of writing to the OOB region only
>> (without messing with the data) ?
> 
> Yes, SPI NAND have this capability. SPI NAND also have definition for NOP,
> just similar with Parallel NAND, fill 0xFF to the area that don't want to 
> write anything is ok.
> 

Which SPI NAND command allows to do this?

AFAICS, you need to issue the following sequence to be able to write to
only the main area, or only the OOB area:

1. Read page to cache
2. Write to cache
3. Write page from cache

So it doesn't seem to support it natively, but through a workaround.
Doesn't this hurt JFFS2 performance a lot?

[..]
>>
>> Do you have any reason for *not* using a UBI-based FS?
> 
> I agree with you, I just considered some legacy users customer still using JFFS2.
> 

Hm.. right. It's important to support legacy users, but I'd say it's not
our primary goal.

-- 
Ezequiel

Re: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Daniel Ehrenberg]

On Tue, Dec 2, 2014 at 4:58 AM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> This commit uses the recently introduced SPI NAND framework to support
> Micron MT29F and Gigadevice GD5F serial NAND devices. These two families
> of devices are fairly similar and so they are supported with only minimal
> differences.
>
> The current support includes:
>
>   * Page read and page program operations (using on-die ECC)
>   * Page out-of-band read
>   * Erase
>   * Reset
>   * Device status retrieval
>   * Device ID retrieval
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> ---
>  drivers/mtd/spi-nand/Kconfig           |  12 +
>  drivers/mtd/spi-nand/Makefile          |   1 +
>  drivers/mtd/spi-nand/spi-nand-device.c | 500 +++++++++++++++++++++++++++++++++
>  3 files changed, 513 insertions(+)
>  create mode 100644 drivers/mtd/spi-nand/spi-nand-device.c
>
> diff --git a/drivers/mtd/spi-nand/Kconfig b/drivers/mtd/spi-nand/Kconfig
> index 3868477..df29abe 100644
> --- a/drivers/mtd/spi-nand/Kconfig
> +++ b/drivers/mtd/spi-nand/Kconfig
> @@ -4,3 +4,15 @@ menuconfig MTD_SPI_NAND
>         select MTD_NAND
>         help
>           This is the framework for the SPI NAND.
> +
> +if MTD_SPI_NAND
> +
> +config MTD_SPI_NAND_DEVICES
> +       tristate "Support for SPI NAND devices (MT29F, GD5F)"
> +       default y
> +       depends on MTD_SPI_NAND
> +       help
> +         Select this option if you require support for the most common SPI NAND
> +         devices such as mt29f and gd5f.
> +
> +endif # MTD_SPI_NAND
> diff --git a/drivers/mtd/spi-nand/Makefile b/drivers/mtd/spi-nand/Makefile
> index d454c52..f4f95b7 100644
> --- a/drivers/mtd/spi-nand/Makefile
> +++ b/drivers/mtd/spi-nand/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_MTD_SPI_NAND)             += spi-nand-base.o
> +obj-$(CONFIG_MTD_SPI_NAND_DEVICES)     += spi-nand-device.o
> diff --git a/drivers/mtd/spi-nand/spi-nand-device.c b/drivers/mtd/spi-nand/spi-nand-device.c
> new file mode 100644
> index 0000000..73050f3
> --- /dev/null
> +++ b/drivers/mtd/spi-nand/spi-nand-device.c
> @@ -0,0 +1,500 @@
> +/*
> + * Copyright (C) 2014 Imagination Technologies Ltd.
> + *
> + * 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; version 2 of the License.
> + *
> + * Notes:
> + * 1. We avoid using a stack-allocated buffer for SPI messages. Using
> + *    a kmalloced buffer is probably better, given we shouldn't assume
> + *    any particular usage by SPI core.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nand.h>
> +#include <linux/sizes.h>
> +#include <linux/spi/spi.h>
> +
> +/* SPI NAND commands */
> +#define        SPI_NAND_WRITE_ENABLE           0x06
> +#define        SPI_NAND_WRITE_DISABLE          0x04
> +#define        SPI_NAND_GET_FEATURE            0x0f
> +#define        SPI_NAND_SET_FEATURE            0x1f
> +#define        SPI_NAND_PAGE_READ              0x13
> +#define        SPI_NAND_READ_CACHE             0x03
> +#define        SPI_NAND_FAST_READ_CACHE        0x0b
> +#define        SPI_NAND_READ_CACHE_X2          0x3b
> +#define        SPI_NAND_READ_CACHE_X4          0x6b
> +#define        SPI_NAND_READ_CACHE_DUAL_IO     0xbb
> +#define        SPI_NAND_READ_CACHE_QUAD_IO     0xeb
> +#define        SPI_NAND_READ_ID                0x9f
> +#define        SPI_NAND_PROGRAM_LOAD           0x02
> +#define        SPI_NAND_PROGRAM_LOAD4          0x32
> +#define        SPI_NAND_PROGRAM_EXEC           0x10
> +#define        SPI_NAND_PROGRAM_LOAD_RANDOM    0x84
> +#define        SPI_NAND_PROGRAM_LOAD_RANDOM4   0xc4
> +#define        SPI_NAND_BLOCK_ERASE            0xd8
> +#define        SPI_NAND_RESET                  0xff
> +
> +#define SPI_NAND_GD5F_READID_LEN       2
> +#define SPI_NAND_MT29F_READID_LEN      2
> +
> +#define SPI_NAND_GD5F_ECC_MASK         (BIT(0) | BIT(1) | BIT(2))
> +#define SPI_NAND_GD5F_ECC_UNCORR       (BIT(0) | BIT(1) | BIT(2))
> +#define SPI_NAND_GD5F_ECC_SHIFT                4
> +
> +#define SPI_NAND_MT29F_ECC_MASK                (BIT(0) | BIT(1))
> +#define SPI_NAND_MT29F_ECC_UNCORR      (BIT(1))
> +#define SPI_NAND_MT29F_ECC_SHIFT               4
> +
> +static struct nand_ecclayout ecc_layout_gd5f = {
> +       .eccbytes = 128,
> +       .eccpos = {
> +               128, 129, 130, 131, 132, 133, 134, 135,
> +               136, 137, 138, 139, 140, 141, 142, 143,
> +               144, 145, 146, 147, 148, 149, 150, 151,
> +               152, 153, 154, 155, 156, 157, 158, 159,
> +               160, 161, 162, 163, 164, 165, 166, 167,
> +               168, 169, 170, 171, 172, 173, 174, 175,
> +               176, 177, 178, 179, 180, 181, 182, 183,
> +               184, 185, 186, 187, 188, 189, 190, 191,
> +               192, 193, 194, 195, 196, 197, 198, 199,
> +               200, 201, 202, 203, 204, 205, 206, 207,
> +               208, 209, 210, 211, 212, 213, 214, 215,
> +               216, 217, 218, 219, 220, 221, 222, 223,
> +               224, 225, 226, 227, 228, 229, 230, 231,
> +               232, 233, 234, 235, 236, 237, 238, 239,
> +               240, 241, 242, 243, 244, 245, 246, 247,
> +               248, 249, 250, 251, 252, 253, 254, 255
> +       },
> +       .oobfree = { {1, 127} }
> +};
> +
> +static struct nand_ecclayout ecc_layout_mt29f = {
> +       .eccbytes = 32,
> +       .eccpos = {
> +               8, 9, 10, 11, 12, 13, 14, 15,
> +               24, 25, 26, 27, 28, 29, 30, 31,
> +               40, 41, 42, 43, 44, 45, 46, 47,
> +               56, 57, 58, 59, 60, 61, 62, 63,
> +        },
> +};
> +
> +static struct nand_flash_dev spi_nand_flash_ids[] = {
> +       {
> +               .name = "SPI NAND 512MiB 3,3V",
> +               .id = { NAND_MFR_GIGADEVICE, 0xb4 },
> +               .chipsize = 512,
> +               .pagesize = SZ_4K,
> +               .erasesize = SZ_256K,
> +               .id_len = 2,
> +               .oobsize = 256,
> +               .ecc.strength_ds = 8,
> +               .ecc.step_ds = 512,
> +               .ecc.layout = &ecc_layout_gd5f,
> +       },
> +       {
> +               .name = "SPI NAND 512MiB 1,8V",
> +               .id = { NAND_MFR_GIGADEVICE, 0xa4 },
> +               .chipsize = 512,
> +               .pagesize = SZ_4K,
> +               .erasesize = SZ_256K,
> +               .id_len = 2,
> +               .oobsize = 256,
> +               .ecc.strength_ds = 8,
> +               .ecc.step_ds = 512,
> +               .ecc.layout = &ecc_layout_gd5f,
> +       },
> +       {
> +               .name = "SPI NAND 512MiB 3,3V",
> +               .id = { NAND_MFR_MICRON, 0x32 },
> +               .chipsize = 512,
> +               .pagesize = SZ_2K,
> +               .erasesize = SZ_128K,
> +               .id_len = 2,
> +               .oobsize = 64,
> +               .ecc.strength_ds = 4,
> +               .ecc.step_ds = 512,
> +               .ecc.layout = &ecc_layout_mt29f,
> +       },
> +       {
> +               .name = "SPI NAND 256MiB 3,3V",
> +               .id = { NAND_MFR_MICRON, 0x22 },
> +               .chipsize = 256,
> +               .pagesize = SZ_2K,
> +               .erasesize = SZ_128K,
> +               .id_len = 2,
> +               .oobsize = 64,
> +               .ecc.strength_ds = 4,
> +               .ecc.step_ds = 512,
> +               .ecc.layout = &ecc_layout_mt29f,
> +       },
> +};
> +
> +enum spi_nand_device_variant {
> +       SPI_NAND_GENERIC,
> +       SPI_NAND_MT29F,
> +       SPI_NAND_GD5F,
> +};

What is the purpose of including SPI_NAND_GENERIC? It looks like it
doesn't work anyway.

> +
> +struct spi_nand_device_cmd {
> +       u8 cmd;
> +       u32 n_addr;
> +       u8 addr[3];
> +       u32 n_tx;
> +       u8 *tx_buf;
> +       u32 n_rx;
> +       u8 *rx_buf;
> +};
> +
> +struct spi_nand_device {
> +       struct spi_nand spi_nand;
> +       struct spi_device *spi;
> +
> +       struct spi_nand_device_cmd cmd;
> +};
> +
> +static int spi_nand_send_command(struct spi_device *spi, int command,
> +                                struct spi_nand_device_cmd *cmd)
> +{
> +       struct spi_message message;
> +       struct spi_transfer x[4];
> +
> +       spi_message_init(&message);
> +       memset(x, 0, sizeof(x));
> +
> +       /* Command */
> +       cmd->cmd = command;
> +       x[0].len = 1;
> +       x[0].tx_buf = &cmd->cmd;
> +       spi_message_add_tail(&x[0], &message);
> +
> +       /* Address */
> +       if (cmd->n_addr) {
> +               x[1].len = cmd->n_addr;
> +               x[1].tx_buf = cmd->addr;
> +               spi_message_add_tail(&x[1], &message);
> +       }
> +
> +       /* Data to be transmitted */
> +       if (cmd->n_tx) {
> +               x[3].len = cmd->n_tx;
> +               x[3].tx_buf = cmd->tx_buf;
> +               spi_message_add_tail(&x[3], &message);
> +       }
> +
> +       /* Data to be received */
> +       if (cmd->n_rx) {
> +               x[3].len = cmd->n_rx;
> +               x[3].rx_buf = cmd->rx_buf;
> +               spi_message_add_tail(&x[3], &message);
> +       }
> +
> +       return spi_sync(spi, &message);
> +}
> +
> +static int spi_nand_device_reset(struct spi_nand *snand)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +
> +       dev_dbg(snand->dev, "%s\n", __func__);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_RESET, cmd);
> +}
> +
> +static int spi_nand_device_read_reg(struct spi_nand *snand, u8 opcode, u8 *buf)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 1;
> +       cmd->addr[0] = opcode;
> +       cmd->n_rx = 1;
> +       cmd->rx_buf = buf;
> +
> +       dev_dbg(snand->dev, "%s: reg 0%x\n", __func__, opcode);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_GET_FEATURE, cmd);
> +}
> +
> +static int spi_nand_device_write_reg(struct spi_nand *snand, u8 opcode, u8 *buf)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 1;
> +       cmd->addr[0] = opcode;
> +       cmd->n_tx = 1;
> +       cmd->tx_buf = buf;
> +
> +       dev_dbg(snand->dev, "%s: reg 0%x\n", __func__, opcode);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_SET_FEATURE, cmd);
> +}
> +
> +static int spi_nand_device_write_enable(struct spi_nand *snand)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +
> +       dev_dbg(snand->dev, "%s\n", __func__);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_WRITE_ENABLE, cmd);
> +}
> +
> +static int spi_nand_device_write_disable(struct spi_nand *snand)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +
> +       dev_dbg(snand->dev, "%s\n", __func__);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_WRITE_DISABLE, cmd);
> +}
> +
> +static int spi_nand_device_write_page(struct spi_nand *snand, unsigned int page_addr)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 3;
> +       cmd->addr[0] = (u8)((page_addr & 0xff0000) >> 16);
> +       cmd->addr[1] = (u8)((page_addr & 0xff00) >> 8);
> +       cmd->addr[2] = (u8)(page_addr & 0xff);
> +
> +       dev_dbg(snand->dev, "%s: page 0x%x\n", __func__, page_addr);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_PROGRAM_EXEC, cmd);
> +}
> +
> +static int spi_nand_device_store_cache(struct spi_nand *snand,
> +                                      unsigned int page_offset, size_t length,
> +                                      u8 *write_buf)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 2;
> +       cmd->addr[0] = (u8)((page_offset & 0xff00) >> 8);
> +       cmd->addr[1] = (u8)(page_offset & 0xff);
> +       cmd->n_tx = length;
> +       cmd->tx_buf = write_buf;
> +
> +       dev_dbg(snand->dev, "%s: offset 0x%x\n", __func__, page_offset);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_PROGRAM_LOAD, cmd);
> +}
> +
> +static int spi_nand_device_load_page(struct spi_nand *snand, unsigned int page_addr)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 3;
> +       cmd->addr[0] = (u8)((page_addr & 0xff0000) >> 16);
> +       cmd->addr[1] = (u8)((page_addr & 0xff00) >> 8);
> +       cmd->addr[2] = (u8)(page_addr & 0xff);
> +
> +       dev_dbg(snand->dev, "%s: page 0x%x\n", __func__, page_addr);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_PAGE_READ, cmd);
> +}
> +
> +static int spi_nand_device_read_cache(struct spi_nand *snand,
> +                                     unsigned int page_offset, size_t length,
> +                                     u8 *read_buf)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 3;
> +       cmd->addr[0] = 0;
> +       cmd->addr[1] = (u8)((page_offset & 0xff00) >> 8);
> +       cmd->addr[2] = (u8)(page_offset & 0xff);
> +       cmd->n_rx = length;
> +       cmd->rx_buf = read_buf;
> +
> +       dev_dbg(snand->dev, "%s: offset 0x%x\n", __func__, page_offset);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_READ_CACHE, cmd);
> +
> +       return 0;
> +}
> +
> +static int spi_nand_device_block_erase(struct spi_nand *snand, unsigned int page_addr)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_addr = 3;
> +       cmd->addr[0] = 0;
> +       cmd->addr[1] = (u8)((page_addr & 0xff00) >> 8);
> +       cmd->addr[2] = (u8)(page_addr & 0xff);
> +
> +       dev_dbg(snand->dev, "%s: block 0x%x\n", __func__, page_addr);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_BLOCK_ERASE, cmd);
> +}
> +
> +static int spi_nand_gd5f_read_id(struct spi_nand *snand, u8 *buf)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_rx = SPI_NAND_GD5F_READID_LEN;
> +       cmd->rx_buf = buf;
> +
> +       dev_dbg(snand->dev, "%s\n", __func__);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_READ_ID, cmd);
> +}
> +
> +static int spi_nand_mt29f_read_id(struct spi_nand *snand, u8 *buf)
> +{
> +       struct spi_nand_device *snand_dev = snand->priv;
> +       struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +       memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +       cmd->n_rx = SPI_NAND_MT29F_READID_LEN;
> +       cmd->rx_buf = buf;
> +
> +       dev_dbg(snand->dev, "%s\n", __func__);
> +
> +       return spi_nand_send_command(snand_dev->spi, SPI_NAND_READ_ID, cmd);
> +}
> +
> +static void spi_nand_mt29f_ecc_status(unsigned int status,
> +                                     unsigned int *corrected,
> +                                     unsigned int *ecc_error)
> +{
> +       unsigned int ecc_status = (status >> SPI_NAND_MT29F_ECC_SHIFT) &
> +                                            SPI_NAND_MT29F_ECC_MASK;
> +
> +       *ecc_error = (ecc_status == SPI_NAND_MT29F_ECC_UNCORR) ? 1 : 0;
> +       if (*ecc_error == 0)
> +               *corrected = ecc_status;
> +}
> +
> +static void spi_nand_gd5f_ecc_status(unsigned int status,
> +                                    unsigned int *corrected,
> +                                    unsigned int *ecc_error)
> +{
> +       unsigned int ecc_status = (status >> SPI_NAND_GD5F_ECC_SHIFT) &
> +                                            SPI_NAND_GD5F_ECC_MASK;
> +
> +       *ecc_error = (ecc_status == SPI_NAND_GD5F_ECC_UNCORR) ? 1 : 0;
> +       if (*ecc_error == 0)
> +               *corrected = 2 + ecc_status;
> +}
> +
> +static int spi_nand_device_probe(struct spi_device *spi)
> +{
> +       enum spi_nand_device_variant variant;
> +       struct spi_nand_device *priv;
> +       struct spi_nand *snand;
> +       int ret;
> +
> +       priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       snand = &priv->spi_nand;
> +
> +       snand->read_cache = spi_nand_device_read_cache;
> +       snand->load_page = spi_nand_device_load_page;
> +       snand->store_cache = spi_nand_device_store_cache;
> +       snand->write_page = spi_nand_device_write_page;
> +       snand->write_reg = spi_nand_device_write_reg;
> +       snand->read_reg = spi_nand_device_read_reg;
> +       snand->block_erase = spi_nand_device_block_erase;
> +       snand->reset = spi_nand_device_reset;
> +       snand->write_enable = spi_nand_device_write_enable;
> +       snand->write_disable = spi_nand_device_write_disable;
> +       snand->dev = &spi->dev;
> +       snand->priv = priv;
> +
> +       /*
> +        * gd5f reads three ID bytes, and mt29f reads one dummy address byte
> +        * and two ID bytes. Therefore, we could detect both in the same
> +        * read_id implementation by reading _with_ and _without_ a dummy byte,
> +        * until a proper manufacturer is found.
> +        *
> +        * This'll mean we won't need to specify any specific compatible string
> +        * for a given device, and instead just support spi-nand.
> +        */
> +       variant = spi_get_device_id(spi)->driver_data;
> +       switch (variant) {
> +       case SPI_NAND_MT29F:
> +               snand->read_id = spi_nand_mt29f_read_id;
> +               snand->get_ecc_status = spi_nand_mt29f_ecc_status;
> +               break;
> +       case SPI_NAND_GD5F:
> +               snand->read_id = spi_nand_gd5f_read_id;
> +               snand->get_ecc_status = spi_nand_gd5f_ecc_status;
> +               break;
> +       default:
> +               dev_err(snand->dev, "unknown device\n");
> +               return -ENODEV;
> +       }
> +
> +       spi_set_drvdata(spi, snand);
> +       priv->spi = spi;
> +
> +       ret = spi_nand_register(snand, spi_nand_flash_ids);
> +       if (ret)
> +               return ret;
> +       return 0;
> +}
> +
> +static int spi_nand_device_remove(struct spi_device *spi)
> +{
> +       struct spi_nand *snand = spi_get_drvdata(spi);
> +
> +       spi_nand_unregister(snand);
> +
> +       return 0;
> +}
> +
> +const struct spi_device_id spi_nand_id_table[] = {
> +       { "spi-nand", SPI_NAND_GENERIC },
> +       { "mt29f", SPI_NAND_MT29F },
> +       { "gd5f", SPI_NAND_GD5F },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(spi, spi_nand_id_table);
> +
> +static struct spi_driver spi_nand_device_driver = {
> +       .driver = {
> +               .name   = "spi_nand_device",
> +               .owner  = THIS_MODULE,
> +       },
> +       .id_table = spi_nand_id_table,
> +       .probe  = spi_nand_device_probe,
> +       .remove = spi_nand_device_remove,
> +};
> +module_spi_driver(spi_nand_device_driver);
> +
> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel.garcia@imgtec.com>");
> +MODULE_DESCRIPTION("SPI NAND device support");
> +MODULE_LICENSE("GPL v2");
> --
> 2.1.0
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Ezequiel Garcia]

On 12/12/2014 10:27 PM, Daniel Ehrenberg wrote:
> On Tue, Dec 2, 2014 at 4:58 AM, Ezequiel Garcia
> <ezequiel.garcia@imgtec.com> wrote:
[..]
>> +
>> +enum spi_nand_device_variant {
>> +       SPI_NAND_GENERIC,
>> +       SPI_NAND_MT29F,
>> +       SPI_NAND_GD5F,
>> +};
> 
> What is the purpose of including SPI_NAND_GENERIC? It looks like it
> doesn't work anyway.
> 

For the time being, I'm not sure we will require a device-specific compatible
string, or rather, auto-detect the device using the device ID.

So, this patch leaves the door opened for both options.

I've been talking to Brian on IRC, and auto-detecting the device seems like
the way to go. However, I wanted to have some feedback on the general approach
having the community approval, before moving forward in any direction.

Thanks for the feedback,
-- 
Ezequiel

Re: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Daniel Ehrenberg]

Sounds good to me.

Reviewed-by: Dan Ehrenberg <dehrenberg@chromium.org>

On Mon, Dec 15, 2014 at 11:36 AM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
>
>
> On 12/12/2014 10:27 PM, Daniel Ehrenberg wrote:
>> On Tue, Dec 2, 2014 at 4:58 AM, Ezequiel Garcia
>> <ezequiel.garcia@imgtec.com> wrote:
> [..]
>>> +
>>> +enum spi_nand_device_variant {
>>> +       SPI_NAND_GENERIC,
>>> +       SPI_NAND_MT29F,
>>> +       SPI_NAND_GD5F,
>>> +};
>>
>> What is the purpose of including SPI_NAND_GENERIC? It looks like it
>> doesn't work anyway.
>>
>
> For the time being, I'm not sure we will require a device-specific compatible
> string, or rather, auto-detect the device using the device ID.
>
> So, this patch leaves the door opened for both options.
>
> I've been talking to Brian on IRC, and auto-detecting the device seems like
> the way to go. However, I wanted to have some feedback on the general approach
> having the community approval, before moving forward in any direction.
>
> Thanks for the feedback,
> --
> Ezequiel

[PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Qi Wang 王起 (qiwang)]

Hi Ezequiel,

> +static struct nand_ecclayout ecc_layout_mt29f = {
> +	.eccbytes = 32,
> +	.eccpos = {
> +		8, 9, 10, 11, 12, 13, 14, 15,
> +		24, 25, 26, 27, 28, 29, 30, 31,
> +		40, 41, 42, 43, 44, 45, 46, 47,
> +		56, 57, 58, 59, 60, 61, 62, 63,
> +	 },

Seems "OOB free" variable is missed. As below:

.oobfree = {
{ .offset = 2, .length = 6 },
{ .offset = 16, .length = 8 },
{ .offset = 32, .length = 8 },
{ .offset = 48, .length = 8 },
},


> +static int spi_nand_send_command(struct spi_device *spi, int command,
> +				 struct spi_nand_device_cmd *cmd) {
> +	struct spi_message message;
> +	struct spi_transfer x[4];
> +
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof(x));
> +
> +	/* Command */
> +	cmd->cmd = command;
> +	x[0].len = 1;
> +	x[0].tx_buf = &cmd->cmd;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	/* Address */
> +	if (cmd->n_addr) {
> +		x[1].len = cmd->n_addr;
> +		x[1].tx_buf = cmd->addr;
> +		spi_message_add_tail(&x[1], &message);
> +	}
> +
> +	/* Data to be transmitted */
> +	if (cmd->n_tx) {
> +		x[3].len = cmd->n_tx;
> +		x[3].tx_buf = cmd->tx_buf;
> +		spi_message_add_tail(&x[3], &message);
> +	}
> +
> +	/* Data to be received */
> +	if (cmd->n_rx) {
> +		x[3].len = cmd->n_rx;
> +		x[3].rx_buf = cmd->rx_buf;
> +		spi_message_add_tail(&x[3], &message);
> +	}

Only x[3] is used in above code, I suggest to separate transfer and received 
For x[2] and x[3], in case some command need to send dummy byte before received any data.

> +static int spi_nand_device_block_erase(struct spi_nand *snand,
> unsigned
> +int page_addr) {
> +	struct spi_nand_device *snand_dev = snand->priv;
> +	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +	cmd->n_addr = 3;
> +	cmd->addr[0] = 0;

Page address may beyond 16 bit in large density product. 
So I don't think addr[0] can be forced to be set 0


> +static int spi_nand_mt29f_read_id(struct spi_nand *snand, u8 *buf) {
> +	struct spi_nand_device *snand_dev = snand->priv;
> +	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> +
> +	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> +	cmd->n_rx = SPI_NAND_MT29F_READID_LEN;
> +	cmd->rx_buf = buf;
> +

Micron SPI NAND need 1 more dummy byte before received ID data.
Code for mt29f_read_id should be like this:

static int spi_nand_mt29f_read_id(struct spi_nand *snand, u8 *buf) {
	struct spi_nand_device *snand_dev = snand->priv;
	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
	u8 dummy = 0;
	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
"
	cmd->n_tx = 1;
	cmd->tx_buf = &dummy;
"
	cmd->n_rx = SPI_NAND_MT29F_READID_LEN;
	cmd->rx_buf = buf;

Thanks
Qi Wang

Re: [PATCH 6/6] mtd: spi-nand: Support common SPI NAND devices

[Ezequiel Garcia]

On 12/22/2014 01:34 AM, Qi Wang 王起 (qiwang) wrote:
> Hi Ezequiel,
> 
>> +static struct nand_ecclayout ecc_layout_mt29f = {
>> +	.eccbytes = 32,
>> +	.eccpos = {
>> +		8, 9, 10, 11, 12, 13, 14, 15,
>> +		24, 25, 26, 27, 28, 29, 30, 31,
>> +		40, 41, 42, 43, 44, 45, 46, 47,
>> +		56, 57, 58, 59, 60, 61, 62, 63,
>> +	 },
> 
> Seems "OOB free" variable is missed. As below:
> 
> .oobfree = {
> { .offset = 2, .length = 6 },
> { .offset = 16, .length = 8 },
> { .offset = 32, .length = 8 },
> { .offset = 48, .length = 8 },
> },
> 

OK, great.

Have you tested this and made sure it works for MT29F and allows to
access the OOB?

> 
>> +static int spi_nand_send_command(struct spi_device *spi, int command,
>> +				 struct spi_nand_device_cmd *cmd) {
>> +	struct spi_message message;
>> +	struct spi_transfer x[4];
>> +
>> +	spi_message_init(&message);
>> +	memset(x, 0, sizeof(x));
>> +
>> +	/* Command */
>> +	cmd->cmd = command;
>> +	x[0].len = 1;
>> +	x[0].tx_buf = &cmd->cmd;
>> +	spi_message_add_tail(&x[0], &message);
>> +
>> +	/* Address */
>> +	if (cmd->n_addr) {
>> +		x[1].len = cmd->n_addr;
>> +		x[1].tx_buf = cmd->addr;
>> +		spi_message_add_tail(&x[1], &message);
>> +	}
>> +
>> +	/* Data to be transmitted */
>> +	if (cmd->n_tx) {
>> +		x[3].len = cmd->n_tx;
>> +		x[3].tx_buf = cmd->tx_buf;
>> +		spi_message_add_tail(&x[3], &message);
>> +	}
>> +
>> +	/* Data to be received */
>> +	if (cmd->n_rx) {
>> +		x[3].len = cmd->n_rx;
>> +		x[3].rx_buf = cmd->rx_buf;
>> +		spi_message_add_tail(&x[3], &message);
>> +	}
> 
> Only x[3] is used in above code, I suggest to separate transfer and received 
> For x[2] and x[3], in case some command need to send dummy byte before received any data.
> 

Ah, yes. Good catch.

>> +static int spi_nand_device_block_erase(struct spi_nand *snand,
>> unsigned
>> +int page_addr) {
>> +	struct spi_nand_device *snand_dev = snand->priv;
>> +	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
>> +
>> +	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
>> +	cmd->n_addr = 3;
>> +	cmd->addr[0] = 0;
> 
> Page address may beyond 16 bit in large density product. 
> So I don't think addr[0] can be forced to be set 0
> 

Right, this was a bad copy paste from the staging driver.

> 
>> +static int spi_nand_mt29f_read_id(struct spi_nand *snand, u8 *buf) {
>> +	struct spi_nand_device *snand_dev = snand->priv;
>> +	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
>> +
>> +	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
>> +	cmd->n_rx = SPI_NAND_MT29F_READID_LEN;
>> +	cmd->rx_buf = buf;
>> +
> 
> Micron SPI NAND need 1 more dummy byte before received ID data.
> Code for mt29f_read_id should be like this:
> 
> static int spi_nand_mt29f_read_id(struct spi_nand *snand, u8 *buf) {
> 	struct spi_nand_device *snand_dev = snand->priv;
> 	struct spi_nand_device_cmd *cmd = &snand_dev->cmd;
> 	u8 dummy = 0;
> 	memset(cmd, 0, sizeof(struct spi_nand_device_cmd));
> "
> 	cmd->n_tx = 1;
> 	cmd->tx_buf = &dummy;
> "
> 	cmd->n_rx = SPI_NAND_MT29F_READID_LEN;
> 	cmd->rx_buf = buf;
> 

That's true. However, notice the staging driver does not use any dummy
byte, and instead fakes it by reading an extra byte.

Maybe you can test this on a MT29F and point at all the required fixes?

Thanks a lot for the feedback!
-- 
Ezequiel