[PATCH v5 0/6] Introduction to SPI NAND framework

[PATCH v5 0/6] Introduction to SPI NAND framework

[Peter Pan]

First of all, thank Boris, Marek and Cyrille for your comments
on v4 and thank Arnaud for your testing on v4. Since I'm quite
busy last week, I failed to reply all the comments, I'm really
sorry for that.

According to Boris's suggestion, I rebased my patches on nand/next
branch with Boris's generic NAND patches. This time I only send
my SPI NAND patches out since it's the focus.

I created my own branch for convenience[3]. You can find both
my SPI NAND patches and Boris's generic NAND framework patches.

This series introductes a SPI NAND framework.
SPI NAND is a new NAND family device with SPI protocol as
its interface. And its command set is totally different
with parallel NAND.

Our first attempt was more than 2 years ago[1]. At that
time, I didn't make BBT shareable and there were too many
duplicate code with parallel NAND, so that serie stoped.
But the discussion never stops. Now Boris has a plan to
make a generic NAND framework which can be shared with
both parallel and SPI NAND. Now the first step of the
new generic NAND framework is finished. And it is waiting
for a user. After discussion with Boris. We both think it's
time to rebuild SPI NAND framework based on the new NAND
framework and send out for reviewing.

This series is based on Boris's nand/generic branch[2], which
is on 4.11-rc1. In this serie, BBT code is totally shared.
Of course SPI NAND can share more code with parallel, this
requires to put more in new NAND core (now only BBT included).
I'd like to send this serie out first, then we can decide
which part should be in new NAND core.

This series only supports basic SPI NAND features and uses
generic spi controller for data transfer, on-die ECC for data
correction. Support advanced features and specific SPI NAND
controller with hardware ECC is the next step.

This series is tested on Xilinx Zedboard with Micron
MT29F2G01ABAGDSF SPI NAND chip.


v5 changes:
- rebase patch on nand/next with Boris's generic NAND framework patches[3]
- replace pr_xxx() with dev_xxx()
- replace kzalloc()i/kfree() with devm_kzalloc()/devm_kfree()
- rename spinand_op_init() to spinand_init_op() for consistency
- remove command opcode in function comments
- use BIT(n) instead (1 << n) in macro
- remove manufactures.c and put spinand_manufacturers table in core.c
- change spinand_write_reg() u8 *buf argument to u8 value,
  since the length is always 1
- remove spinand_manufacture->detect() check, since it is always != NULL
- alloc spinand_ecc_engine struct in vendor.c when using on-die ECC
  (for hardware ECC, it should be in controllers/*.c)
- add comment header for struct spinand_op
- fix timeout bug in spinand_wait(), thanks for Arnaud's debug
- make spinand_manufacturers const
- add ecc_engine_ops pointer in struct micron_spinand_info
- make controller->cap assignment right with SPI_TX/RX_QUAD/DUAL flag

v4 changes:
- initialize struct mtd_oob_ops to 0 in bbt.c
- rename new added helper in nand.h to nand_check_xxxx()
- add struct mtd_oob_ops consistency check in nand_check_oob_ops()
- add dataleft in struct nand_page_iter instead of offs
- remove spinand_manufacturers->ops->detect() check since it is mandatory
- remove spinand_set_manufacturer_ops() and do the job in
  spinand_manufacturer_detect()
- move .priv out of struct spinand_controller
- add spinand_alloc/free/register/unregister() and make
  spinand_detect/init() static
- make BBT be configured by device tree
- chip->id.data stores raw ID directly
- refine device info print message after detect success
- add struct mtd_layout_ops pointer in struct micron_spinand_info
- remove micron_spinand_init() and do its job in micron_spinand_detect()
- fix BBT block cannot be erased bug

v3 changes:
- rebase patch on 4.11-rc1[2]
- change read ID method. read 4 bytes ID out then let ->detect() of each
  manufacutre driver to decode ID and detect the device.
- make SPI NAND id table private to each manufacutre driver
- fix coding style to make checkpatch.pl happy
- update the MAINTAINERS file for spi nand code
- add nand_size() helper in nand.h
- use nand_for_each_page() helper in spinand_do_read/write_ops()
- create helper to check boundaries in generic NAND code and use it
  in SPI NAND core
- rename spinand_base.c to core.c
- manufactures' drivers expose spinand_manufacturer struct instead of
  spinand_manufacturer_ops struct to keep Manufacture ID macro in
  manufactures' drivers and rename spinand_ids.c to manufacture.c
- rename spinand_micron.c to micron.c
- rename chips/ directory to controllers/
- rename generic_spi.c to generic-spi.c
- replace ->build_column_addr() and ->get_dummy() hooks with ->prepare_op() in
  spinand_manufacturer_ops struct
- rename __spinand_erase() to spinand_erase()
- rename spinand_erase() to spinand_erase_skip_bbt()
- rename spinand_scan_ident() to spinand_detect()
- rename spinand_scan_tail() to spinand_init()
- move non detect related code from spinand_detect() to spinand_init()
- remove spinand_fill_nandd, assign nand->ops in spinand_detect()
- merge v2 patch 3(bad block support) and patch 4(BBT support)
- drop getchip parameter, remove spinand_get/remove_device(), take the lock
  by caller directly
- fix function comment headers
- use nand_bbt_is_initialized() helper
- replace spinand_ecc_engine and spinand_controller object in spinand_device
  struct with pointer
- replace struct spinand_manufacturer_ops pointer in spinand_device struct
  with spinand_manufacturer struct

v2 changes:
- replace "spi_nand" with "spinand".
- rename spi nand related structs for better understanding.
- introduce spi nand controller, manufacturer and ecc_engine struct.
- add spi nand manufacturer initialization function refer to Boris's
  manuf-init branch.
- remove NAND_SKIP_BBTSCAN from series. Add it later when enabling HW ECC.
- reorganize series according to Boris's suggestion.


[1]http://lists.infradead.org/pipermail/linux-mtd/2015-January/057223.html
[2]https://github.com/bbrezillon/linux-0day/tree/nand/generic
[3]https://github.com/peterpansjtu/linux/tree/nand/spinand


Peter Pan (6):
  nand: spi: add basic blocks for infrastructure
  nand: spi: add basic operations support
  nand: spi: Add bad block support
  nand: spi: add Micron spi nand support
  nand: spi: Add generic SPI controller support
  MAINTAINERS: Add SPI NAND entry

 MAINTAINERS                                    |    9 +
 drivers/mtd/nand/Kconfig                       |    1 +
 drivers/mtd/nand/Makefile                      |    1 +
 drivers/mtd/nand/spi/Kconfig                   |    7 +
 drivers/mtd/nand/spi/Makefile                  |    3 +
 drivers/mtd/nand/spi/controllers/Kconfig       |    5 +
 drivers/mtd/nand/spi/controllers/Makefile      |    1 +
 drivers/mtd/nand/spi/controllers/generic-spi.c |  159 +++
 drivers/mtd/nand/spi/core.c                    | 1522 ++++++++++++++++++++++++
 drivers/mtd/nand/spi/micron.c                  |  254 ++++
 include/linux/mtd/spinand.h                    |  261 ++++
 11 files changed, 2223 insertions(+)
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/controllers/Kconfig
 create mode 100644 drivers/mtd/nand/spi/controllers/Makefile
 create mode 100644 drivers/mtd/nand/spi/controllers/generic-spi.c
 create mode 100644 drivers/mtd/nand/spi/core.c
 create mode 100644 drivers/mtd/nand/spi/micron.c
 create mode 100644 include/linux/mtd/spinand.h

-- 
1.9.1

[PATCH v5 1/6] nand: spi: add basic blocks for infrastructure

[Peter Pan]

This is the first commit for spi nand framkework.
This commit is to add add basic building blocks
for the SPI NAND infrastructure.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/Kconfig      |   1 +
 drivers/mtd/nand/Makefile     |   1 +
 drivers/mtd/nand/spi/Kconfig  |   5 +
 drivers/mtd/nand/spi/Makefile |   1 +
 drivers/mtd/nand/spi/core.c   | 455 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   | 257 ++++++++++++++++++++++++
 6 files changed, 720 insertions(+)
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/core.c
 create mode 100644 include/linux/mtd/spinand.h

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 1c1a1f4..7695fd8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -2,3 +2,4 @@ config MTD_NAND_CORE
 	tristate
 
 source "drivers/mtd/nand/raw/Kconfig"
+source "drivers/mtd/nand/spi/Kconfig"
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index fe430d9..6221958 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
 nandcore-objs :=  bbt.o
 
 obj-y	+= raw/
+obj-$(CONFIG_MTD_SPI_NAND)	+= spi/
diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
new file mode 100644
index 0000000..d77c46e
--- /dev/null
+++ b/drivers/mtd/nand/spi/Kconfig
@@ -0,0 +1,5 @@
+menuconfig MTD_SPI_NAND
+	tristate "SPI NAND device Support"
+	depends on MTD_NAND
+	help
+	  This is the framework for the SPI NAND device drivers.
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
new file mode 100644
index 0000000..a677a4d
--- /dev/null
+++ b/drivers/mtd/nand/spi/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MTD_SPI_NAND) += core.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
new file mode 100644
index 0000000..510c7cb
--- /dev/null
+++ b/drivers/mtd/nand/spi/core.c
@@ -0,0 +1,455 @@
+/*
+ *
+ * Copyright (c) 2009-2017 Micron Technology, Inc.
+ *
+ * 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/kernel.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/mtd/spinand.h>
+#include <linux/slab.h>
+
+/*
+ * spinand_exec_op - execute SPI NAND operation by controller ->exec_op() hook
+ * @chip: SPI NAND device structure
+ * @op: pointer to spinand_op struct
+ */
+static inline int spinand_exec_op(struct spinand_device *chip,
+				  struct spinand_op *op)
+{
+	return chip->controller.controller->ops->exec_op(chip, op);
+}
+
+/*
+ * spinand_init_op - initialize spinand_op struct
+ * @op: pointer to spinand_op struct
+ */
+static inline void spinand_init_op(struct spinand_op *op)
+{
+	memset(op, 0, sizeof(struct spinand_op));
+	op->addr_nbits = 1;
+	op->data_nbits = 1;
+}
+
+/*
+ * spinand_read_reg - read SPI NAND register
+ * @chip: SPI NAND device structure
+ * @reg; register to read
+ * @buf: buffer to store value
+ */
+static int spinand_read_reg(struct spinand_device *chip, u8 reg, u8 *buf)
+{
+	struct spinand_op op;
+	int ret;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_GET_FEATURE;
+	op.n_addr = 1;
+	op.addr[0] = reg;
+	op.n_rx = 1;
+	op.rx_buf = buf;
+
+	ret = spinand_exec_op(chip, &op);
+	if (ret < 0)
+		dev_err(chip->dev, "err: %d read register %d\n", ret, reg);
+
+	return ret;
+}
+
+/*
+ * spinand_write_reg - write SPI NAND register
+ * @chip: SPI NAND device structure
+ * @reg; register to write
+ * @value: value to write
+ */
+static int spinand_write_reg(struct spinand_device *chip, u8 reg, u8 value)
+{
+	struct spinand_op op;
+	int ret;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_SET_FEATURE;
+	op.n_addr = 1;
+	op.addr[0] = reg;
+	op.n_tx = 1;
+	op.tx_buf = &value;
+
+	ret = spinand_exec_op(chip, &op);
+	if (ret < 0)
+		dev_err(chip->dev, "err: %d write register %d\n", ret, reg);
+
+	return ret;
+}
+
+/*
+ * spinand_read_status - get status register value
+ * @chip: SPI NAND device structure
+ * @status: buffer to store value
+ * Description:
+ *   After read, write, or erase, the NAND device is expected to set the
+ *   busy status.
+ *   This function is to allow reading the status of the command: read,
+ *   write, and erase.
+ */
+static int spinand_read_status(struct spinand_device *chip, u8 *status)
+{
+	return spinand_read_reg(chip, REG_STATUS, status);
+}
+
+/*
+ * spinand_wait - wait until the command is done
+ * @chip: SPI NAND device structure
+ * @s: buffer to store status register value (can be NULL)
+ */
+static int spinand_wait(struct spinand_device *chip, u8 *s)
+{
+	unsigned long timeo =  jiffies + msecs_to_jiffies(400);
+	u8 status;
+
+	do {
+		spinand_read_status(chip, &status);
+		if ((status & STATUS_OIP_MASK) == STATUS_READY)
+			goto out;
+	} while (time_before(jiffies, timeo));
+
+	/*
+	 * Extra read, just in case the STATUS_READY bit has changed
+	 * since our last check
+	 */
+	spinand_read_status(chip, &status);
+out:
+	if (s)
+		*s = status;
+
+	return (status & STATUS_OIP_MASK) == STATUS_READY ? 0 :	-ETIMEDOUT;
+}
+
+/*
+ * spinand_read_id - read SPI NAND ID
+ * @chip: SPI NAND device structure
+ * @buf: buffer to store id
+ * Description:
+ *   Manufacturers' read ID method is not unique. Some need a dummy before
+ *   reading, some's ID has three byte.
+ *   This function send one byte opcode (9Fh) and then read
+ *   SPINAND_MAX_ID_LEN (4 currently) bytes. Manufacturer's detect function
+ *   need to filter out real ID from the 4 bytes.
+ */
+static int spinand_read_id(struct spinand_device *chip, u8 *buf)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_READ_ID;
+	op.n_rx = SPINAND_MAX_ID_LEN;
+	op.rx_buf = buf;
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
+ * spinand_reset - reset SPI NAND device
+ * @chip: SPI NAND device structure
+ */
+static int spinand_reset(struct spinand_device *chip)
+{
+	struct spinand_op op;
+	int ret;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_RESET;
+
+	ret = spinand_exec_op(chip, &op);
+	if (ret < 0) {
+		dev_err(chip->dev, "reset failed!\n");
+		goto out;
+	}
+	ret = spinand_wait(chip, NULL);
+
+out:
+	return ret;
+}
+
+/*
+ * spinand_lock_block - write block lock register to lock/unlock device
+ * @chip: SPI NAND device structure
+ * @lock: value to set to block lock register
+ */
+static int spinand_lock_block(struct spinand_device *chip, u8 lock)
+{
+	return spinand_write_reg(chip, REG_BLOCK_LOCK, lock);
+}
+
+/*
+ * spinand_set_rd_wr_op - choose the best read write command
+ * @chip: SPI NAND device structure
+ * Description:
+ *   Chose the fastest r/w command according to spi controller's and
+ *   device's ability.
+ */
+static void spinand_set_rd_wr_op(struct spinand_device *chip)
+{
+	u32 controller_cap = chip->controller.controller->caps;
+	u32 rw_mode = chip->rw_mode;
+
+	if ((controller_cap & SPINAND_CAP_RD_QUAD) &&
+	    (rw_mode & SPINAND_RD_QUAD))
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_QUAD_IO;
+	else if ((controller_cap & SPINAND_CAP_RD_X4) &&
+		 (rw_mode & SPINAND_RD_X4))
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_X4;
+	else if ((controller_cap & SPINAND_CAP_RD_DUAL) &&
+		 (rw_mode & SPINAND_RD_DUAL))
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_DUAL_IO;
+	else if ((controller_cap & SPINAND_CAP_RD_X2) &&
+		 (rw_mode & SPINAND_RD_X2))
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_X2;
+	else
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_FAST;
+
+	if ((controller_cap & SPINAND_CAP_WR_X4) &&
+	    (rw_mode & SPINAND_WR_X4))
+		chip->write_cache_op = SPINAND_CMD_PROG_LOAD_X4;
+	else
+		chip->write_cache_op = SPINAND_CMD_PROG_LOAD;
+}
+
+static const struct spinand_manufacturer *spinand_manufacturers[] = {};
+
+/*
+ * spinand_manufacturer_detect - detect SPI NAND device by each manufacturer
+ * @chip: SPI NAND device structure
+ *
+ * ->detect() should decode raw id in chip->id.data and initialize device
+ * related part in spinand_device structure if it is the right device.
+ * ->detect() can not be NULL.
+ */
+static int spinand_manufacturer_detect(struct spinand_device *chip)
+{
+	int i = 0;
+
+	for (; i < ARRAY_SIZE(spinand_manufacturers); i++) {
+		if (spinand_manufacturers[i]->ops->detect(chip)) {
+			chip->manufacturer.manu = spinand_manufacturers[i];
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+/*
+ * spinand_manufacturer_init - manufacturer initialization function.
+ * @chip: SPI NAND device structure
+ *
+ * Manufacturer drivers should put all their specific initialization code in
+ * their ->init() hook.
+ */
+static int spinand_manufacturer_init(struct spinand_device *chip)
+{
+	if (chip->manufacturer.manu->ops->init)
+		return chip->manufacturer.manu->ops->init(chip);
+
+	return 0;
+}
+
+/*
+ * spinand_manufacturer_cleanup - manufacturer cleanup function.
+ * @chip: SPI NAND device structure
+ *
+ * Manufacturer drivers should put all their specific cleanup code in their
+ * ->cleanup() hook.
+ */
+static void spinand_manufacturer_cleanup(struct spinand_device *chip)
+{
+	/* Release manufacturer private data */
+	if (chip->manufacturer.manu->ops->cleanup)
+		return chip->manufacturer.manu->ops->cleanup(chip);
+}
+
+/*
+ * spinand_dt_init - Initialize SPI NAND by device tree node
+ * @chip: SPI NAND device structure
+ *
+ * TODO: put ecc_mode, ecc_strength, ecc_step, bbt, etc in here
+ * and move it in generic NAND core.
+ */
+static void spinand_dt_init(struct spinand_device *chip)
+{
+}
+
+/*
+ * spinand_detect - detect the SPI NAND device
+ * @chip: SPI NAND device structure
+ */
+static int spinand_detect(struct spinand_device *chip)
+{
+	struct nand_device *nand = &chip->base;
+	int ret;
+
+	spinand_reset(chip);
+	spinand_read_id(chip, chip->id.data);
+	chip->id.len = SPINAND_MAX_ID_LEN;
+
+	ret = spinand_manufacturer_detect(chip);
+	if (ret) {
+		dev_err(chip->dev, "unknown raw ID %*phN\n",
+			SPINAND_MAX_ID_LEN, chip->id.data);
+		goto out;
+	}
+
+	dev_info(chip->dev, "%s (%s) is found.\n", chip->name,
+		 chip->manufacturer.manu->name);
+	dev_info(chip->dev,
+		 "%d MiB, block size: %d KiB, page size: %d, OOB size: %d\n",
+		 (int)(nand_size(nand) >> 20), nand_eraseblock_size(nand) >> 10,
+		 nand_page_size(nand), nand_per_page_oobsize(nand));
+
+out:
+	return ret;
+}
+
+/*
+ * spinand_init - initialize the SPI NAND device
+ * @chip: SPI NAND device structure
+ */
+static int spinand_init(struct spinand_device *chip)
+{
+	struct mtd_info *mtd = spinand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	struct spinand_ecc_engine *ecc_engine;
+	int ret;
+
+	spinand_dt_init(chip);
+	spinand_set_rd_wr_op(chip);
+
+	chip->buf = devm_kzalloc(chip->dev,
+				 nand_page_size(nand) +
+				 nand_per_page_oobsize(nand),
+				 GFP_KERNEL);
+	if (!chip->buf) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	chip->oobbuf = chip->buf + nand_page_size(nand);
+
+	spinand_manufacturer_init(chip);
+
+	mtd->name = chip->name;
+	mtd->size = nand_size(nand);
+	mtd->erasesize = nand_eraseblock_size(nand);
+	mtd->writesize = nand_page_size(nand);
+	mtd->writebufsize = mtd->writesize;
+	mtd->owner = THIS_MODULE;
+	mtd->type = MTD_NANDFLASH;
+	mtd->flags = MTD_CAP_NANDFLASH;
+	if (!mtd->ecc_strength)
+		mtd->ecc_strength = ecc_engine->strength ?
+				    ecc_engine->strength : 1;
+
+	mtd->oobsize = nand_per_page_oobsize(nand);
+	ret = mtd_ooblayout_count_freebytes(mtd);
+	if (ret < 0)
+		ret = 0;
+	mtd->oobavail = ret;
+
+	if (!mtd->bitflip_threshold)
+		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3,
+						      4);
+	/* After power up, all blocks are locked, so unlock it here. */
+	spinand_lock_block(chip, BL_ALL_UNLOCKED);
+
+	return nand_register(nand);
+
+err:
+	return ret;
+}
+
+/*
+ * spinand_alloc - [SPI NAND Interface] allocate SPI NAND device instance
+ * @dev: pointer to device model structure
+ */
+struct spinand_device *spinand_alloc(struct device *dev)
+{
+	struct spinand_device *chip;
+	struct mtd_info *mtd;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return ERR_PTR(-ENOMEM);
+
+	spinand_set_of_node(chip, dev->of_node);
+	mutex_init(&chip->lock);
+	chip->dev = dev;
+	mtd = spinand_to_mtd(chip);
+	mtd->dev.parent = dev;
+
+	return chip;
+}
+EXPORT_SYMBOL_GPL(spinand_alloc);
+
+/*
+ * spinand_free - [SPI NAND Interface] free SPI NAND device instance
+ * @chip: SPI NAND device structure
+ */
+void spinand_free(struct spinand_device *chip)
+{
+	devm_kfree(chip->dev, chip);
+}
+EXPORT_SYMBOL_GPL(spinand_free);
+
+/*
+ * spinand_register - [SPI NAND Interface] register SPI NAND device
+ * @chip: SPI NAND device structure
+ */
+int spinand_register(struct spinand_device *chip)
+{
+	int ret;
+
+	ret = spinand_detect(chip);
+	if (ret) {
+		dev_err(chip->dev,
+			"Detect SPI NAND failed with error %d.\n", ret);
+		return ret;
+	}
+
+	ret = spinand_init(chip);
+	if (ret)
+		dev_err(chip->dev,
+			"Init SPI NAND failed with error %d.\n", ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(spinand_register);
+
+/*
+ * spinand_unregister - [SPI NAND Interface] unregister SPI NAND device
+ * @chip: SPI NAND device structure
+ */
+int spinand_unregister(struct spinand_device *chip)
+{
+	struct nand_device *nand = &chip->base;
+
+	nand_unregister(nand);
+	spinand_manufacturer_cleanup(chip);
+	devm_kfree(chip->dev, chip->buf);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spinand_unregister);
+
+MODULE_DESCRIPTION("SPI NAND framework");
+MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
new file mode 100644
index 0000000..78ea9a6
--- /dev/null
+++ b/include/linux/mtd/spinand.h
@@ -0,0 +1,257 @@
+/*
+ *
+ * Copyright (c) 2009-2017 Micron Technology, Inc.
+ *
+ * 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.
+ */
+#ifndef __LINUX_MTD_SPINAND_H
+#define __LINUX_MTD_SPINAND_H
+
+#include <linux/mutex.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+
+/*
+ * Standard SPI NAND flash commands
+ */
+#define SPINAND_CMD_RESET			0xff
+#define SPINAND_CMD_GET_FEATURE			0x0f
+#define SPINAND_CMD_SET_FEATURE			0x1f
+#define SPINAND_CMD_PAGE_READ			0x13
+#define SPINAND_CMD_READ_FROM_CACHE		0x03
+#define SPINAND_CMD_READ_FROM_CACHE_FAST	0x0b
+#define SPINAND_CMD_READ_FROM_CACHE_X2		0x3b
+#define SPINAND_CMD_READ_FROM_CACHE_DUAL_IO	0xbb
+#define SPINAND_CMD_READ_FROM_CACHE_X4		0x6b
+#define SPINAND_CMD_READ_FROM_CACHE_QUAD_IO	0xeb
+#define SPINAND_CMD_BLK_ERASE			0xd8
+#define SPINAND_CMD_PROG_EXC			0x10
+#define SPINAND_CMD_PROG_LOAD			0x02
+#define SPINAND_CMD_PROG_LOAD_RDM_DATA		0x84
+#define SPINAND_CMD_PROG_LOAD_X4		0x32
+#define SPINAND_CMD_PROG_LOAD_RDM_DATA_X4	0x34
+#define SPINAND_CMD_READ_ID			0x9f
+#define SPINAND_CMD_WR_DISABLE			0x04
+#define SPINAND_CMD_WR_ENABLE			0x06
+
+/* feature register */
+#define REG_BLOCK_LOCK		0xa0
+#define REG_CFG			0xb0
+#define REG_STATUS		0xc0
+
+/* status register */
+#define STATUS_OIP_MASK		BIT(0)
+#define STATUS_CRBSY_MASK	BIT(7)
+#define STATUS_READY		0
+#define STATUS_BUSY		BIT(0)
+
+#define STATUS_E_FAIL_MASK	BIT(2)
+#define STATUS_E_FAIL		BIT(2)
+
+#define STATUS_P_FAIL_MASK	BIT(3)
+#define STATUS_P_FAIL		BIT(3)
+
+/* configuration register */
+#define CFG_ECC_MASK		BIT(4)
+#define CFG_ECC_ENABLE		BIT(4)
+
+/* block lock register */
+#define BL_ALL_UNLOCKED		0X00
+
+struct spinand_op;
+struct spinand_device;
+
+#define SPINAND_MAX_ID_LEN	4
+
+/*
+ * struct spinand_id - SPI NAND id structure
+ * @data: buffer containing the id bytes. Currently 4 bytes large, but can
+ *	  be extended if required.
+ * @len: ID length.
+ */
+struct spinand_id {
+	u8 data[SPINAND_MAX_ID_LEN];
+	int len;
+};
+
+struct spinand_controller_ops {
+	int (*exec_op)(struct spinand_device *chip,
+		       struct spinand_op *op);
+};
+
+struct spinand_manufacturer_ops {
+	/*
+	 * Firstly, ->detect() should not be NULL.
+	 * ->detect() implementation for manufacturer A never sends
+	 * any manufacturer specific SPI command to a SPI NAND from
+	 * manufacturer B, so the proper way is to decode the raw id
+	 * data in chip->id.data first, if manufacture ID dismatch,
+	 * return directly and let others to detect.
+	 */
+	bool (*detect)(struct spinand_device *chip);
+	int (*init)(struct spinand_device *chip);
+	void (*cleanup)(struct spinand_device *chip);
+};
+
+struct spinand_manufacturer {
+	u8 id;
+	char *name;
+	const struct spinand_manufacturer_ops *ops;
+};
+
+struct spinand_ecc_engine_ops {
+	void (*get_ecc_status)(struct spinand_device *chip,
+			       unsigned int status, unsigned int *corrected,
+			       unsigned int *ecc_errors);
+	void (*disable_ecc)(struct spinand_device *chip);
+	void (*enable_ecc)(struct spinand_device *chip);
+};
+
+enum spinand_ecc_type {
+	SPINAND_ECC_ONDIE,
+	SPINAND_ECC_HW,
+};
+
+struct spinand_ecc_engine {
+	u32 strength;
+	u32 steps;
+	const struct spinand_ecc_engine_ops *ops;
+};
+
+#define SPINAND_CAP_RD_X1	BIT(0)
+#define SPINAND_CAP_RD_X2	BIT(1)
+#define SPINAND_CAP_RD_X4	BIT(2)
+#define SPINAND_CAP_RD_DUAL	BIT(3)
+#define SPINAND_CAP_RD_QUAD	BIT(4)
+#define SPINAND_CAP_WR_X1	BIT(5)
+#define SPINAND_CAP_WR_X2	BIT(6)
+#define SPINAND_CAP_WR_X4	BIT(7)
+#define SPINAND_CAP_WR_DUAL	BIT(8)
+#define SPINAND_CAP_WR_QUAD	BIT(9)
+#define SPINAND_CAP_HW_ECC	BIT(10)
+
+struct spinand_controller {
+	struct spinand_controller_ops *ops;
+	u32 caps;
+};
+
+/*
+ * struct spinand_device - SPI NAND device
+ * @base: NAND device instance
+ * @lock: protection lock
+ * @name: name of the chip
+ * @dev: struct device pointer
+ * @id: ID structure
+ * @read_cache_op: Opcode of read from cache
+ * @write_cache_op: Opcode of program load
+ * @buf: buffer for read/write data
+ * @oobbuf: buffer for read/write oob
+ * @rw_mode: read/write mode of SPI NAND chip
+ * @controller: SPI NAND controller instance
+ * @manufacturer: SPI NAND manufacturer instance, describe
+ *                manufacturer related objects
+ * @ecc_engine: SPI NAND ECC engine instance
+ */
+struct spinand_device {
+	struct nand_device base;
+	struct mutex lock;
+	char *name;
+	struct device *dev;
+	struct spinand_id id;
+	u8 read_cache_op;
+	u8 write_cache_op;
+	u8 *buf;
+	u8 *oobbuf;
+	u32 rw_mode;
+	struct {
+		struct spinand_controller *controller;
+		void *priv;
+	} controller;
+	struct {
+		const struct spinand_manufacturer *manu;
+		void *priv;
+	} manufacturer;
+	struct {
+		struct spinand_ecc_engine *engine;
+		enum spinand_ecc_type type;
+		void *context;
+	} ecc;
+};
+
+static inline struct spinand_device *mtd_to_spinand(struct mtd_info *mtd)
+{
+	return container_of(mtd_to_nand(mtd), struct spinand_device, base);
+}
+
+static inline struct mtd_info *spinand_to_mtd(struct spinand_device *chip)
+{
+	return nand_to_mtd(&chip->base);
+}
+
+static inline void spinand_set_of_node(struct spinand_device *chip,
+				       struct device_node *np)
+{
+	nand_set_of_node(&chip->base, np);
+}
+
+#define SPINAND_MAX_ADDR_LEN	4
+
+/*
+ * struct spinand_op - SPI NAND operation description
+ * @cmd: opcode to send
+ * @n_addr: address bytes
+ * @addr_nbits: number of bit used to transfer address
+ * @dummy_types: dummy bytes followed address
+ * @addr: buffer held address
+ * @n_tx: size of tx_buf
+ * @tx_buf: data to be written
+ * @n_rx: size of rx_buf
+ * @rx_buf: data to be read
+ * @data_nbits: number of bit used to transfer data
+ */
+struct spinand_op {
+	u8 cmd;
+	u8 n_addr;
+	u8 addr_nbits;
+	u8 dummy_bytes;
+	u8 addr[SPINAND_MAX_ADDR_LEN];
+	u32 n_tx;
+	const u8 *tx_buf;
+	u32 n_rx;
+	u8 *rx_buf;
+	u8 data_nbits;
+};
+
+/* SPI NAND supported OP mode */
+#define SPINAND_RD_X1		0x00000001
+#define SPINAND_RD_X2		0x00000002
+#define SPINAND_RD_X4		0x00000004
+#define SPINAND_RD_DUAL		0x00000008
+#define SPINAND_RD_QUAD		0x00000010
+#define SPINAND_WR_X1		0x00000020
+#define SPINAND_WR_X2		0x00000040
+#define SPINAND_WR_X4		0x00000080
+#define SPINAND_WR_DUAL		0x00000100
+#define SPINAND_WR_QUAD		0x00000200
+
+#define SPINAND_RD_COMMON	(SPINAND_RD_X1 | SPINAND_RD_X2 | \
+				 SPINAND_RD_X4 | SPINAND_RD_DUAL | \
+				 SPINAND_RD_QUAD)
+#define SPINAND_WR_COMMON	(SPINAND_WR_X1 | SPINAND_WR_X4)
+#define SPINAND_OP_COMMON	(SPINAND_RD_COMMON | SPINAND_WR_COMMON)
+
+struct spinand_device *spinand_alloc(struct device *dev);
+void spinand_free(struct spinand_device *chip);
+int spinand_register(struct spinand_device *chip);
+int spinand_unregister(struct spinand_device *chip);
+#endif /* __LINUX_MTD_SPINAND_H */
-- 
1.9.1

[PATCH v5 2/6] nand: spi: add basic operations support

[Peter Pan]

This commit is to support read, readoob, write,
writeoob and erase operations in the new spi nand
framework.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/spi/core.c | 750 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h |   2 +
 2 files changed, 752 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 510c7cb..eb7af9d 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -108,6 +108,232 @@ static int spinand_read_status(struct spinand_device *chip, u8 *status)
 }
 
 /*
+ * spinand_get_cfg - get configuration register value
+ * @chip: SPI NAND device structure
+ * @cfg: buffer to store value
+ * Description:
+ *   Configuration register includes OTP config, Lock Tight enable/disable
+ *   and Internal ECC enable/disable.
+ */
+static int spinand_get_cfg(struct spinand_device *chip, u8 *cfg)
+{
+	return spinand_read_reg(chip, REG_CFG, cfg);
+}
+
+/*
+ * spinand_set_cfg - set value to configuration register
+ * @chip: SPI NAND device structure
+ * @cfg: value to set
+ * Description:
+ *   Configuration register includes OTP config, Lock Tight enable/disable
+ *   and Internal ECC enable/disable.
+ */
+static int spinand_set_cfg(struct spinand_device *chip, u8 cfg)
+{
+	return spinand_write_reg(chip, REG_CFG, cfg);
+}
+
+/*
+ * spinand_enable_ecc - enable internal ECC
+ * @chip: SPI NAND device structure
+ */
+static void spinand_enable_ecc(struct spinand_device *chip)
+{
+	u8 cfg = 0;
+
+	spinand_get_cfg(chip, &cfg);
+	if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE)
+		return;
+	cfg |= CFG_ECC_ENABLE;
+	spinand_set_cfg(chip, cfg);
+}
+
+/*
+ * spinand_disable_ecc - disable internal ECC
+ * @chip: SPI NAND device structure
+ */
+static void spinand_disable_ecc(struct spinand_device *chip)
+{
+	u8 cfg = 0;
+
+	spinand_get_cfg(chip, &cfg);
+	if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE) {
+		cfg &= ~CFG_ECC_ENABLE;
+		spinand_set_cfg(chip, cfg);
+	}
+}
+
+/*
+ * spinand_write_enable - send command 06h to enable write or erase the
+ * NAND cells
+ * @chip: SPI NAND device structure
+ */
+static int spinand_write_enable(struct spinand_device *chip)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_WR_ENABLE;
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
+ * spinand_read_page_to_cache - send command 13h to read data from NAND array
+ * to cache
+ * @chip: SPI NAND device structure
+ * @page_addr: page to read
+ */
+static int spinand_read_page_to_cache(struct spinand_device *chip,
+				      u32 page_addr)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_PAGE_READ;
+	op.n_addr = 3;
+	op.addr[0] = (u8)(page_addr >> 16);
+	op.addr[1] = (u8)(page_addr >> 8);
+	op.addr[2] = (u8)page_addr;
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
+ * spinand_get_address_bits - return address should be transferred
+ * by how many bits
+ * @opcode: command's operation code
+ */
+static int spinand_get_address_bits(u8 opcode)
+{
+	switch (opcode) {
+	case SPINAND_CMD_READ_FROM_CACHE_QUAD_IO:
+		return 4;
+	case SPINAND_CMD_READ_FROM_CACHE_DUAL_IO:
+		return 2;
+	default:
+		return 1;
+	}
+}
+
+/*
+ * spinand_get_data_bits - return data should be transferred by how many bits
+ * @opcode: command's operation code
+ */
+static int spinand_get_data_bits(u8 opcode)
+{
+	switch (opcode) {
+	case SPINAND_CMD_READ_FROM_CACHE_QUAD_IO:
+	case SPINAND_CMD_READ_FROM_CACHE_X4:
+	case SPINAND_CMD_PROG_LOAD_X4:
+	case SPINAND_CMD_PROG_LOAD_RDM_DATA_X4:
+		return 4;
+	case SPINAND_CMD_READ_FROM_CACHE_DUAL_IO:
+	case SPINAND_CMD_READ_FROM_CACHE_X2:
+		return 2;
+	default:
+		return 1;
+	}
+}
+
+/*
+ * spinand_read_from_cache - read data out from cache register
+ * @chip: SPI NAND device structure
+ * @page_addr: page to read
+ * @column: the location to read from the cache
+ * @len: number of bytes to read
+ * @rbuf: buffer held @len bytes
+ */
+static int spinand_read_from_cache(struct spinand_device *chip, u32 page_addr,
+				   u32 column, size_t len, u8 *rbuf)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = chip->read_cache_op;
+	op.n_addr = 2;
+	op.addr[0] = (u8)(column >> 8);
+	op.addr[1] = (u8)column;
+	op.addr_nbits = spinand_get_address_bits(chip->read_cache_op);
+	op.n_rx = len;
+	op.rx_buf = rbuf;
+	op.data_nbits = spinand_get_data_bits(chip->read_cache_op);
+	if (chip->manufacturer.manu->ops->prepare_op)
+		chip->manufacturer.manu->ops->prepare_op(chip, &op,
+							 page_addr, column);
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
+ * spinand_write_to_cache - write data to cache register
+ * @chip: SPI NAND device structure
+ * @page_addr: page to write
+ * @column: the location to write to the cache
+ * @len: number of bytes to write
+ * @wrbuf: buffer held @len bytes
+ */
+static int spinand_write_to_cache(struct spinand_device *chip, u32 page_addr,
+				  u32 column, size_t len, const u8 *wbuf)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = chip->write_cache_op;
+	op.n_addr = 2;
+	op.addr[0] = (u8)(column >> 8);
+	op.addr[1] = (u8)column;
+	op.addr_nbits = spinand_get_address_bits(chip->write_cache_op);
+	op.n_tx = len;
+	op.tx_buf = wbuf;
+	op.data_nbits = spinand_get_data_bits(chip->write_cache_op);
+	if (chip->manufacturer.manu->ops->prepare_op)
+		chip->manufacturer.manu->ops->prepare_op(chip, &op,
+							 page_addr, column);
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
+ * spinand_program_execute - send command 10h to write a page from
+ * cache to the NAND array
+ * @chip: SPI NAND device structure
+ * @page_addr: the physical page location to write the page.
+ */
+static int spinand_program_execute(struct spinand_device *chip, u32 page_addr)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_PROG_EXC;
+	op.n_addr = 3;
+	op.addr[0] = (u8)(page_addr >> 16);
+	op.addr[1] = (u8)(page_addr >> 8);
+	op.addr[2] = (u8)page_addr;
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
+ * spinand_erase_block_erase - send command D8h to erase a block
+ * @chip: SPI NAND device structure
+ * @page_addr: the start page address of block to be erased.
+ */
+static int spinand_erase_block(struct spinand_device *chip, u32 page_addr)
+{
+	struct spinand_op op;
+
+	spinand_init_op(&op);
+	op.cmd = SPINAND_CMD_BLK_ERASE;
+	op.n_addr = 3;
+	op.addr[0] = (u8)(page_addr >> 16);
+	op.addr[1] = (u8)(page_addr >> 8);
+	op.addr[2] = (u8)page_addr;
+
+	return spinand_exec_op(chip, &op);
+}
+
+/*
  * spinand_wait - wait until the command is done
  * @chip: SPI NAND device structure
  * @s: buffer to store status register value (can be NULL)
@@ -192,6 +418,525 @@ static int spinand_lock_block(struct spinand_device *chip, u8 lock)
 }
 
 /*
+ * spinand_get_ecc_status - get ecc correction information from status register
+ * @chip: SPI NAND device structure
+ * @status: status register value
+ * @corrected: corrected bit flip number
+ * @ecc_error: ecc correction error or not
+ */
+static void spinand_get_ecc_status(struct spinand_device *chip,
+				   unsigned int status,
+				   unsigned int *corrected,
+				   unsigned int *ecc_error)
+{
+	return chip->ecc.engine->ops->get_ecc_status(chip, status, corrected,
+						     ecc_error);
+}
+
+/*
+ * spinand_do_read_page - read page from device to buffer
+ * @mtd: MTD device structure
+ * @page_addr: page address/raw address
+ * @ecc_off: without ecc or not
+ * @corrected: how many bit flip corrected
+ * @oob_only: read OOB only or the whole page
+ */
+static int spinand_do_read_page(struct mtd_info *mtd, u32 page_addr,
+				bool ecc_off, int *corrected, bool oob_only)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret, ecc_error = 0;
+	u8 status;
+
+	spinand_read_page_to_cache(chip, page_addr);
+	ret = spinand_wait(chip, &status);
+	if (ret < 0) {
+		dev_err(chip->dev, "error %d waiting page 0x%x to cache\n",
+			ret, page_addr);
+		return ret;
+	}
+	if (!oob_only)
+		spinand_read_from_cache(chip, page_addr, 0,
+					nand_page_size(nand) +
+					nand_per_page_oobsize(nand),
+					chip->buf);
+	else
+		spinand_read_from_cache(chip, page_addr, nand_page_size(nand),
+					nand_per_page_oobsize(nand),
+					chip->oobbuf);
+	if (!ecc_off) {
+		spinand_get_ecc_status(chip, status, corrected, &ecc_error);
+		/*
+		 * If there's an ECC error, print a message and notify MTD
+		 * about it. Then complete the read, to load actual data on
+		 * the buffer (instead of the status result).
+		 */
+		if (ecc_error) {
+			dev_err(chip->dev,
+				"internal ECC error reading page 0x%x\n",
+				page_addr);
+			mtd->ecc_stats.failed++;
+		} else if (*corrected) {
+			mtd->ecc_stats.corrected += *corrected;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * spinand_do_write_page - write data from buffer to device
+ * @mtd: MTD device structure
+ * @page_addr: page address/raw address
+ * @oob_only: write OOB only or the whole page
+ */
+static int spinand_do_write_page(struct mtd_info *mtd, u32 page_addr,
+				 bool oob_only)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	u8 status;
+	int ret = 0;
+
+	spinand_write_enable(chip);
+	if (!oob_only)
+		spinand_write_to_cache(chip, page_addr, 0,
+				       nand_page_size(nand) +
+				       nand_per_page_oobsize(nand), chip->buf);
+	else
+		spinand_write_to_cache(chip, page_addr, nand_page_size(nand),
+				       nand_per_page_oobsize(nand),
+				       chip->oobbuf);
+	spinand_program_execute(chip, page_addr);
+	ret = spinand_wait(chip, &status);
+	if (ret < 0) {
+		dev_err(chip->dev, "error %d reading page 0x%x from cache\n",
+			ret, page_addr);
+		return ret;
+	}
+	if ((status & STATUS_P_FAIL_MASK) == STATUS_P_FAIL) {
+		dev_err(chip->dev, "program page 0x%x failed\n", page_addr);
+		ret = -EIO;
+	}
+	return ret;
+}
+
+/*
+ * spinand_transfer_oob - transfer oob to client buffer
+ * @chip: SPI NAND device structure
+ * @oob: oob destination address
+ * @ops: oob ops structure
+ * @len: size of oob to transfer
+ */
+static int spinand_transfer_oob(struct spinand_device *chip, u8 *oob,
+				struct mtd_oob_ops *ops, size_t len)
+{
+	struct mtd_info *mtd = spinand_to_mtd(chip);
+	int ret = 0;
+
+	switch (ops->mode) {
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_RAW:
+		memcpy(oob, chip->oobbuf + ops->ooboffs, len);
+		break;
+	case MTD_OPS_AUTO_OOB:
+		ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
+						  ops->ooboffs, len);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+/*
+ * spinand_fill_oob - transfer client buffer to oob
+ * @chip: SPI NAND device structure
+ * @oob: oob data buffer
+ * @len: oob data write length
+ * @ops: oob ops structure
+ */
+static int spinand_fill_oob(struct spinand_device *chip, uint8_t *oob,
+			    size_t len, struct mtd_oob_ops *ops)
+{
+	struct mtd_info *mtd = spinand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret = 0;
+
+	memset(chip->oobbuf, 0xff, nand_per_page_oobsize(nand));
+	switch (ops->mode) {
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_RAW:
+		memcpy(chip->oobbuf + ops->ooboffs, oob, len);
+		break;
+	case MTD_OPS_AUTO_OOB:
+		ret = mtd_ooblayout_set_databytes(mtd, oob, chip->oobbuf,
+						  ops->ooboffs, len);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+/*
+ * spinand_read_pages - read data from device to buffer
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @ops: oob operations description structure
+ * @max_bitflips: maximum bitflip count
+ */
+static int spinand_read_pages(struct mtd_info *mtd, loff_t from,
+			      struct mtd_oob_ops *ops,
+			      unsigned int *max_bitflips)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int size, ret;
+	unsigned int corrected = 0;
+	bool ecc_off = ops->mode == MTD_OPS_RAW;
+	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
+		     mtd->oobavail : mtd->oobsize;
+	bool oob_only = !ops->datbuf;
+	struct nand_page_iter iter;
+
+	ops->retlen = 0;
+	ops->oobretlen = 0;
+	*max_bitflips = 0;
+
+	nand_for_each_page(nand, from, ops->len, ops->ooboffs, ops->ooblen,
+			   ooblen, &iter) {
+		ret = spinand_do_read_page(mtd, iter.page, ecc_off,
+					   &corrected, oob_only);
+		if (ret)
+			break;
+		*max_bitflips = max(*max_bitflips, corrected);
+		if (ops->datbuf) {
+			size = min_t(int, iter.dataleft,
+				     nand_page_size(nand) - iter.pageoffs);
+			memcpy(ops->datbuf + ops->retlen,
+			       chip->buf + iter.pageoffs, size);
+			ops->retlen += size;
+		}
+		if (ops->oobbuf) {
+			size = min_t(int, iter.oobleft, ooblen);
+			ret = spinand_transfer_oob(chip,
+						   ops->oobbuf + ops->oobretlen,
+						   ops, size);
+			if (ret) {
+				dev_err(chip->dev, "Transfer oob error %d\n", ret);
+				return ret;
+			}
+			ops->oobretlen += size;
+		}
+	}
+
+	return ret;
+}
+
+/*
+ * spinand_do_read_ops - read data from device to buffer
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @ops: oob operations description structure
+ */
+static int spinand_do_read_ops(struct mtd_info *mtd, loff_t from,
+			       struct mtd_oob_ops *ops)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret;
+	struct mtd_ecc_stats stats;
+	unsigned int max_bitflips = 0;
+	bool ecc_off = ops->mode == MTD_OPS_RAW;
+
+	ret = nand_check_address(nand, from);
+	if (ret) {
+		dev_err(chip->dev, "%s: invalid read address\n", __func__);
+		return ret;
+	}
+	ret = nand_check_oob_ops(nand, from, ops);
+	if (ret) {
+		dev_err(chip->dev,
+			"%s: invalid oob operation input\n", __func__);
+		return ret;
+	}
+	mutex_lock(&chip->lock);
+	stats = mtd->ecc_stats;
+	if (ecc_off)
+		spinand_disable_ecc(chip);
+	ret = spinand_read_pages(mtd, from, ops, &max_bitflips);
+	if (ecc_off)
+		spinand_enable_ecc(chip);
+	if (ret)
+		goto out;
+
+	if (mtd->ecc_stats.failed - stats.failed) {
+		ret = -EBADMSG;
+		goto out;
+	}
+	ret = max_bitflips;
+
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+/*
+ * spinand_write_pages - write data from buffer to device
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @ops: oob operations description structure
+ */
+static int spinand_write_pages(struct mtd_info *mtd, loff_t to,
+			       struct mtd_oob_ops *ops)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret = 0;
+	int size = 0;
+	int oob_size = 0;
+	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
+		     mtd->oobavail : mtd->oobsize;
+	bool oob_only = !ops->datbuf;
+	struct nand_page_iter iter;
+
+	ops->retlen = 0;
+	ops->oobretlen = 0;
+
+	nand_for_each_page(nand, to, ops->len, ops->ooboffs, ops->ooblen,
+			   ooblen, &iter) {
+		memset(chip->buf, 0xff,
+		       nand_page_size(nand) + nand_per_page_oobsize(nand));
+		if (ops->oobbuf) {
+			oob_size = min_t(int, iter.oobleft, ooblen);
+			ret = spinand_fill_oob(chip,
+					       ops->oobbuf + ops->oobretlen,
+					       oob_size, ops);
+			if (ret) {
+				dev_err(chip->dev, "Fill oob error %d\n", ret);
+				return ret;
+			}
+		}
+		if (ops->datbuf) {
+			size = min_t(int, iter.dataleft,
+				     nand_page_size(nand) - iter.pageoffs);
+			memcpy(chip->buf + iter.pageoffs,
+			       ops->datbuf + ops->retlen, size);
+		}
+		ret = spinand_do_write_page(mtd, iter.page, oob_only);
+		if (ret) {
+			dev_err(chip->dev, "error %d writing page 0x%x\n",
+				ret, iter.page);
+			return ret;
+		}
+		if (ops->datbuf)
+			ops->retlen += size;
+		if (ops->oobbuf)
+			ops->oobretlen += oob_size;
+	}
+
+	return ret;
+}
+
+/*
+ * spinand_do_write_ops - write data from buffer to device
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @ops: oob operations description structure
+ */
+static int spinand_do_write_ops(struct mtd_info *mtd, loff_t to,
+				struct mtd_oob_ops *ops)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret = 0;
+	bool ecc_off = ops->mode == MTD_OPS_RAW;
+
+	ret = nand_check_address(nand, to);
+	if (ret) {
+		dev_err(chip->dev, "%s: invalid write address\n", __func__);
+		return ret;
+	}
+	ret = nand_check_oob_ops(nand, to, ops);
+	if (ret) {
+		dev_err(chip->dev,
+			"%s: invalid oob operation input\n", __func__);
+		return ret;
+	}
+	if (nand_oob_ops_across_page(mtd_to_nand(mtd), ops)) {
+		dev_err(chip->dev,
+			"%s: try to across page when writing with OOB\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&chip->lock);
+	if (ecc_off)
+		spinand_disable_ecc(chip);
+	ret = spinand_write_pages(mtd, to, ops);
+	if (ecc_off)
+		spinand_enable_ecc(chip);
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+/*
+ * spinand_read - [MTD Interface] read page data
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @len: number of bytes to read
+ * @retlen: pointer to variable to store the number of read bytes
+ * @buf: the databuffer to put data
+ */
+static int spinand_read(struct mtd_info *mtd, loff_t from, size_t len,
+			size_t *retlen, u8 *buf)
+{
+	struct mtd_oob_ops ops;
+	int ret;
+
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ret = spinand_do_read_ops(mtd, from, &ops);
+	*retlen = ops.retlen;
+
+	return ret;
+}
+
+/*
+ * spinand_write - [MTD Interface] write page data
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @len: number of bytes to write
+ * @retlen: pointer to variable to store the number of written bytes
+ * @buf: the data to write
+ */
+static int spinand_write(struct mtd_info *mtd, loff_t to, size_t len,
+			 size_t *retlen, const u8 *buf)
+{
+	struct mtd_oob_ops ops;
+	int ret;
+
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = (uint8_t *)buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ret =  spinand_do_write_ops(mtd, to, &ops);
+	*retlen = ops.retlen;
+
+	return ret;
+}
+
+/*
+ * spinand_read_oob - [MTD Interface] read page data and/or out-of-band
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @ops: oob operation description structure
+ */
+static int spinand_read_oob(struct mtd_info *mtd, loff_t from,
+			    struct mtd_oob_ops *ops)
+{
+	int ret = -ENOTSUPP;
+
+	ops->retlen = 0;
+	switch (ops->mode) {
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
+	case MTD_OPS_RAW:
+		ret = spinand_do_read_ops(mtd, from, ops);
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * spinand_write_oob - [MTD Interface] write page data and/or out-of-band
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @ops: oob operation description structure
+ */
+static int spinand_write_oob(struct mtd_info *mtd, loff_t to,
+			     struct mtd_oob_ops *ops)
+{
+	int ret = -ENOTSUPP;
+
+	ops->retlen = 0;
+	switch (ops->mode) {
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
+	case MTD_OPS_RAW:
+		ret = spinand_do_write_ops(mtd, to, ops);
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * spinand_erase - [MTD Interface] erase block(s)
+ * @mtd: MTD device structure
+ * @einfo: erase instruction
+ */
+static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	loff_t offs = einfo->addr, len = einfo->len;
+	u8 status;
+	int ret;
+
+	ret = nand_check_erase_ops(nand, einfo);
+	if (ret) {
+		dev_err(chip->dev, "invalid erase operation input\n");
+		return ret;
+	}
+
+	mutex_lock(&chip->lock);
+	einfo->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+	einfo->state = MTD_ERASING;
+
+	while (len) {
+		spinand_write_enable(chip);
+		spinand_erase_block(chip, nand_offs_to_page(nand, offs));
+		ret = spinand_wait(chip, &status);
+		if (ret < 0) {
+			dev_err(chip->dev, "block erase command wait failed\n");
+			einfo->state = MTD_ERASE_FAILED;
+			goto erase_exit;
+		}
+		if ((status & STATUS_E_FAIL_MASK) == STATUS_E_FAIL) {
+			dev_err(chip->dev, "erase block 0x%012llx failed\n", offs);
+			einfo->state = MTD_ERASE_FAILED;
+			einfo->fail_addr = offs;
+			goto erase_exit;
+		}
+
+		/* Increment page address and decrement length */
+		len -= nand_eraseblock_size(nand);
+		offs += nand_eraseblock_size(nand);
+	}
+
+	einfo->state = MTD_ERASE_DONE;
+
+erase_exit:
+
+	ret = einfo->state == MTD_ERASE_DONE ? 0 : -EIO;
+
+	mutex_unlock(&chip->lock);
+
+	/* Do call back function */
+	if (!ret)
+		mtd_erase_callback(einfo);
+
+	return ret;
+}
+
+/*
  * spinand_set_rd_wr_op - choose the best read write command
  * @chip: SPI NAND device structure
  * Description:
@@ -364,6 +1109,11 @@ static int spinand_init(struct spinand_device *chip)
 	if (ret < 0)
 		ret = 0;
 	mtd->oobavail = ret;
+	mtd->_erase = spinand_erase;
+	mtd->_read = spinand_read;
+	mtd->_write = spinand_write;
+	mtd->_read_oob = spinand_read_oob;
+	mtd->_write_oob = spinand_write_oob;
 
 	if (!mtd->bitflip_threshold)
 		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3,
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 78ea9a6..5c8bb70 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -101,6 +101,8 @@ struct spinand_manufacturer_ops {
 	bool (*detect)(struct spinand_device *chip);
 	int (*init)(struct spinand_device *chip);
 	void (*cleanup)(struct spinand_device *chip);
+	void (*prepare_op)(struct spinand_device *chip, struct spinand_op *op,
+			   u32 page, u32 column);
 };
 
 struct spinand_manufacturer {
-- 
1.9.1

[PATCH v5 3/6] nand: spi: Add bad block support

[Peter Pan]

Add isbad and markbad support for SPI NAND. And do not
erase bad blocks in spi_nand_erase. BBT is also enabled
in this patch.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/spi/core.c | 329 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 322 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index eb7af9d..52113fc 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -19,6 +19,10 @@
 #include <linux/jiffies.h>
 #include <linux/mtd/spinand.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+
+static int spinand_erase_skip_bbt(struct mtd_info *mtd,
+				  struct erase_info *einfo);
 
 /*
  * spinand_exec_op - execute SPI NAND operation by controller ->exec_op() hook
@@ -878,11 +882,154 @@ static int spinand_write_oob(struct mtd_info *mtd, loff_t to,
 }
 
 /*
- * spinand_erase - [MTD Interface] erase block(s)
+ * spinand_block_bad - check if block at offset is bad by bad block marker
+ * @mtd: MTD device structure
+ * @offs: offset from device start
+ */
+static int spinand_block_bad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+	struct mtd_oob_ops ops = {0};
+	u32 block_addr;
+	u8 bad[2] = {0, 0};
+	u8 ret = 0;
+	unsigned int max_bitflips;
+
+	block_addr = nand_offs_to_eraseblock(nand, offs);
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ops.ooblen = 2;
+	ops.oobbuf = bad;
+	spinand_read_pages(mtd, nand_eraseblock_to_offs(nand, block_addr),
+			   &ops, &max_bitflips);
+	if (bad[0] != 0xFF || bad[1] != 0xFF)
+		ret =  1;
+
+	return ret;
+}
+
+/*
+ * spinand_block_checkbad - check if a block is marked bad
+ * @mtd: MTD device structure
+ * @offs: offset from device start
+ * @allowbbt: 1, if allowe to access the bbt area
+ * Description:
+ *   Check, if the block is bad. Either by reading the bad block table or
+ *   reading bad block marker.
+ */
+static int spinand_block_checkbad(struct mtd_info *mtd, loff_t offs,
+				  int allowbbt)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret;
+
+	if (nand_bbt_is_initialized(nand))
+		ret = nand_isbad_bbt(nand, offs, allowbbt);
+	else
+		ret = spinand_block_bad(mtd, offs);
+
+	return ret;
+}
+
+/*
+ * spinand_block_isbad - [MTD Interface] check if block at offset is bad
+ * @mtd: MTD device structure
+ * @offs: offset from device start
+ */
+static int spinand_block_isbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct spinand_device *chip = mtd_to_spinand(mtd);
+	int ret;
+
+	mutex_lock(&chip->lock);
+	ret = spinand_block_checkbad(mtd, offs, 0);
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+/*
+ * spinand_block_markbad_lowlevel - mark a block bad
+ * @mtd: MTD device structure
+ * @offs: offset from device start
+ *
+ * This function performs the generic bad block marking steps (i.e., bad
+ * block table(s) and/or marker(s)).
+ *
+ * We try operations in the following order:
+ *  (1) erase the affected block, to allow OOB marker to be written cleanly
+ *  (2) write bad block marker to OOB area of affected block (unless flag
+ *      NAND_BBT_NO_OOB_BBM is present)
+ *  (3) update the BBT
+ */
+static int spinand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+	struct mtd_oob_ops ops = {0};
+	struct erase_info einfo = {0};
+	u32 block_addr;
+	u8 buf[2] = {0, 0};
+	int res, ret = 0;
+
+	if (!nand_bbt_is_initialized(nand) ||
+	    !(nand->bbt.options & NAND_BBT_NO_OOB_BBM)) {
+		/*erase bad block before mark bad block*/
+		einfo.mtd = mtd;
+		einfo.addr = offs;
+		einfo.len = nand_eraseblock_size(nand);
+		spinand_erase_skip_bbt(mtd, &einfo);
+
+		block_addr = nand_offs_to_eraseblock(nand, offs);
+		ops.mode = MTD_OPS_PLACE_OOB;
+		ops.ooblen = 2;
+		ops.oobbuf = buf;
+		ret = spinand_do_write_ops(mtd,
+					   nand_eraseblock_to_offs(nand,
+								   block_addr),
+					   &ops);
+	}
+
+	/* Mark block bad in BBT */
+	if (nand_bbt_is_initialized(nand)) {
+		res = nand_markbad_bbt(nand, offs);
+		if (!ret)
+			ret = res;
+	}
+
+	if (!ret)
+		mtd->ecc_stats.badblocks++;
+
+	return ret;
+}
+
+/*
+ * spinand_block_markbad - [MTD Interface] mark block at the given offset
+ * as bad
+ * @mtd: MTD device structure
+ * @offs: offset relative to mtd start
+ */
+static int spinand_block_markbad(struct mtd_info *mtd, loff_t offs)
+{
+	int ret;
+
+	ret = spinand_block_isbad(mtd, offs);
+	if (ret) {
+		/* If it was bad already, return success and do nothing */
+		if (ret > 0)
+			return 0;
+		return ret;
+	}
+
+	return spinand_block_markbad_lowlevel(mtd, offs);
+}
+
+/*
+ * spinand_erase - erase block(s)
  * @mtd: MTD device structure
  * @einfo: erase instruction
+ * @allowbbt: allow to access bbt
  */
-static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
+static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo,
+			 int allowbbt)
 {
 	struct spinand_device *chip = mtd_to_spinand(mtd);
 	struct nand_device *nand = mtd_to_nand(mtd);
@@ -901,6 +1048,14 @@ static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
 	einfo->state = MTD_ERASING;
 
 	while (len) {
+		/* Check if we have a bad block, we do not erase bad blocks! */
+		if (spinand_block_checkbad(mtd, offs, allowbbt)) {
+			dev_warn(chip->dev,
+				"attempt to erase a bad block at 0x%012llx\n",
+				 offs);
+			einfo->state = MTD_ERASE_FAILED;
+			goto erase_exit;
+		}
 		spinand_write_enable(chip);
 		spinand_erase_block(chip, nand_offs_to_page(nand, offs));
 		ret = spinand_wait(chip, &status);
@@ -937,6 +1092,33 @@ static int spinand_erase(struct mtd_info *mtd, struct erase_info *einfo)
 }
 
 /*
+ * spinand_erase_skip_bbt - [MTD Interface] erase block(s) except BBT
+ * @mtd: MTD device structure
+ * @einfo: erase instruction
+ */
+static int spinand_erase_skip_bbt(struct mtd_info *mtd,
+				  struct erase_info *einfo)
+{
+	return spinand_erase(mtd, einfo, 0);
+}
+
+/*
+ * spinand_block_isreserved - [MTD Interface] check if a block is
+ * marked reserved.
+ * @mtd: MTD device structure
+ * @offs: offset from device start
+ */
+static int spinand_block_isreserved(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+
+	if (!nand_bbt_is_initialized(nand))
+		return 0;
+	/* Return info from the table */
+	return nand_isreserved_bbt(nand, offs);
+}
+
+/*
  * spinand_set_rd_wr_op - choose the best read write command
  * @chip: SPI NAND device structure
  * Description:
@@ -970,6 +1152,106 @@ static void spinand_set_rd_wr_op(struct spinand_device *chip)
 		chip->write_cache_op = SPINAND_CMD_PROG_LOAD;
 }
 
+/*
+ * spinand_erase_bbt - erase block(s) including BBT
+ * @nand: nand device structure
+ * @einfo: erase instruction
+ */
+static int spinand_erase_bbt(struct nand_device *nand,
+			     struct erase_info *einfo)
+{
+	return spinand_erase(nand_to_mtd(nand), einfo, 1);
+}
+
+/*
+ * spinand_erase_bbt - write bad block marker to certain block
+ * @nand: nand device structure
+ * @block: block to mark bad
+ */
+static int spinand_markbad(struct nand_device *nand, int block)
+{
+	struct mtd_oob_ops ops = {0};
+	u8 buf[2] = {0, 0};
+
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ops.ooboffs = 0;
+	ops.ooblen = 2;
+	ops.oobbuf = buf;
+
+	return spinand_do_write_ops(nand_to_mtd(nand),
+				    nand_eraseblock_to_offs(nand, block),
+				    &ops);
+}
+
+static const struct nand_ops spinand_ops = {
+	.erase = spinand_erase_bbt,
+	.markbad = spinand_markbad,
+};
+
+/*
+ * Define some generic bad/good block scan pattern which are used
+ * while scanning a device for factory marked good/bad blocks.
+ */
+static u8 scan_ff_pattern[] = { 0xff, 0xff };
+
+#define BADBLOCK_SCAN_MASK (~NAND_BBT_NO_OOB)
+
+/*
+ * spinand_create_badblock_pattern - creates a BBT descriptor structure
+ * @chip: SPI NAND device structure
+ *
+ * This function allocates and initializes a nand_bbt_descr for BBM detection.
+ * The new descriptor is stored in nand->bbt.bbp. Thus, nand->bbt.bbp should
+ * be NULL when passed to this function.
+ */
+static int spinand_create_badblock_pattern(struct spinand_device *chip)
+{
+	struct nand_device *nand = &chip->base;
+	struct nand_bbt_descr *bd;
+
+	if (nand->bbt.bbp) {
+		dev_err(chip->dev,
+			"Bad block pattern already allocated; not replacing\n");
+		return -EINVAL;
+	}
+	bd = devm_kzalloc(chip->dev, sizeof(*bd), GFP_KERNEL);
+	if (!bd)
+		return -ENOMEM;
+	bd->options = nand->bbt.options & BADBLOCK_SCAN_MASK;
+	bd->offs = 0;
+	bd->len = 2;
+	bd->pattern = scan_ff_pattern;
+	bd->options |= NAND_BBT_DYNAMICSTRUCT;
+	nand->bbt.bbp = bd;
+
+	return 0;
+}
+
+/*
+ * spinand_scan_bbt - scan BBT in SPI NAND device
+ * @chip: SPI NAND device structure
+ */
+static int spinand_scan_bbt(struct spinand_device *chip)
+{
+	struct nand_device *nand = &chip->base;
+	int ret;
+
+	/*
+	 * It's better to put BBT marker in-band, since some oob area
+	 * is not ecc protected by internal(on-die) ECC
+	 */
+	if (nand->bbt.options & NAND_BBT_USE_FLASH)
+		nand->bbt.options |= NAND_BBT_NO_OOB;
+	nand->bbt.td = NULL;
+	nand->bbt.md = NULL;
+
+	ret = spinand_create_badblock_pattern(chip);
+	if (ret)
+		return ret;
+
+	return nand_scan_bbt(nand);
+}
+
 static const struct spinand_manufacturer *spinand_manufacturers[] = {};
 
 /*
@@ -1024,14 +1306,30 @@ static void spinand_manufacturer_cleanup(struct spinand_device *chip)
 }
 
 /*
+ * TODO: move of_get_nand_on_flash_bbt() to generic NAND core
+ */
+static bool of_get_nand_on_flash_bbt(struct device_node *np)
+{
+	return of_property_read_bool(np, "nand-on-flash-bbt");
+}
+
+/*
  * spinand_dt_init - Initialize SPI NAND by device tree node
  * @chip: SPI NAND device structure
  *
- * TODO: put ecc_mode, ecc_strength, ecc_step, bbt, etc in here
- * and move it in generic NAND core.
+ * TODO: put ecc_mode, ecc_strength, ecc_step, etc in here and move
+ * it in generic NAND core.
  */
 static void spinand_dt_init(struct spinand_device *chip)
 {
+	struct nand_device *nand = &chip->base;
+	struct device_node *dn = nand_get_of_node(nand);
+
+	if (!dn)
+		return;
+
+	if (of_get_nand_on_flash_bbt(dn))
+		nand->bbt.options |= NAND_BBT_USE_FLASH;
 }
 
 /*
@@ -1085,13 +1383,14 @@ static int spinand_init(struct spinand_device *chip)
 				 GFP_KERNEL);
 	if (!chip->buf) {
 		ret = -ENOMEM;
-		goto err;
+		goto err1;
 	}
 
 	chip->oobbuf = chip->buf + nand_page_size(nand);
 
 	spinand_manufacturer_init(chip);
 
+	nand->ops = &spinand_ops;
 	mtd->name = chip->name;
 	mtd->size = nand_size(nand);
 	mtd->erasesize = nand_eraseblock_size(nand);
@@ -1109,11 +1408,14 @@ static int spinand_init(struct spinand_device *chip)
 	if (ret < 0)
 		ret = 0;
 	mtd->oobavail = ret;
-	mtd->_erase = spinand_erase;
+	mtd->_erase = spinand_erase_skip_bbt;
 	mtd->_read = spinand_read;
 	mtd->_write = spinand_write;
 	mtd->_read_oob = spinand_read_oob;
 	mtd->_write_oob = spinand_write_oob;
+	mtd->_block_isbad = spinand_block_isbad;
+	mtd->_block_markbad = spinand_block_markbad;
+	mtd->_block_isreserved = spinand_block_isreserved;
 
 	if (!mtd->bitflip_threshold)
 		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3,
@@ -1121,9 +1423,18 @@ static int spinand_init(struct spinand_device *chip)
 	/* After power up, all blocks are locked, so unlock it here. */
 	spinand_lock_block(chip, BL_ALL_UNLOCKED);
 
+	/* Build bad block table */
+	ret = spinand_scan_bbt(chip);
+	if (ret) {
+		dev_err(chip->dev, "Scan Bad Block Table failed.\n");
+		goto err2;
+	}
+
 	return nand_register(nand);
 
-err:
+err2:
+	devm_kfree(chip->dev, chip->buf);
+err1:
 	return ret;
 }
 
@@ -1191,10 +1502,14 @@ int spinand_register(struct spinand_device *chip)
 int spinand_unregister(struct spinand_device *chip)
 {
 	struct nand_device *nand = &chip->base;
+	struct nand_bbt_descr *bd = nand->bbt.bbp;
 
 	nand_unregister(nand);
 	spinand_manufacturer_cleanup(chip);
 	devm_kfree(chip->dev, chip->buf);
+	kfree(nand->bbt.bbt);
+	if (bd->options & NAND_BBT_DYNAMICSTRUCT)
+		devm_kfree(chip->dev, bd);
 
 	return 0;
 }
-- 
1.9.1

[PATCH v5 4/6] nand: spi: add Micron spi nand support

[Peter Pan]

This commit is to add support for Micron MT29F2G01ABAGD
spi nand chip.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/spi/Makefile |   1 +
 drivers/mtd/nand/spi/core.c   |   4 +-
 drivers/mtd/nand/spi/micron.c | 254 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |   2 +
 4 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/micron.c

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index a677a4d..df6c2ea 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_MTD_SPI_NAND) += core.o
+obj-$(CONFIG_MTD_SPI_NAND) += micron.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 52113fc..b2530fa 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -1252,7 +1252,9 @@ static int spinand_scan_bbt(struct spinand_device *chip)
 	return nand_scan_bbt(nand);
 }
 
-static const struct spinand_manufacturer *spinand_manufacturers[] = {};
+static const struct spinand_manufacturer *spinand_manufacturers[] = {
+	&micron_spinand_manufacture
+};
 
 /*
  * spinand_manufacturer_detect - detect SPI NAND device by each manufacturer
diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
new file mode 100644
index 0000000..f134194
--- /dev/null
+++ b/drivers/mtd/nand/spi/micron.c
@@ -0,0 +1,254 @@
+/*
+ *
+ * Copyright (c) 2009-2017 Micron Technology, Inc.
+ *
+ * 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/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_MICRON		0x2C
+
+#define SPI_NAND_M7XA_ECC_MASK		0x70
+#define SPI_NAND_M7XA_ECC_0_BIT		0x00
+#define SPI_NAND_M7XA_ECC_1_3_BIT	0x10
+#define SPI_NAND_M7XA_ECC_4_6_BIT	0x30
+#define SPI_NAND_M7XA_ECC_7_8_BIT	0x50
+#define SPI_NAND_M7XA_ECC_UNCORR	0x20
+
+struct micron_spinand_info {
+	char *name;
+	u8 dev_id;
+	u32 page_size;
+	u32 oob_size;
+	u32 pages_per_blk;
+	u32 blks_per_lun;
+	u32 luns_per_chip;
+	u32 ecc_strength;
+	u32 ecc_steps;
+	u32 rw_mode;
+	const struct mtd_ooblayout_ops *ooblayout_ops;
+	const struct spinand_ecc_engine_ops *ecc_engine_ops;
+};
+
+#define MICRON_SPI_NAND_INFO(nm, did, pagesz, oobsz, pg_per_blk,	\
+			     blk_per_lun, lun_per_chip, ecc_stren,	\
+			     ecc_stps, rwmode, ooblayoutops,		\
+			     ecc_ops)		\
+	{	\
+		.name = (nm), .dev_id = (did),		\
+		.page_size = (pagesz), .oob_size = (oobsz),		\
+		.pages_per_blk = (pg_per_blk),		\
+		.blks_per_lun = (blk_per_lun),		\
+		.luns_per_chip = (lun_per_chip),	\
+		.ecc_strength = (ecc_stren),	\
+		.ecc_steps = (ecc_stps),	\
+		.rw_mode = (rwmode),		\
+		.ooblayout_ops = (ooblayoutops),	\
+		.ecc_engine_ops = (ecc_ops)	\
+	}
+
+static int m7xa_ooblayout_ecc(struct mtd_info *mtd, int section,
+			      struct mtd_oob_region *oobregion)
+{
+	if (section)
+		return -ERANGE;
+
+	oobregion->length = 64;
+	oobregion->offset = 64;
+
+	return 0;
+}
+
+static int m7xa_ooblayout_free(struct mtd_info *mtd, int section,
+			       struct mtd_oob_region *oobregion)
+{
+	if (section)
+		return -ERANGE;
+
+	oobregion->length = 62;
+	oobregion->offset = 2;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops m7xa_ooblayout_ops = {
+	.ecc = m7xa_ooblayout_ecc,
+	.free = m7xa_ooblayout_free,
+};
+
+/*
+ * m7xa_get_ecc_status - get M7XA ecc correction info from status register
+ * @chip: SPI NAND device structure
+ * @status: status register value
+ * @corrected: corrected bit flip number
+ * @ecc_error: ecc correction error or not
+ */
+static void m7xa_get_ecc_status(struct spinand_device *chip,
+				unsigned int status, unsigned int *corrected,
+				unsigned int *ecc_error)
+{
+	unsigned int ecc_status = status & SPI_NAND_M7XA_ECC_MASK;
+
+	*ecc_error = (ecc_status == SPI_NAND_M7XA_ECC_UNCORR);
+	switch (ecc_status) {
+	case SPI_NAND_M7XA_ECC_0_BIT:
+		*corrected = 0;
+		break;
+	case SPI_NAND_M7XA_ECC_1_3_BIT:
+		*corrected = 3;
+		break;
+	case SPI_NAND_M7XA_ECC_4_6_BIT:
+		*corrected = 6;
+		break;
+	case SPI_NAND_M7XA_ECC_7_8_BIT:
+		*corrected = 8;
+		break;
+	}
+}
+
+static const struct spinand_ecc_engine_ops m7xa_ecc_engine_ops = {
+	.get_ecc_status = m7xa_get_ecc_status,
+};
+
+static const struct micron_spinand_info micron_spinand_table[] = {
+	MICRON_SPI_NAND_INFO("MT29F2G01ABAGD", 0x24, 2048, 128, 64, 2048, 1,
+			     8, 512, SPINAND_OP_COMMON, &m7xa_ooblayout_ops,
+			     &m7xa_ecc_engine_ops),
+};
+
+static int micron_spinand_get_dummy(struct spinand_device *chip,
+				    struct spinand_op *op)
+{
+	u8 opcode = op->cmd;
+
+	switch (opcode) {
+	case SPINAND_CMD_READ_FROM_CACHE:
+	case SPINAND_CMD_READ_FROM_CACHE_FAST:
+	case SPINAND_CMD_READ_FROM_CACHE_X2:
+	case SPINAND_CMD_READ_FROM_CACHE_DUAL_IO:
+	case SPINAND_CMD_READ_FROM_CACHE_X4:
+	case SPINAND_CMD_READ_ID:
+		return 1;
+	case SPINAND_CMD_READ_FROM_CACHE_QUAD_IO:
+		return 2;
+	default:
+		return 0;
+	}
+}
+
+/*
+ * micron_spinand_scan_id_table - scan chip info in id table
+ * @chip: SPI-NAND device structure
+ * @id: point to manufacture id and device id
+ * Description:
+ *   If found in id table, config chip with table information.
+ */
+static bool micron_spinand_scan_id_table(struct spinand_device *chip, u8 dev_id)
+{
+	struct mtd_info *mtd = spinand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	struct micron_spinand_info *item = NULL;
+	struct nand_memory_organization *memorg = &nand->memorg;
+	struct spinand_ecc_engine *ecc_engine = NULL;
+	int i = 0;
+
+	for (; i < ARRAY_SIZE(micron_spinand_table); i++) {
+		item = (struct micron_spinand_info *)micron_spinand_table + i;
+		if (dev_id != item->dev_id)
+			continue;
+		chip->name = item->name;
+		memorg->eraseblocksize = item->page_size * item->pages_per_blk;
+		memorg->pagesize = item->page_size;
+		memorg->oobsize = item->oob_size;
+		memorg->diesize = memorg->eraseblocksize * item->blks_per_lun;
+		memorg->ndies = item->luns_per_chip;
+		if (chip->ecc.type == SPINAND_ECC_ONDIE) {
+			ecc_engine = devm_kzalloc(chip->dev,
+						  sizeof(*ecc_engine),
+						  GFP_KERNEL);
+			if (!ecc_engine) {
+				dev_err(chip->dev,
+					"fail to allocate ecc engine.\n");
+				return false;
+			}
+			ecc_engine->ops = item->ecc_engine_ops;
+			ecc_engine->strength = item->ecc_strength;
+			ecc_engine->steps = item->ecc_steps;
+			mtd_set_ooblayout(mtd, item->ooblayout_ops);
+			chip->ecc.engine = ecc_engine;
+		}
+		chip->rw_mode = item->rw_mode;
+
+		return true;
+	}
+
+	return false;
+}
+
+/*
+ * micron_spinand_detect - initialize device related part in spinand_device
+ * struct if it is Micron device.
+ * @chip: SPI NAND device structure
+ */
+static bool micron_spinand_detect(struct spinand_device *chip)
+{
+	u8 *id = chip->id.data;
+
+	/*
+	 * Micron SPI NAND read ID need a dummy byte,
+	 * so the first byte in raw_id is dummy.
+	 */
+	if (id[1] != SPINAND_MFR_MICRON)
+		return false;
+
+	return micron_spinand_scan_id_table(chip, id[2]);
+}
+
+/*
+ * micron_spinand_cleanup -  free manufacutre related resources
+ * @chip: SPI NAND device structure
+ */
+static void micron_spinand_cleanup(struct spinand_device *chip)
+{
+	if (chip->ecc.type == SPINAND_ECC_ONDIE)
+		devm_kfree(chip->dev, chip->ecc.engine);
+}
+
+/*
+ * micron_spinand_prepare_op - Fix address for cache operation.
+ * @chip: SPI NAND device structure
+ * @op: pointer to spinand_op struct
+ * @page: page address
+ * @column: column address
+ */
+static void micron_spinand_prepare_op(struct spinand_device *chip,
+				      struct spinand_op *op, u32 page,
+				      u32 column)
+{
+	op->addr[0] |= (u8)((nand_page_to_eraseblock(&chip->base, page)
+			     & 0x1) << 4);
+	op->n_addr += micron_spinand_get_dummy(chip, op);
+}
+
+static const struct spinand_manufacturer_ops micron_spinand_manuf_ops = {
+	.detect = micron_spinand_detect,
+	.cleanup = micron_spinand_cleanup,
+	.prepare_op = micron_spinand_prepare_op,
+};
+
+const struct spinand_manufacturer micron_spinand_manufacture = {
+	.id = SPINAND_MFR_MICRON,
+	.name = "Micron",
+	.ops = &micron_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 5c8bb70..3c6dd48 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -111,6 +111,8 @@ struct spinand_manufacturer {
 	const struct spinand_manufacturer_ops *ops;
 };
 
+extern const struct spinand_manufacturer micron_spinand_manufacture;
+
 struct spinand_ecc_engine_ops {
 	void (*get_ecc_status)(struct spinand_device *chip,
 			       unsigned int status, unsigned int *corrected,
-- 
1.9.1

[PATCH v5 5/6] nand: spi: Add generic SPI controller support

[Peter Pan]

This commit supports to use generic spi controller
as spi nand controller.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/spi/Kconfig                   |   2 +
 drivers/mtd/nand/spi/Makefile                  |   1 +
 drivers/mtd/nand/spi/controllers/Kconfig       |   5 +
 drivers/mtd/nand/spi/controllers/Makefile      |   1 +
 drivers/mtd/nand/spi/controllers/generic-spi.c | 159 +++++++++++++++++++++++++
 5 files changed, 168 insertions(+)
 create mode 100644 drivers/mtd/nand/spi/controllers/Kconfig
 create mode 100644 drivers/mtd/nand/spi/controllers/Makefile
 create mode 100644 drivers/mtd/nand/spi/controllers/generic-spi.c

diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
index d77c46e..6bd1c65 100644
--- a/drivers/mtd/nand/spi/Kconfig
+++ b/drivers/mtd/nand/spi/Kconfig
@@ -3,3 +3,5 @@ menuconfig MTD_SPI_NAND
 	depends on MTD_NAND
 	help
 	  This is the framework for the SPI NAND device drivers.
+
+source "drivers/mtd/nand/spi/controllers/Kconfig"
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index df6c2ea..822e48d 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MTD_SPI_NAND) += core.o
 obj-$(CONFIG_MTD_SPI_NAND) += micron.o
+obj-$(CONFIG_MTD_SPI_NAND) += controllers/
diff --git a/drivers/mtd/nand/spi/controllers/Kconfig b/drivers/mtd/nand/spi/controllers/Kconfig
new file mode 100644
index 0000000..8ab7023
--- /dev/null
+++ b/drivers/mtd/nand/spi/controllers/Kconfig
@@ -0,0 +1,5 @@
+config GENERIC_SPI_NAND
+	tristate "SPI NAND with generic SPI bus Support"
+	depends on MTD_SPI_NAND && SPI
+	help
+	  This is to support SPI NAND device with generic SPI bus.
diff --git a/drivers/mtd/nand/spi/controllers/Makefile b/drivers/mtd/nand/spi/controllers/Makefile
new file mode 100644
index 0000000..46cbf29
--- /dev/null
+++ b/drivers/mtd/nand/spi/controllers/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_GENERIC_SPI_NAND) += generic-spi.o
diff --git a/drivers/mtd/nand/spi/controllers/generic-spi.c b/drivers/mtd/nand/spi/controllers/generic-spi.c
new file mode 100644
index 0000000..d354874
--- /dev/null
+++ b/drivers/mtd/nand/spi/controllers/generic-spi.c
@@ -0,0 +1,159 @@
+/*
+ * Copyright (c) 2009-2017 Micron Technology, Inc.
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/mtd/spinand.h>
+
+struct gen_spi_spinand_controller {
+	struct spinand_controller ctrl;
+	struct spi_device *spi;
+};
+
+#define to_gen_spi_spinand_controller(c) \
+	container_of(c, struct gen_spi_spinand_controller, ctrl)
+
+/*
+ * gen_spi_spinand_exec_op - to process a command to send to the
+ * SPI NAND by generic SPI bus
+ * @chip: SPI NAND device structure
+ * @op: SPI NAND operation descriptor
+ */
+static int gen_spi_spinand_exec_op(struct spinand_device *chip,
+				   struct spinand_op *op)
+{
+	struct spi_message message;
+	struct spi_transfer x[3];
+	struct spinand_controller *scontroller = chip->controller.controller;
+	struct gen_spi_spinand_controller *controller;
+
+	controller = to_gen_spi_spinand_controller(scontroller);
+	spi_message_init(&message);
+	memset(x, 0, sizeof(x));
+	x[0].len = 1;
+	x[0].tx_nbits = 1;
+	x[0].tx_buf = &op->cmd;
+	spi_message_add_tail(&x[0], &message);
+
+	if (op->n_addr + op->dummy_bytes) {
+		x[1].len = op->n_addr + op->dummy_bytes;
+		x[1].tx_nbits = op->addr_nbits;
+		x[1].tx_buf = op->addr;
+		spi_message_add_tail(&x[1], &message);
+	}
+	if (op->n_tx) {
+		x[2].len = op->n_tx;
+		x[2].tx_nbits = op->data_nbits;
+		x[2].tx_buf = op->tx_buf;
+		spi_message_add_tail(&x[2], &message);
+	} else if (op->n_rx) {
+		x[2].len = op->n_rx;
+		x[2].rx_nbits = op->data_nbits;
+		x[2].rx_buf = op->rx_buf;
+		spi_message_add_tail(&x[2], &message);
+	}
+	return spi_sync(controller->spi, &message);
+}
+
+static struct spinand_controller_ops gen_spi_spinand_ops = {
+	.exec_op = gen_spi_spinand_exec_op,
+};
+
+static int gen_spi_spinand_probe(struct spi_device *spi)
+{
+	struct spinand_device *chip;
+	struct gen_spi_spinand_controller *controller;
+	struct spinand_controller *spinand_controller;
+	struct device *dev = &spi->dev;
+	u16 mode = spi->mode;
+	int ret;
+
+	chip = spinand_alloc(dev);
+	if (IS_ERR(chip)) {
+		ret = PTR_ERR(chip);
+		goto err1;
+	}
+	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
+	if (!controller) {
+		ret = -ENOMEM;
+		goto err2;
+	}
+	controller->spi = spi;
+	spinand_controller = &controller->ctrl;
+	spinand_controller->ops = &gen_spi_spinand_ops;
+	spinand_controller->caps = SPINAND_CAP_RD_X1 | SPINAND_CAP_WR_X1;
+
+	if ((mode & SPI_RX_QUAD) && (mode & SPI_TX_QUAD))
+		spinand_controller->caps |= SPINAND_CAP_RD_QUAD;
+	if ((mode & SPI_RX_DUAL) && (mode & SPI_TX_DUAL))
+		spinand_controller->caps |= SPINAND_CAP_RD_DUAL;
+	if (mode & SPI_RX_QUAD)
+		spinand_controller->caps |= SPINAND_CAP_RD_X4;
+	if (mode & SPI_RX_DUAL)
+		spinand_controller->caps |= SPINAND_CAP_RD_X2;
+	if (mode & SPI_TX_QUAD)
+		spinand_controller->caps |= SPINAND_CAP_WR_QUAD |
+					    SPINAND_CAP_WR_X4;
+	if (mode & SPI_TX_DUAL)
+		spinand_controller->caps |= SPINAND_CAP_WR_DUAL |
+					    SPINAND_CAP_WR_X2;
+	chip->controller.controller = spinand_controller;
+	/*
+	 * generic spi controller doesn't have ecc capability,
+	 * so use on-die ecc.
+	 */
+	chip->ecc.type = SPINAND_ECC_ONDIE;
+	spi_set_drvdata(spi, chip);
+
+	ret = spinand_register(chip);
+	if (ret)
+		goto err3;
+
+	return 0;
+
+err3:
+	devm_kfree(dev, controller);
+err2:
+	spinand_free(chip);
+err1:
+	return ret;
+}
+
+static int gen_spi_spinand_remove(struct spi_device *spi)
+{
+	struct spinand_device *chip = spi_get_drvdata(spi);
+	struct spinand_controller *scontroller = chip->controller.controller;
+	struct gen_spi_spinand_controller *controller;
+
+	spinand_unregister(chip);
+	controller = to_gen_spi_spinand_controller(scontroller);
+	devm_kfree(&spi->dev, controller);
+	spinand_free(chip);
+
+	return 0;
+}
+
+static struct spi_driver gen_spi_spinand_driver = {
+	.driver = {
+		.name	= "generic_spinand",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= gen_spi_spinand_probe,
+	.remove	= gen_spi_spinand_remove,
+};
+module_spi_driver(gen_spi_spinand_driver);
+
+MODULE_DESCRIPTION("Generic SPI controller to support SPI NAND");
+MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v5 6/6] MAINTAINERS: Add SPI NAND entry

[Peter Pan]

Add maintainer information for SPI NAND.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cc2139d..5ba6c7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8610,6 +8610,15 @@ S:	Maintained
 F:	drivers/mtd/nand/
 F:	include/linux/mtd/*nand*.h
 
+SPI NAND FLASH DRIVER
+M:	Peter Pan <peterpandong@micron.com>
+L:	linux-mtd@lists.infradead.org
+W:	http://www.linux-mtd.infradead.org/
+Q:	http://patchwork.ozlabs.org/project/linux-mtd/list/
+S:	Maintained
+F:	drivers/mtd/nand/spi/
+F:	include/linux/mtd/spinand.h
+
 NATSEMI ETHERNET DRIVER (DP8381x)
 S:	Orphan
 F:	drivers/net/ethernet/natsemi/natsemi.c
-- 
1.9.1

Re: [PATCH v5 1/6] nand: spi: add basic blocks for infrastructure

[Boris Brezillon]

Hi Peter,

On Mon, 10 Apr 2017 15:51:48 +0800
Peter Pan <peterpandong@micron.com> wrote:

> +
> +/*
> + * spinand_free - [SPI NAND Interface] free SPI NAND device instance
> + * @chip: SPI NAND device structure
> + */
> +void spinand_free(struct spinand_device *chip)
> +{
> +	devm_kfree(chip->dev, chip);

This is unneeded. Everything allocated with devm_kmalloc() should be
automatically freed when the underlying device is released.

> +}
> +EXPORT_SYMBOL_GPL(spinand_free);
> +
> +/*
> + * spinand_register - [SPI NAND Interface] register SPI NAND device
> + * @chip: SPI NAND device structure
> + */
> +int spinand_register(struct spinand_device *chip)
> +{
> +	int ret;
> +
> +	ret = spinand_detect(chip);
> +	if (ret) {
> +		dev_err(chip->dev,
> +			"Detect SPI NAND failed with error %d.\n", ret);
> +		return ret;
> +	}
> +
> +	ret = spinand_init(chip);
> +	if (ret)
> +		dev_err(chip->dev,
> +			"Init SPI NAND failed with error %d.\n", ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(spinand_register);
> +
> +/*
> + * spinand_unregister - [SPI NAND Interface] unregister SPI NAND device
> + * @chip: SPI NAND device structure
> + */
> +int spinand_unregister(struct spinand_device *chip)
> +{
> +	struct nand_device *nand = &chip->base;
> +
> +	nand_unregister(nand);

I realize nand_unregister() is not propagating the error returned by
mtd_device_unregister(), which is wrong. Can you fix that and make sure
you propagate the error here?

> +	spinand_manufacturer_cleanup(chip);
> +	devm_kfree(chip->dev, chip->buf);

Why calling devm_kfree() on something that will anyway be freed
automatically.

BTW, you should not allocate the buffer with devm_kzalloc(), just in
case some drivers want to use it for DMA accesses (see [1] for a
better explanation).

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(spinand_unregister);

Sorry, I didn't have time to properly review your v4, and I realize
I'm not happy with these new spinand_register/unregister() functions.

I initially suggested to have a model where you register a SPI NAND
controller (spinand_controller_register()) and the core takes care of
populating the devices connected to this controller for you, I don't
think I suggested to add the spinand_register/unregister() helpers.
As initially replied to Arnaud, the introduction of
spinand_controller_register() is not a hard requirement, so let's keep
that for later.

Back to your spinand_register/unregister() functions. The main problem
I see is the fact that these functions are asymmetric:
spinand_unregister() path is calling nand_unregister() while
spinand_register() is not calling nand_register(). This kind of
asymmetry is disturbing and usually leads to bugs in drivers.

This leaves 2 solutions:

1/ only expose spinand_init/cleanup() functions (spinand_init() should
   call spinand_detect() in this case) and let the driver call
   mtd_device_register/unregister() manually
2/ call mtd_device_register() from spinand_register() even if this
   implies hardcoding advanced parameters like the default partitions
   of the part probe types

#1 is probably safer for now since SPI NAND controller drivers might
want to tweak the config set in spinand_init() (for example to pass
controller side ECC engine ops).

I still need to review the rest of the series carefully, so please don't
send a new version until this is done. Just ping me if you don't have
any news from me after 2 weeks ;).

Regards,

Boris

[1]https://lkml.org/lkml/2017/3/30/168

Re: [PATCH v5 1/6] nand: spi: add basic blocks for infrastructure

[Marek Vasut]

On 04/10/2017 09:51 AM, Peter Pan wrote:
> This is the first commit for spi nand framkework.
> This commit is to add add basic building blocks
> for the SPI NAND infrastructure.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>

[...]

> +/* SPI NAND supported OP mode */
> +#define SPINAND_RD_X1		0x00000001

You might want to use BIT() here too.

btw SPI NOR seems to have something similar coming up, see
[PATCH v5 0/6] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories

> +#define SPINAND_RD_X2		0x00000002
> +#define SPINAND_RD_X4		0x00000004
> +#define SPINAND_RD_DUAL		0x00000008
> +#define SPINAND_RD_QUAD		0x00000010
> +#define SPINAND_WR_X1		0x00000020
> +#define SPINAND_WR_X2		0x00000040
> +#define SPINAND_WR_X4		0x00000080
> +#define SPINAND_WR_DUAL		0x00000100
> +#define SPINAND_WR_QUAD		0x00000200
> +
> +#define SPINAND_RD_COMMON	(SPINAND_RD_X1 | SPINAND_RD_X2 | \
> +				 SPINAND_RD_X4 | SPINAND_RD_DUAL | \
> +				 SPINAND_RD_QUAD)
> +#define SPINAND_WR_COMMON	(SPINAND_WR_X1 | SPINAND_WR_X4)
> +#define SPINAND_OP_COMMON	(SPINAND_RD_COMMON | SPINAND_WR_COMMON)
> +
> +struct spinand_device *spinand_alloc(struct device *dev);
> +void spinand_free(struct spinand_device *chip);
> +int spinand_register(struct spinand_device *chip);
> +int spinand_unregister(struct spinand_device *chip);
> +#endif /* __LINUX_MTD_SPINAND_H */
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v5 4/6] nand: spi: add Micron spi nand support

[Marek Vasut]

On 04/10/2017 09:51 AM, Peter Pan wrote:
> This commit is to add support for Micron MT29F2G01ABAGD
> spi nand chip.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  drivers/mtd/nand/spi/Makefile |   1 +
>  drivers/mtd/nand/spi/core.c   |   4 +-
>  drivers/mtd/nand/spi/micron.c | 254 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h   |   2 +
>  4 files changed, 260 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mtd/nand/spi/micron.c
> 
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index a677a4d..df6c2ea 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_MTD_SPI_NAND) += core.o
> +obj-$(CONFIG_MTD_SPI_NAND) += micron.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 52113fc..b2530fa 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1252,7 +1252,9 @@ static int spinand_scan_bbt(struct spinand_device *chip)
>  	return nand_scan_bbt(nand);
>  }
>  
> -static const struct spinand_manufacturer *spinand_manufacturers[] = {};
> +static const struct spinand_manufacturer *spinand_manufacturers[] = {
> +	&micron_spinand_manufacture
> +};
>  
>  /*
>   * spinand_manufacturer_detect - detect SPI NAND device by each manufacturer
> diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> new file mode 100644
> index 0000000..f134194
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/micron.c
> @@ -0,0 +1,254 @@
> +/*
> + *
> + * Copyright (c) 2009-2017 Micron Technology, Inc.
> + *
> + * 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/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_MICRON		0x2C
> +
> +#define SPI_NAND_M7XA_ECC_MASK		0x70
> +#define SPI_NAND_M7XA_ECC_0_BIT		0x00
> +#define SPI_NAND_M7XA_ECC_1_3_BIT	0x10
> +#define SPI_NAND_M7XA_ECC_4_6_BIT	0x30
> +#define SPI_NAND_M7XA_ECC_7_8_BIT	0x50
> +#define SPI_NAND_M7XA_ECC_UNCORR	0x20
> +
> +struct micron_spinand_info {
> +	char *name;
> +	u8 dev_id;
> +	u32 page_size;
> +	u32 oob_size;
> +	u32 pages_per_blk;
> +	u32 blks_per_lun;
> +	u32 luns_per_chip;
> +	u32 ecc_strength;
> +	u32 ecc_steps;
> +	u32 rw_mode;
> +	const struct mtd_ooblayout_ops *ooblayout_ops;
> +	const struct spinand_ecc_engine_ops *ecc_engine_ops;
> +};
> +
> +#define MICRON_SPI_NAND_INFO(nm, did, pagesz, oobsz, pg_per_blk,	\
> +			     blk_per_lun, lun_per_chip, ecc_stren,	\
> +			     ecc_stps, rwmode, ooblayoutops,		\
> +			     ecc_ops)		\
> +	{	\
> +		.name = (nm), .dev_id = (did),		\
> +		.page_size = (pagesz), .oob_size = (oobsz),		\

Nit: Keep each entry on a separate line please.

> +		.pages_per_blk = (pg_per_blk),		\
> +		.blks_per_lun = (blk_per_lun),		\
> +		.luns_per_chip = (lun_per_chip),	\
> +		.ecc_strength = (ecc_stren),	\
> +		.ecc_steps = (ecc_stps),	\
> +		.rw_mode = (rwmode),		\
> +		.ooblayout_ops = (ooblayoutops),	\
> +		.ecc_engine_ops = (ecc_ops)	\
> +	}

[...]

-- 
Best regards,
Marek Vasut

Re: [PATCH v5 5/6] nand: spi: Add generic SPI controller support

[Marek Vasut]

On 04/10/2017 09:51 AM, Peter Pan wrote:
> This commit supports to use generic spi controller
> as spi nand controller.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  drivers/mtd/nand/spi/Kconfig                   |   2 +
>  drivers/mtd/nand/spi/Makefile                  |   1 +
>  drivers/mtd/nand/spi/controllers/Kconfig       |   5 +
>  drivers/mtd/nand/spi/controllers/Makefile      |   1 +
>  drivers/mtd/nand/spi/controllers/generic-spi.c | 159 +++++++++++++++++++++++++
>  5 files changed, 168 insertions(+)
>  create mode 100644 drivers/mtd/nand/spi/controllers/Kconfig
>  create mode 100644 drivers/mtd/nand/spi/controllers/Makefile
>  create mode 100644 drivers/mtd/nand/spi/controllers/generic-spi.c

[...]

> +static int gen_spi_spinand_probe(struct spi_device *spi)
> +{
> +	struct spinand_device *chip;
> +	struct gen_spi_spinand_controller *controller;
> +	struct spinand_controller *spinand_controller;
> +	struct device *dev = &spi->dev;
> +	u16 mode = spi->mode;
> +	int ret;
> +
> +	chip = spinand_alloc(dev);
> +	if (IS_ERR(chip)) {
> +		ret = PTR_ERR(chip);
> +		goto err1;
> +	}

Would it make sense to embed the struct spinand_device into struct
gen_spi_nand_controller , so you'd get rid of one allocation ?

> +	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> +	if (!controller) {
> +		ret = -ENOMEM;
> +		goto err2;
> +	}
> +	controller->spi = spi;
> +	spinand_controller = &controller->ctrl;
> +	spinand_controller->ops = &gen_spi_spinand_ops;
> +	spinand_controller->caps = SPINAND_CAP_RD_X1 | SPINAND_CAP_WR_X1;
> +
> +	if ((mode & SPI_RX_QUAD) && (mode & SPI_TX_QUAD))
> +		spinand_controller->caps |= SPINAND_CAP_RD_QUAD;
> +	if ((mode & SPI_RX_DUAL) && (mode & SPI_TX_DUAL))
> +		spinand_controller->caps |= SPINAND_CAP_RD_DUAL;
> +	if (mode & SPI_RX_QUAD)
> +		spinand_controller->caps |= SPINAND_CAP_RD_X4;
> +	if (mode & SPI_RX_DUAL)
> +		spinand_controller->caps |= SPINAND_CAP_RD_X2;
> +	if (mode & SPI_TX_QUAD)
> +		spinand_controller->caps |= SPINAND_CAP_WR_QUAD |
> +					    SPINAND_CAP_WR_X4;
> +	if (mode & SPI_TX_DUAL)
> +		spinand_controller->caps |= SPINAND_CAP_WR_DUAL |
> +					    SPINAND_CAP_WR_X2;
> +	chip->controller.controller = spinand_controller;
> +	/*
> +	 * generic spi controller doesn't have ecc capability,
> +	 * so use on-die ecc.
> +	 */
> +	chip->ecc.type = SPINAND_ECC_ONDIE;
> +	spi_set_drvdata(spi, chip);
> +
> +	ret = spinand_register(chip);
> +	if (ret)
> +		goto err3;
> +
> +	return 0;
> +
> +err3:
> +	devm_kfree(dev, controller);
> +err2:
> +	spinand_free(chip);
> +err1:
> +	return ret;
> +}
> +
> +static int gen_spi_spinand_remove(struct spi_device *spi)
> +{
> +	struct spinand_device *chip = spi_get_drvdata(spi);
> +	struct spinand_controller *scontroller = chip->controller.controller;
> +	struct gen_spi_spinand_controller *controller;
> +
> +	spinand_unregister(chip);
> +	controller = to_gen_spi_spinand_controller(scontroller);
> +	devm_kfree(&spi->dev, controller);
> +	spinand_free(chip);
> +
> +	return 0;
> +}
> +
> +static struct spi_driver gen_spi_spinand_driver = {
> +	.driver = {
> +		.name	= "generic_spinand",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= gen_spi_spinand_probe,
> +	.remove	= gen_spi_spinand_remove,
> +};
> +module_spi_driver(gen_spi_spinand_driver);
> +
> +MODULE_DESCRIPTION("Generic SPI controller to support SPI NAND");
> +MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
> +MODULE_LICENSE("GPL v2");
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v5 1/6] nand: spi: add basic blocks for infrastructure

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:48 +0800
Peter Pan <peterpandong@micron.com> wrote:

> +/*
> + * spinand_dt_init - Initialize SPI NAND by device tree node
> + * @chip: SPI NAND device structure
> + *
> + * TODO: put ecc_mode, ecc_strength, ecc_step, bbt, etc in here
> + * and move it in generic NAND core.
> + */
> +static void spinand_dt_init(struct spinand_device *chip)
> +{
> +}

Please drop this function until you really have something to put in
there.

> +
> +/*
> + * spinand_detect - detect the SPI NAND device
> + * @chip: SPI NAND device structure
> + */
> +static int spinand_detect(struct spinand_device *chip)
> +{
> +	struct nand_device *nand = &chip->base;
> +	int ret;
> +
> +	spinand_reset(chip);
> +	spinand_read_id(chip, chip->id.data);
> +	chip->id.len = SPINAND_MAX_ID_LEN;
> +
> +	ret = spinand_manufacturer_detect(chip);
> +	if (ret) {
> +		dev_err(chip->dev, "unknown raw ID %*phN\n",
> +			SPINAND_MAX_ID_LEN, chip->id.data);
> +		goto out;
> +	}
> +
> +	dev_info(chip->dev, "%s (%s) is found.\n", chip->name,
> +		 chip->manufacturer.manu->name);
> +	dev_info(chip->dev,
> +		 "%d MiB, block size: %d KiB, page size: %d, OOB size: %d\n",
> +		 (int)(nand_size(nand) >> 20), nand_eraseblock_size(nand) >> 10,
> +		 nand_page_size(nand), nand_per_page_oobsize(nand));
> +
> +out:
> +	return ret;
> +}
> +
> +/*
> + * spinand_init - initialize the SPI NAND device
> + * @chip: SPI NAND device structure
> + */
> +static int spinand_init(struct spinand_device *chip)
> +{
> +	struct mtd_info *mtd = spinand_to_mtd(chip);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	struct spinand_ecc_engine *ecc_engine;
> +	int ret;
> +
> +	spinand_dt_init(chip);
> +	spinand_set_rd_wr_op(chip);
> +
> +	chip->buf = devm_kzalloc(chip->dev,
> +				 nand_page_size(nand) +
> +				 nand_per_page_oobsize(nand),
> +				 GFP_KERNEL);
> +	if (!chip->buf) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	chip->oobbuf = chip->buf + nand_page_size(nand);
> +
> +	spinand_manufacturer_init(chip);
> +
> +	mtd->name = chip->name;
> +	mtd->size = nand_size(nand);
> +	mtd->erasesize = nand_eraseblock_size(nand);
> +	mtd->writesize = nand_page_size(nand);
> +	mtd->writebufsize = mtd->writesize;
> +	mtd->owner = THIS_MODULE;
> +	mtd->type = MTD_NANDFLASH;
> +	mtd->flags = MTD_CAP_NANDFLASH;
> +	if (!mtd->ecc_strength)
> +		mtd->ecc_strength = ecc_engine->strength ?
> +				    ecc_engine->strength : 1;

Why 1 if the engine strength is 0?

> +
> +	mtd->oobsize = nand_per_page_oobsize(nand);
> +	ret = mtd_ooblayout_count_freebytes(mtd);
> +	if (ret < 0)
> +		ret = 0;
> +	mtd->oobavail = ret;
> +
> +	if (!mtd->bitflip_threshold)
> +		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3,
> +						      4);
> +	/* After power up, all blocks are locked, so unlock it here. */
> +	spinand_lock_block(chip, BL_ALL_UNLOCKED);
> +
> +	return nand_register(nand);
> +
> +err:
> +	return ret;
> +}
> +

[...]

> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> new file mode 100644
> index 0000000..78ea9a6
> --- /dev/null
> +++ b/include/linux/mtd/spinand.h
> @@ -0,0 +1,257 @@

[...]

> +
> +struct spinand_controller_ops {
> +	int (*exec_op)(struct spinand_device *chip,
> +		       struct spinand_op *op);
> +};
> +
> +struct spinand_manufacturer_ops {
> +	/*
> +	 * Firstly, ->detect() should not be NULL.
> +	 * ->detect() implementation for manufacturer A never sends
> +	 * any manufacturer specific SPI command to a SPI NAND from
> +	 * manufacturer B, so the proper way is to decode the raw id
> +	 * data in chip->id.data first, if manufacture ID dismatch,
> +	 * return directly and let others to detect.
> +	 */
> +	bool (*detect)(struct spinand_device *chip);
> +	int (*init)(struct spinand_device *chip);
> +	void (*cleanup)(struct spinand_device *chip);
> +};
> +
> +struct spinand_manufacturer {
> +	u8 id;
> +	char *name;
> +	const struct spinand_manufacturer_ops *ops;
> +};
> +
> +struct spinand_ecc_engine_ops {
> +	void (*get_ecc_status)(struct spinand_device *chip,
> +			       unsigned int status, unsigned int *corrected,
> +			       unsigned int *ecc_errors);
> +	void (*disable_ecc)(struct spinand_device *chip);
> +	void (*enable_ecc)(struct spinand_device *chip);
> +};
> +
> +enum spinand_ecc_type {
> +	SPINAND_ECC_ONDIE,
> +	SPINAND_ECC_HW,
> +};
> +
> +struct spinand_ecc_engine {
> +	u32 strength;
> +	u32 steps;
> +	const struct spinand_ecc_engine_ops *ops;
> +};
> +
> +#define SPINAND_CAP_RD_X1	BIT(0)
> +#define SPINAND_CAP_RD_X2	BIT(1)
> +#define SPINAND_CAP_RD_X4	BIT(2)
> +#define SPINAND_CAP_RD_DUAL	BIT(3)
> +#define SPINAND_CAP_RD_QUAD	BIT(4)
> +#define SPINAND_CAP_WR_X1	BIT(5)
> +#define SPINAND_CAP_WR_X2	BIT(6)
> +#define SPINAND_CAP_WR_X4	BIT(7)
> +#define SPINAND_CAP_WR_DUAL	BIT(8)
> +#define SPINAND_CAP_WR_QUAD	BIT(9)
> +#define SPINAND_CAP_HW_ECC	BIT(10)
> +
> +struct spinand_controller {
> +	struct spinand_controller_ops *ops;
> +	u32 caps;
> +};

Can you please document all structures using kernel-doc headers?

Re: [PATCH v5 1/6] nand: spi: add basic blocks for infrastructure

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:48 +0800
Peter Pan <peterpandong@micron.com> wrote:

> +
> +struct spinand_ecc_engine_ops {
> +	void (*get_ecc_status)(struct spinand_device *chip,
> +			       unsigned int status, unsigned int *corrected,
> +			       unsigned int *ecc_errors);

No need to specify ecc, we're interacting with an ECC engine, so I
guess it's already assumed.

	void (*get_stats)(struct spinand_device *chip,
			  unsigned int status,
			  unsigned int *corrected,
			  unsigned int *errors);

BTW, we probably need max_bitflips, unless corrected already contains
this information, in which case it should probably be clarified (better
name + kernel doc).

> +	void (*disable_ecc)(struct spinand_device *chip);
> +	void (*enable_ecc)(struct spinand_device *chip);

Ditto, just enable()/disable().

> +};
> +
> +enum spinand_ecc_type {
> +	SPINAND_ECC_ONDIE,
> +	SPINAND_ECC_HW,
> +};

Probably something we should extract from rawnand.h and move to nand.h
so that we can share the same definition (until we also share the ECC
engine interface).

> +
> +struct spinand_ecc_engine {
> +	u32 strength;
> +	u32 steps;
> +	const struct spinand_ecc_engine_ops *ops;
> +};

The same ECC engine engine can possibly be used with different NAND
devices. So what you're describing here is more an ECC engine user than
the ECC engine itself.

Ideally we should have:

struct nand_ecc_engine {
	/* Some info describing engine caps. */
	const struct spinand_ecc_engine_ops *ops;
};

struct nand_ecc_engine_user {
	u32 strength;
	u32 steps;
	struct nand_ecc_engine *engine;
};

Anyway, I think we can keep that for later.

BTW, would you mind keeping all that is ECC related outside of this
initial series, or at least, add it in separate patches?

Re: [PATCH v5 2/6] nand: spi: add basic operations support

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:49 +0800
Peter Pan <peterpandong@micron.com> wrote:

>  
>  /*
> + * spinand_get_ecc_status - get ecc correction information from status register
> + * @chip: SPI NAND device structure
> + * @status: status register value
> + * @corrected: corrected bit flip number
> + * @ecc_error: ecc correction error or not
> + */
> +static void spinand_get_ecc_status(struct spinand_device *chip,
> +				   unsigned int status,
> +				   unsigned int *corrected,
> +				   unsigned int *ecc_error)
> +{
> +	return chip->ecc.engine->ops->get_ecc_status(chip, status, corrected,
> +						     ecc_error);
> +}
> +
> +/*
> + * spinand_do_read_page - read page from device to buffer
> + * @mtd: MTD device structure
> + * @page_addr: page address/raw address
> + * @ecc_off: without ecc or not
> + * @corrected: how many bit flip corrected
> + * @oob_only: read OOB only or the whole page
> + */
> +static int spinand_do_read_page(struct mtd_info *mtd, u32 page_addr,
> +				bool ecc_off, int *corrected, bool oob_only)
> +{
> +	struct spinand_device *chip = mtd_to_spinand(mtd);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	int ret, ecc_error = 0;
> +	u8 status;
> +
> +	spinand_read_page_to_cache(chip, page_addr);
> +	ret = spinand_wait(chip, &status);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "error %d waiting page 0x%x to cache\n",
> +			ret, page_addr);
> +		return ret;
> +	}
> +	if (!oob_only)
> +		spinand_read_from_cache(chip, page_addr, 0,
> +					nand_page_size(nand) +
> +					nand_per_page_oobsize(nand),
> +					chip->buf);
> +	else
> +		spinand_read_from_cache(chip, page_addr, nand_page_size(nand),
> +					nand_per_page_oobsize(nand),
> +					chip->oobbuf);
> +	if (!ecc_off) {

I see at least one problem for software ECC, or hardware ECC engine
that are not deeply integrated in the NAND controller pipeline (those
only generating ECC bytes without pushing them to the NAND).

You probably need a hook to let ECC engines correct the in-band and OOB
buffers (eccengine->correct() ?).

> +		spinand_get_ecc_status(chip, status, corrected, &ecc_error);
> +		/*
> +		 * If there's an ECC error, print a message and notify MTD
> +		 * about it. Then complete the read, to load actual data on
> +		 * the buffer (instead of the status result).
> +		 */
> +		if (ecc_error) {
> +			dev_err(chip->dev,
> +				"internal ECC error reading page 0x%x\n",
> +				page_addr);
> +			mtd->ecc_stats.failed++;
> +		} else if (*corrected) {
> +			mtd->ecc_stats.corrected += *corrected;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * spinand_do_write_page - write data from buffer to device
> + * @mtd: MTD device structure
> + * @page_addr: page address/raw address
> + * @oob_only: write OOB only or the whole page
> + */
> +static int spinand_do_write_page(struct mtd_info *mtd, u32 page_addr,
> +				 bool oob_only)
> +{
> +	struct spinand_device *chip = mtd_to_spinand(mtd);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	u8 status;
> +	int ret = 0;
> +

Same as for the read path: you'll probably need and extra
eccgine->generate_ecc_bytes() hook to let the ECC generates ECC bytes
if required.

> +	spinand_write_enable(chip);
> +	if (!oob_only)
> +		spinand_write_to_cache(chip, page_addr, 0,
> +				       nand_page_size(nand) +
> +				       nand_per_page_oobsize(nand), chip->buf);
> +	else
> +		spinand_write_to_cache(chip, page_addr, nand_page_size(nand),
> +				       nand_per_page_oobsize(nand),
> +				       chip->oobbuf);
> +	spinand_program_execute(chip, page_addr);
> +	ret = spinand_wait(chip, &status);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "error %d reading page 0x%x from cache\n",
> +			ret, page_addr);
> +		return ret;
> +	}
> +	if ((status & STATUS_P_FAIL_MASK) == STATUS_P_FAIL) {
> +		dev_err(chip->dev, "program page 0x%x failed\n", page_addr);
> +		ret = -EIO;
> +	}
> +	return ret;
> +}
> +

Re: [PATCH v5 3/6] nand: spi: Add bad block support

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:50 +0800
Peter Pan <peterpandong@micron.com> wrote:

> +
> +/*
> + * spinand_scan_bbt - scan BBT in SPI NAND device
> + * @chip: SPI NAND device structure
> + */
> +static int spinand_scan_bbt(struct spinand_device *chip)
> +{
> +	struct nand_device *nand = &chip->base;
> +	int ret;
> +
> +	/*
> +	 * It's better to put BBT marker in-band, since some oob area
> +	 * is not ecc protected by internal(on-die) ECC
> +	 */
> +	if (nand->bbt.options & NAND_BBT_USE_FLASH)
> +		nand->bbt.options |= NAND_BBT_NO_OOB;

Hm, really? Do we want to force NAND_BBT_NO_OOB for everyone?

> +	nand->bbt.td = NULL;
> +	nand->bbt.md = NULL;
> +
> +	ret = spinand_create_badblock_pattern(chip);
> +	if (ret)
> +		return ret;
> +
> +	return nand_scan_bbt(nand);
> +}
> +

[...]
  
>  /*
> @@ -1085,13 +1383,14 @@ static int spinand_init(struct spinand_device *chip)
>  				 GFP_KERNEL);
>  	if (!chip->buf) {
>  		ret = -ENOMEM;
> -		goto err;
> +		goto err1;
>  	}
>  
>  	chip->oobbuf = chip->buf + nand_page_size(nand);
>  
>  	spinand_manufacturer_init(chip);
>  
> +	nand->ops = &spinand_ops;
>  	mtd->name = chip->name;
>  	mtd->size = nand_size(nand);
>  	mtd->erasesize = nand_eraseblock_size(nand);
> @@ -1109,11 +1408,14 @@ static int spinand_init(struct spinand_device *chip)
>  	if (ret < 0)
>  		ret = 0;
>  	mtd->oobavail = ret;
> -	mtd->_erase = spinand_erase;
> +	mtd->_erase = spinand_erase_skip_bbt;
>  	mtd->_read = spinand_read;
>  	mtd->_write = spinand_write;
>  	mtd->_read_oob = spinand_read_oob;
>  	mtd->_write_oob = spinand_write_oob;
> +	mtd->_block_isbad = spinand_block_isbad;
> +	mtd->_block_markbad = spinand_block_markbad;
> +	mtd->_block_isreserved = spinand_block_isreserved;
>  
>  	if (!mtd->bitflip_threshold)
>  		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3,
> @@ -1121,9 +1423,18 @@ static int spinand_init(struct spinand_device *chip)
>  	/* After power up, all blocks are locked, so unlock it here. */
>  	spinand_lock_block(chip, BL_ALL_UNLOCKED);
>  
> +	/* Build bad block table */
> +	ret = spinand_scan_bbt(chip);
> +	if (ret) {
> +		dev_err(chip->dev, "Scan Bad Block Table failed.\n");
> +		goto err2;
> +	}
> +
>  	return nand_register(nand);
>  
> -err:
> +err2:

General note: please use better descriptions for your labels, like
err_free_buf

> +	devm_kfree(chip->dev, chip->buf);

This is not needed (automatically released).

> +err1:
>  	return ret;
>  }
>  
> @@ -1191,10 +1502,14 @@ int spinand_register(struct spinand_device *chip)
>  int spinand_unregister(struct spinand_device *chip)
>  {
>  	struct nand_device *nand = &chip->base;
> +	struct nand_bbt_descr *bd = nand->bbt.bbp;
>  
>  	nand_unregister(nand);
>  	spinand_manufacturer_cleanup(chip);
>  	devm_kfree(chip->dev, chip->buf);
> +	kfree(nand->bbt.bbt);
> +	if (bd->options & NAND_BBT_DYNAMICSTRUCT)
> +		devm_kfree(chip->dev, bd);

Ditto.

>  
>  	return 0;
>  }

Re: [PATCH v5 4/6] nand: spi: add Micron spi nand support

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:51 +0800
Peter Pan <peterpandong@micron.com> wrote:

> This commit is to add support for Micron MT29F2G01ABAGD
> spi nand chip.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  drivers/mtd/nand/spi/Makefile |   1 +
>  drivers/mtd/nand/spi/core.c   |   4 +-
>  drivers/mtd/nand/spi/micron.c | 254 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h   |   2 +
>  4 files changed, 260 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mtd/nand/spi/micron.c
> 
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index a677a4d..df6c2ea 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_MTD_SPI_NAND) += core.o
> +obj-$(CONFIG_MTD_SPI_NAND) += micron.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 52113fc..b2530fa 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1252,7 +1252,9 @@ static int spinand_scan_bbt(struct spinand_device *chip)
>  	return nand_scan_bbt(nand);
>  }
>  
> -static const struct spinand_manufacturer *spinand_manufacturers[] = {};
> +static const struct spinand_manufacturer *spinand_manufacturers[] = {
> +	&micron_spinand_manufacture

			^ manufacturer

> +};
>  

[...]

> +
> +/*
> + * micron_spinand_scan_id_table - scan chip info in id table
> + * @chip: SPI-NAND device structure
> + * @id: point to manufacture id and device id
> + * Description:
> + *   If found in id table, config chip with table information.
> + */
> +static bool micron_spinand_scan_id_table(struct spinand_device *chip, u8 dev_id)
> +{
> +	struct mtd_info *mtd = spinand_to_mtd(chip);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	struct micron_spinand_info *item = NULL;
> +	struct nand_memory_organization *memorg = &nand->memorg;
> +	struct spinand_ecc_engine *ecc_engine = NULL;
> +	int i = 0;
> +
> +	for (; i < ARRAY_SIZE(micron_spinand_table); i++) {
> +		item = (struct micron_spinand_info *)micron_spinand_table + i;
> +		if (dev_id != item->dev_id)
> +			continue;
> +		chip->name = item->name;
> +		memorg->eraseblocksize = item->page_size * item->pages_per_blk;
> +		memorg->pagesize = item->page_size;
> +		memorg->oobsize = item->oob_size;
> +		memorg->diesize = memorg->eraseblocksize * item->blks_per_lun;
> +		memorg->ndies = item->luns_per_chip;
> +		if (chip->ecc.type == SPINAND_ECC_ONDIE) {
> +			ecc_engine = devm_kzalloc(chip->dev,
> +						  sizeof(*ecc_engine),
> +						  GFP_KERNEL);
> +			if (!ecc_engine) {
> +				dev_err(chip->dev,
> +					"fail to allocate ecc engine.\n");
> +				return false;
> +			}
> +			ecc_engine->ops = item->ecc_engine_ops;
> +			ecc_engine->strength = item->ecc_strength;
> +			ecc_engine->steps = item->ecc_steps;
> +			mtd_set_ooblayout(mtd, item->ooblayout_ops);
> +			chip->ecc.engine = ecc_engine;
> +		}

I'd prefer to have the ECC initialization done in ->init().

> +		chip->rw_mode = item->rw_mode;
> +
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/*
> + * micron_spinand_detect - initialize device related part in spinand_device
> + * struct if it is Micron device.
> + * @chip: SPI NAND device structure
> + */
> +static bool micron_spinand_detect(struct spinand_device *chip)
> +{
> +	u8 *id = chip->id.data;
> +
> +	/*
> +	 * Micron SPI NAND read ID need a dummy byte,
> +	 * so the first byte in raw_id is dummy.
> +	 */
> +	if (id[1] != SPINAND_MFR_MICRON)
> +		return false;
> +
> +	return micron_spinand_scan_id_table(chip, id[2]);
> +}
> +
> +/*
> + * micron_spinand_cleanup -  free manufacutre related resources
> + * @chip: SPI NAND device structure
> + */
> +static void micron_spinand_cleanup(struct spinand_device *chip)
> +{
> +	if (chip->ecc.type == SPINAND_ECC_ONDIE)
> +		devm_kfree(chip->dev, chip->ecc.engine);

Not needed.

> +}
> +
> +/*
> + * micron_spinand_prepare_op - Fix address for cache operation.
> + * @chip: SPI NAND device structure
> + * @op: pointer to spinand_op struct
> + * @page: page address
> + * @column: column address
> + */
> +static void micron_spinand_prepare_op(struct spinand_device *chip,
> +				      struct spinand_op *op, u32 page,
> +				      u32 column)
> +{
> +	op->addr[0] |= (u8)((nand_page_to_eraseblock(&chip->base, page)
> +			     & 0x1) << 4);
> +	op->n_addr += micron_spinand_get_dummy(chip, op);
> +}
> +
> +static const struct spinand_manufacturer_ops micron_spinand_manuf_ops = {
> +	.detect = micron_spinand_detect,
> +	.cleanup = micron_spinand_cleanup,
> +	.prepare_op = micron_spinand_prepare_op,
> +};
> +
> +const struct spinand_manufacturer micron_spinand_manufacture = {
> +	.id = SPINAND_MFR_MICRON,
> +	.name = "Micron",
> +	.ops = &micron_spinand_manuf_ops,
> +};
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index 5c8bb70..3c6dd48 100644
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -111,6 +111,8 @@ struct spinand_manufacturer {
>  	const struct spinand_manufacturer_ops *ops;
>  };
>  
> +extern const struct spinand_manufacturer micron_spinand_manufacture;

Same typo here:
s/micron_spinand_manufacture/micron_spinand_manufacturer/

> +
>  struct spinand_ecc_engine_ops {
>  	void (*get_ecc_status)(struct spinand_device *chip,
>  			       unsigned int status, unsigned int *corrected,

Re: [PATCH v5 5/6] nand: spi: Add generic SPI controller support

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:52 +0800
Peter Pan <peterpandong@micron.com> wrote:

> This commit supports to use generic spi controller
> as spi nand controller.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  drivers/mtd/nand/spi/Kconfig                   |   2 +
>  drivers/mtd/nand/spi/Makefile                  |   1 +
>  drivers/mtd/nand/spi/controllers/Kconfig       |   5 +
>  drivers/mtd/nand/spi/controllers/Makefile      |   1 +
>  drivers/mtd/nand/spi/controllers/generic-spi.c | 159 +++++++++++++++++++++++++
>  5 files changed, 168 insertions(+)
>  create mode 100644 drivers/mtd/nand/spi/controllers/Kconfig
>  create mode 100644 drivers/mtd/nand/spi/controllers/Makefile
>  create mode 100644 drivers/mtd/nand/spi/controllers/generic-spi.c
> 
> diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
> index d77c46e..6bd1c65 100644
> --- a/drivers/mtd/nand/spi/Kconfig
> +++ b/drivers/mtd/nand/spi/Kconfig
> @@ -3,3 +3,5 @@ menuconfig MTD_SPI_NAND
>  	depends on MTD_NAND
>  	help
>  	  This is the framework for the SPI NAND device drivers.
> +
> +source "drivers/mtd/nand/spi/controllers/Kconfig"
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index df6c2ea..822e48d 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_MTD_SPI_NAND) += core.o
>  obj-$(CONFIG_MTD_SPI_NAND) += micron.o
> +obj-$(CONFIG_MTD_SPI_NAND) += controllers/
> diff --git a/drivers/mtd/nand/spi/controllers/Kconfig b/drivers/mtd/nand/spi/controllers/Kconfig
> new file mode 100644
> index 0000000..8ab7023
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/controllers/Kconfig
> @@ -0,0 +1,5 @@
> +config GENERIC_SPI_NAND
> +	tristate "SPI NAND with generic SPI bus Support"
> +	depends on MTD_SPI_NAND && SPI
> +	help
> +	  This is to support SPI NAND device with generic SPI bus.
> diff --git a/drivers/mtd/nand/spi/controllers/Makefile b/drivers/mtd/nand/spi/controllers/Makefile
> new file mode 100644
> index 0000000..46cbf29
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/controllers/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_GENERIC_SPI_NAND) += generic-spi.o
> diff --git a/drivers/mtd/nand/spi/controllers/generic-spi.c b/drivers/mtd/nand/spi/controllers/generic-spi.c
> new file mode 100644
> index 0000000..d354874
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/controllers/generic-spi.c
> @@ -0,0 +1,159 @@
> +/*
> + * Copyright (c) 2009-2017 Micron Technology, Inc.
> + *
> + * 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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/mtd/spinand.h>
> +
> +struct gen_spi_spinand_controller {
> +	struct spinand_controller ctrl;
> +	struct spi_device *spi;
> +};
> +
> +#define to_gen_spi_spinand_controller(c) \
> +	container_of(c, struct gen_spi_spinand_controller, ctrl)
> +
> +/*
> + * gen_spi_spinand_exec_op - to process a command to send to the
> + * SPI NAND by generic SPI bus
> + * @chip: SPI NAND device structure
> + * @op: SPI NAND operation descriptor
> + */
> +static int gen_spi_spinand_exec_op(struct spinand_device *chip,
> +				   struct spinand_op *op)
> +{
> +	struct spi_message message;
> +	struct spi_transfer x[3];
> +	struct spinand_controller *scontroller = chip->controller.controller;
> +	struct gen_spi_spinand_controller *controller;
> +
> +	controller = to_gen_spi_spinand_controller(scontroller);
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof(x));
> +	x[0].len = 1;
> +	x[0].tx_nbits = 1;
> +	x[0].tx_buf = &op->cmd;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	if (op->n_addr + op->dummy_bytes) {
> +		x[1].len = op->n_addr + op->dummy_bytes;
> +		x[1].tx_nbits = op->addr_nbits;
> +		x[1].tx_buf = op->addr;
> +		spi_message_add_tail(&x[1], &message);
> +	}

Can you add empty lines to separate each block of code (this comment
applies to the whole contribution).

> +	if (op->n_tx) {
> +		x[2].len = op->n_tx;
> +		x[2].tx_nbits = op->data_nbits;
> +		x[2].tx_buf = op->tx_buf;
> +		spi_message_add_tail(&x[2], &message);
> +	} else if (op->n_rx) {
> +		x[2].len = op->n_rx;
> +		x[2].rx_nbits = op->data_nbits;
> +		x[2].rx_buf = op->rx_buf;
> +		spi_message_add_tail(&x[2], &message);
> +	}

Empty line here too.

> +	return spi_sync(controller->spi, &message);
> +}
> +
> +static struct spinand_controller_ops gen_spi_spinand_ops = {
> +	.exec_op = gen_spi_spinand_exec_op,
> +};
> +
> +static int gen_spi_spinand_probe(struct spi_device *spi)
> +{
> +	struct spinand_device *chip;
> +	struct gen_spi_spinand_controller *controller;
> +	struct spinand_controller *spinand_controller;
> +	struct device *dev = &spi->dev;
> +	u16 mode = spi->mode;
> +	int ret;
> +
> +	chip = spinand_alloc(dev);
> +	if (IS_ERR(chip)) {
> +		ret = PTR_ERR(chip);
> +		goto err1;
> +	}

Ditto.

> +	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> +	if (!controller) {
> +		ret = -ENOMEM;
> +		goto err2;
> +	}

Ditto (I think you got it now, so I'll stop here ;)).

> +	controller->spi = spi;
> +	spinand_controller = &controller->ctrl;
> +	spinand_controller->ops = &gen_spi_spinand_ops;
> +	spinand_controller->caps = SPINAND_CAP_RD_X1 | SPINAND_CAP_WR_X1;
> +
> +	if ((mode & SPI_RX_QUAD) && (mode & SPI_TX_QUAD))
> +		spinand_controller->caps |= SPINAND_CAP_RD_QUAD;
> +	if ((mode & SPI_RX_DUAL) && (mode & SPI_TX_DUAL))
> +		spinand_controller->caps |= SPINAND_CAP_RD_DUAL;
> +	if (mode & SPI_RX_QUAD)
> +		spinand_controller->caps |= SPINAND_CAP_RD_X4;
> +	if (mode & SPI_RX_DUAL)
> +		spinand_controller->caps |= SPINAND_CAP_RD_X2;
> +	if (mode & SPI_TX_QUAD)
> +		spinand_controller->caps |= SPINAND_CAP_WR_QUAD |
> +					    SPINAND_CAP_WR_X4;
> +	if (mode & SPI_TX_DUAL)
> +		spinand_controller->caps |= SPINAND_CAP_WR_DUAL |
> +					    SPINAND_CAP_WR_X2;
> +	chip->controller.controller = spinand_controller;
> +	/*
> +	 * generic spi controller doesn't have ecc capability,
> +	 * so use on-die ecc.
> +	 */
> +	chip->ecc.type = SPINAND_ECC_ONDIE;

Hm, can't we use the DT property to select that? It seems a bit weird
to have on-die ECC enabled by default.

> +	spi_set_drvdata(spi, chip);
> +
> +	ret = spinand_register(chip);
> +	if (ret)
> +		goto err3;
> +
> +	return 0;
> +
> +err3:
> +	devm_kfree(dev, controller);
> +err2:

Already said that earlier, try to find better names for your labels.
Here, err_free_spinand. But anyway, since everything is allocated with
devm_, I think you can just skip these steps.

> +	spinand_free(chip);
> +err1:
> +	return ret;
> +}
> +
> +static int gen_spi_spinand_remove(struct spi_device *spi)
> +{
> +	struct spinand_device *chip = spi_get_drvdata(spi);
> +	struct spinand_controller *scontroller = chip->controller.controller;
> +	struct gen_spi_spinand_controller *controller;
> +
> +	spinand_unregister(chip);
> +	controller = to_gen_spi_spinand_controller(scontroller);
> +	devm_kfree(&spi->dev, controller);
> +	spinand_free(chip);
> +
> +	return 0;
> +}
> +
> +static struct spi_driver gen_spi_spinand_driver = {
> +	.driver = {
> +		.name	= "generic_spinand",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe	= gen_spi_spinand_probe,
> +	.remove	= gen_spi_spinand_remove,
> +};
> +module_spi_driver(gen_spi_spinand_driver);
> +
> +MODULE_DESCRIPTION("Generic SPI controller to support SPI NAND");
> +MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH v5 1/6] nand: spi: add basic blocks for infrastructure

[Boris Brezillon]

On Mon, 10 Apr 2017 15:51:48 +0800
Peter Pan <peterpandong@micron.com> wrote:

> +/*
> + * spinand_alloc - [SPI NAND Interface] allocate SPI NAND device instance
> + * @dev: pointer to device model structure
> + */
> +struct spinand_device *spinand_alloc(struct device *dev)

Since you're using devm_kzalloc() here, can we rename the function
devm_spinand_alloc()?

> +{
> +	struct spinand_device *chip;
> +	struct mtd_info *mtd;
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return ERR_PTR(-ENOMEM);
> +
> +	spinand_set_of_node(chip, dev->of_node);
> +	mutex_init(&chip->lock);
> +	chip->dev = dev;
> +	mtd = spinand_to_mtd(chip);
> +	mtd->dev.parent = dev;
> +
> +	return chip;
> +}
> +EXPORT_SYMBOL_GPL(spinand_alloc);

Re: [PATCH v5 5/6] nand: spi: Add generic SPI controller support

[Boris Brezillon]

On Fri, 14 Apr 2017 15:26:24 +0200
Marek Vasut <marex@denx.de> wrote:

> On 04/10/2017 09:51 AM, Peter Pan wrote:
> > This commit supports to use generic spi controller
> > as spi nand controller.
> > 
> > Signed-off-by: Peter Pan <peterpandong@micron.com>
> > ---
> >  drivers/mtd/nand/spi/Kconfig                   |   2 +
> >  drivers/mtd/nand/spi/Makefile                  |   1 +
> >  drivers/mtd/nand/spi/controllers/Kconfig       |   5 +
> >  drivers/mtd/nand/spi/controllers/Makefile      |   1 +
> >  drivers/mtd/nand/spi/controllers/generic-spi.c | 159 +++++++++++++++++++++++++
> >  5 files changed, 168 insertions(+)
> >  create mode 100644 drivers/mtd/nand/spi/controllers/Kconfig
> >  create mode 100644 drivers/mtd/nand/spi/controllers/Makefile
> >  create mode 100644 drivers/mtd/nand/spi/controllers/generic-spi.c  
> 
> [...]
> 
> > +static int gen_spi_spinand_probe(struct spi_device *spi)
> > +{
> > +	struct spinand_device *chip;
> > +	struct gen_spi_spinand_controller *controller;
> > +	struct spinand_controller *spinand_controller;
> > +	struct device *dev = &spi->dev;
> > +	u16 mode = spi->mode;
> > +	int ret;
> > +
> > +	chip = spinand_alloc(dev);
> > +	if (IS_ERR(chip)) {
> > +		ret = PTR_ERR(chip);
> > +		goto err1;
> > +	}  
> 
> Would it make sense to embed the struct spinand_device into struct
> gen_spi_nand_controller , so you'd get rid of one allocation ?

I have other plans. I'd like drivers to only register their
SPI NAND controller, and let the core parse the DT (or board info) to
create the spinand devices and inform the controller that a new device
is being added (using a ->add() hook).

With this model, the core does not know anything about driver-specific
private data, and thus has to allocate a spinand device no matter what.

Even if we're not here yet, I'd like to keep that in mind, and already
force SPI NAND controller drivers to use spinand_alloc instead of
embedding the struct.

> 
> > +	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> > +	if (!controller) {
> > +		ret = -ENOMEM;
> > +		goto err2;
> > +	}
> > +	controller->spi = spi;
> > +	spinand_controller = &controller->ctrl;
> > +	spinand_controller->ops = &gen_spi_spinand_ops;
> > +	spinand_controller->caps = SPINAND_CAP_RD_X1 | SPINAND_CAP_WR_X1;
> > +
> > +	if ((mode & SPI_RX_QUAD) && (mode & SPI_TX_QUAD))
> > +		spinand_controller->caps |= SPINAND_CAP_RD_QUAD;
> > +	if ((mode & SPI_RX_DUAL) && (mode & SPI_TX_DUAL))
> > +		spinand_controller->caps |= SPINAND_CAP_RD_DUAL;
> > +	if (mode & SPI_RX_QUAD)
> > +		spinand_controller->caps |= SPINAND_CAP_RD_X4;
> > +	if (mode & SPI_RX_DUAL)
> > +		spinand_controller->caps |= SPINAND_CAP_RD_X2;
> > +	if (mode & SPI_TX_QUAD)
> > +		spinand_controller->caps |= SPINAND_CAP_WR_QUAD |
> > +					    SPINAND_CAP_WR_X4;
> > +	if (mode & SPI_TX_DUAL)
> > +		spinand_controller->caps |= SPINAND_CAP_WR_DUAL |
> > +					    SPINAND_CAP_WR_X2;
> > +	chip->controller.controller = spinand_controller;
> > +	/*
> > +	 * generic spi controller doesn't have ecc capability,
> > +	 * so use on-die ecc.
> > +	 */
> > +	chip->ecc.type = SPINAND_ECC_ONDIE;
> > +	spi_set_drvdata(spi, chip);
> > +
> > +	ret = spinand_register(chip);
> > +	if (ret)
> > +		goto err3;
> > +
> > +	return 0;
> > +
> > +err3:
> > +	devm_kfree(dev, controller);
> > +err2:
> > +	spinand_free(chip);
> > +err1:
> > +	return ret;
> > +}
> > +
> > +static int gen_spi_spinand_remove(struct spi_device *spi)
> > +{
> > +	struct spinand_device *chip = spi_get_drvdata(spi);
> > +	struct spinand_controller *scontroller = chip->controller.controller;
> > +	struct gen_spi_spinand_controller *controller;
> > +
> > +	spinand_unregister(chip);
> > +	controller = to_gen_spi_spinand_controller(scontroller);
> > +	devm_kfree(&spi->dev, controller);
> > +	spinand_free(chip);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct spi_driver gen_spi_spinand_driver = {
> > +	.driver = {
> > +		.name	= "generic_spinand",
> > +		.owner	= THIS_MODULE,
> > +	},
> > +	.probe	= gen_spi_spinand_probe,
> > +	.remove	= gen_spi_spinand_remove,
> > +};
> > +module_spi_driver(gen_spi_spinand_driver);
> > +
> > +MODULE_DESCRIPTION("Generic SPI controller to support SPI NAND");
> > +MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
> > +MODULE_LICENSE("GPL v2");
> >   
> 
> 

Re: [PATCH v5 0/6] Introduction to SPI NAND framework

[Boris Brezillon]

Hi Peter,

On Mon, 10 Apr 2017 15:51:47 +0800
Peter Pan <peterpandong@micron.com> wrote:

> First of all, thank Boris, Marek and Cyrille for your comments
> on v4 and thank Arnaud for your testing on v4. Since I'm quite
> busy last week, I failed to reply all the comments, I'm really
> sorry for that.
> 
> According to Boris's suggestion, I rebased my patches on nand/next
> branch with Boris's generic NAND patches. This time I only send
> my SPI NAND patches out since it's the focus.
> 
> I created my own branch for convenience[3]. You can find both
> my SPI NAND patches and Boris's generic NAND framework patches.
> 
> This series introductes a SPI NAND framework.
> SPI NAND is a new NAND family device with SPI protocol as
> its interface. And its command set is totally different
> with parallel NAND.
> 
> Our first attempt was more than 2 years ago[1]. At that
> time, I didn't make BBT shareable and there were too many
> duplicate code with parallel NAND, so that serie stoped.
> But the discussion never stops. Now Boris has a plan to
> make a generic NAND framework which can be shared with
> both parallel and SPI NAND. Now the first step of the
> new generic NAND framework is finished. And it is waiting
> for a user. After discussion with Boris. We both think it's
> time to rebuild SPI NAND framework based on the new NAND
> framework and send out for reviewing.
> 
> This series is based on Boris's nand/generic branch[2], which
> is on 4.11-rc1. In this serie, BBT code is totally shared.
> Of course SPI NAND can share more code with parallel, this
> requires to put more in new NAND core (now only BBT included).
> I'd like to send this serie out first, then we can decide
> which part should be in new NAND core.
> 
> This series only supports basic SPI NAND features and uses
> generic spi controller for data transfer, on-die ECC for data
> correction. Support advanced features and specific SPI NAND
> controller with hardware ECC is the next step.
> 
> This series is tested on Xilinx Zedboard with Micron
> MT29F2G01ABAGDSF SPI NAND chip.

I think we're almost good here. Just need to address the comments made
by Marek an I and you can send a v6 containing all the materials
(including my generic-NAND layer).

If you don't mind, I'd recommend that we keep ECC support for later, or
at least separate the code in different patches so I can take
everything expect that if I'm not happy with the code.

Hopefully, everything should be ready for 4.13 (finally :-)).

Thanks for your patience.

Boris

Re: [PATCH v5 0/6] Introduction to SPI NAND framework

[Peter Pan]

Hi Boris and Marek,

On Tue, Apr 18, 2017 at 3:42 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Hi Peter,
>
> On Mon, 10 Apr 2017 15:51:47 +0800
> Peter Pan <peterpandong@micron.com> wrote:
>
>> First of all, thank Boris, Marek and Cyrille for your comments
>> on v4 and thank Arnaud for your testing on v4. Since I'm quite
>> busy last week, I failed to reply all the comments, I'm really
>> sorry for that.
>>
>> According to Boris's suggestion, I rebased my patches on nand/next
>> branch with Boris's generic NAND patches. This time I only send
>> my SPI NAND patches out since it's the focus.
>>
>> I created my own branch for convenience[3]. You can find both
>> my SPI NAND patches and Boris's generic NAND framework patches.
>>
>> This series introductes a SPI NAND framework.
>> SPI NAND is a new NAND family device with SPI protocol as
>> its interface. And its command set is totally different
>> with parallel NAND.
>>
>> Our first attempt was more than 2 years ago[1]. At that
>> time, I didn't make BBT shareable and there were too many
>> duplicate code with parallel NAND, so that serie stoped.
>> But the discussion never stops. Now Boris has a plan to
>> make a generic NAND framework which can be shared with
>> both parallel and SPI NAND. Now the first step of the
>> new generic NAND framework is finished. And it is waiting
>> for a user. After discussion with Boris. We both think it's
>> time to rebuild SPI NAND framework based on the new NAND
>> framework and send out for reviewing.
>>
>> This series is based on Boris's nand/generic branch[2], which
>> is on 4.11-rc1. In this serie, BBT code is totally shared.
>> Of course SPI NAND can share more code with parallel, this
>> requires to put more in new NAND core (now only BBT included).
>> I'd like to send this serie out first, then we can decide
>> which part should be in new NAND core.
>>
>> This series only supports basic SPI NAND features and uses
>> generic spi controller for data transfer, on-die ECC for data
>> correction. Support advanced features and specific SPI NAND
>> controller with hardware ECC is the next step.
>>
>> This series is tested on Xilinx Zedboard with Micron
>> MT29F2G01ABAGDSF SPI NAND chip.
>
> I think we're almost good here. Just need to address the comments made
> by Marek an I and you can send a v6 containing all the materials
> (including my generic-NAND layer).
>
> If you don't mind, I'd recommend that we keep ECC support for later, or
> at least separate the code in different patches so I can take
> everything expect that if I'm not happy with the code.
>
> Hopefully, everything should be ready for 4.13 (finally :-)).
>
> Thanks for your patience.

Right now I'm busy with other affairs and cannot go deep in your comments.
I hope I can return to SPI NAND soon. I will try to reply the comments
and send v6 out next month. Thanks for your effort on this series.

Peter Pan