[PATCH 00/11] Introduction to SPI NAND framework

[PATCH 00/11] Introduction to SPI NAND framework

[Peter Pan]

This serie 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 serie is based on Boris's nand/generic branch[2], which
is on 4.9-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 serie 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.


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


Peter Pan (11):
  nand: Add SPI NAND cmd set and register definition
  nand: spi: create spi_nand_chip struct
  nand: spi: Abstract SPI NAND cmd set to functions
  nand: spi: Add read function support
  nand: spi: Add write function support
  nand: spi: Add erase function support
  nand: spi: Add init/release function
  nand: spi: Add bad block support
  nand: spi: Add BBT support
  nand: spi: Add generic SPI controller support
  nand: spi: Add arguments check for read/write

 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/chips/Kconfig       |    5 +
 drivers/mtd/nand/spi/chips/Makefile      |    1 +
 drivers/mtd/nand/spi/chips/generic_spi.c |  269 ++++++
 drivers/mtd/nand/spi/spi-nand-base.c     | 1537 ++++++++++++++++++++++++++++++
 drivers/mtd/nand/spi/spi-nand-cmd.c      |   69 ++
 include/linux/mtd/spi-nand.h             |  269 ++++++
 10 files changed, 2162 insertions(+)
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/chips/Kconfig
 create mode 100644 drivers/mtd/nand/spi/chips/Makefile
 create mode 100644 drivers/mtd/nand/spi/chips/generic_spi.c
 create mode 100644 drivers/mtd/nand/spi/spi-nand-base.c
 create mode 100644 drivers/mtd/nand/spi/spi-nand-cmd.c
 create mode 100644 include/linux/mtd/spi-nand.h

-- 
1.9.1

[PATCH 01/11] nand: Add SPI NAND cmd set and register definition

[Peter Pan]

This commit adds SPI NAND command set and register
definition according Micron SPI NAND data sheet.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 include/linux/mtd/spi-nand.h | 112 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 include/linux/mtd/spi-nand.h

diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
new file mode 100644
index 0000000..68442e08
--- /dev/null
+++ b/include/linux/mtd/spi-nand.h
@@ -0,0 +1,112 @@
+/**
+* spi-nand.h
+*
+* 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_SPI_NAND_H
+#define __LINUX_MTD_SPI_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_PAGE_CACHE_RDM		0x30
+#define SPINAND_CMD_READ_PAGE_CACHE_LAST	0x3f
+#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
+#define SPINAND_CMD_END				0x0
+
+
+/* feature registers */
+#define REG_BLOCK_LOCK		0xa0
+#define REG_CFG			0xb0
+#define REG_STATUS		0xc0
+#define REG_DIE_SELECT		0xd0
+
+/* status */
+#define STATUS_OIP_MASK		0x01
+#define STATUS_CRBSY_MASK	0x80
+#define STATUS_READY		(0 << 0)
+#define STATUS_BUSY		(1 << 0)
+
+#define STATUS_E_FAIL_MASK	0x04
+#define STATUS_E_FAIL		(1 << 2)
+
+#define STATUS_P_FAIL_MASK	0x08
+#define STATUS_P_FAIL		(1 << 3)
+
+
+/*Configuration register defines*/
+#define CFG_QE_MASK		0x01
+#define CFG_QE_ENABLE		0x01
+#define CFG_ECC_MASK		0x10
+#define CFG_ECC_ENABLE		0x10
+#define CFG_LOT_MASK		0x20
+#define CFG_LOT_ENABLE		0x20
+#define CFG_OTP_MASK		0xc2
+#define CFG_OTP_ENTER		0x40
+#define CFG_OTP_EXIT		0x00
+
+/* block lock */
+#define BL_ALL_LOCKED		0x7c
+#define BL_U_1_1024_LOCKED		0x08
+#define BL_U_1_512_LOCKED		0x10
+#define BL_U_1_256_LOCKED		0x18
+#define BL_U_1_128_LOCKED		0x20
+#define BL_U_1_64_LOCKED		0x28
+#define BL_U_1_32_LOCKED		0x30
+#define BL_U_1_16_LOCKED		0x38
+#define BL_U_1_8_LOCKED		0x40
+#define BL_U_1_4_LOCKED		0x48
+#define BL_U_1_2_LOCKED		0x50
+#define BL_L_1_1024_LOCKED		0x0c
+#define BL_L_1_512_LOCKED		0x14
+#define BL_L_1_256_LOCKED		0x1c
+#define BL_L_1_128_LOCKED		0x24
+#define BL_L_1_64_LOCKED		0x2c
+#define BL_L_1_32_LOCKED		0x34
+#define BL_L_1_16_LOCKED		0x3c
+#define BL_L_1_8_LOCKED		0x44
+#define BL_L_1_4_LOCKED		0x4c
+#define BL_L_1_2_LOCKED		0x54
+#define BL_ALL_UNLOCKED		0X00
+
+/* die select */
+#define DIE_SELECT_MASK		0x40
+#define DIE_SELECT_DS0		0x00
+#define DIE_SELECT_DS1		0x40
+
+#define SPI_NAND_MT29F_ECC_MASK		0x70
+#define SPI_NAND_MT29F_ECC_0_BIT	0x00
+#define SPI_NAND_MT29F_ECC_1_3_BIT	0x10
+#define SPI_NAND_MT29F_ECC_4_6_BIT	0x30
+#define SPI_NAND_MT29F_ECC_7_8_BIT	0x50
+#define SPI_NAND_MT29F_ECC_UNCORR	0x20
+
+#endif /* __LINUX_MTD_SPI_NAND_H */
-- 
1.9.1

[PATCH 02/11] nand: spi: create spi_nand_chip struct

[Peter Pan]

Create spi_nand_chip struct and helper functions.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 include/linux/mtd/spi-nand.h | 67 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
index 68442e08..23b16f0 100644
--- a/include/linux/mtd/spi-nand.h
+++ b/include/linux/mtd/spi-nand.h
@@ -16,6 +16,12 @@
 #ifndef __LINUX_MTD_SPI_NAND_H
 #define __LINUX_MTD_SPI_NAND_H
 
+#include <linux/wait.h>
+#include <linux/spinlock.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/flashchip.h>
+#include <linux/mtd/nand.h>
+
 /*
  * Standard SPI-NAND flash commands
  */
@@ -109,4 +115,65 @@
 #define SPI_NAND_MT29F_ECC_7_8_BIT	0x50
 #define SPI_NAND_MT29F_ECC_UNCORR	0x20
 
+/**
+ * struct spi_nand_chip - SPI-NAND Private Flash Chip Data
+ * @base:		[INTERN] NAND device instance
+ * @chip_lock:		[INTERN] protection lock
+ * @name:		name of the chip
+ * @wq:			[INTERN] wait queue to sleep on if a SPI-NAND operation
+ *			is in progress used instead of the per chip wait queue
+ *			when a hw controller is available.
+ * @mfr_id:		[BOARDSPECIFIC] manufacture id
+ * @dev_id:		[BOARDSPECIFIC] device id
+ * @state:		[INTERN] the current state of the SPI-NAND device
+ * @read_cache_op:	[REPLACEABLE] Opcode of read from cache
+ * @write_cache_op:	[REPLACEABLE] Opcode of program load
+ * @buf:		[INTERN] buffer for read/write data
+ * @oobbuf:		[INTERN] buffer for read/write oob
+ * @controller_caps:	[INTERN] capacities of SPI NAND controller
+ * @size:		[INTERN] the size of chip
+ * @options:		[BOARDSPECIFIC] various chip options. They can partly
+ *			be set to inform nand_scan about special functionality.
+ * @ecc_strength:	[INTERN] ECC correctability from the datasheet.
+ * @priv:		[BOARDSPECIFIC] pointer to controller data
+ */
+struct spi_nand_chip {
+	struct nand_device	base;
+	spinlock_t	chip_lock;
+	char		*name;
+	wait_queue_head_t wq;
+	u8		mfr_id;
+	u8		dev_id;
+	flstate_t	state;
+	u8		read_cache_op;
+	u8		write_cache_op;
+	u8		*buf;
+	u8		*oobbuf;
+	u32		controller_caps;
+	u64		size;
+	u32		options;
+	u32		ecc_strength;
+	void		*priv;
+};
+
+static inline struct spi_nand_chip *mtd_to_spi_nand(struct mtd_info *mtd)
+{
+	return container_of(mtd_to_nand(mtd), struct spi_nand_chip, base);
+}
+
+static inline struct mtd_info *spi_nand_to_mtd(struct spi_nand_chip *chip)
+{
+	return nand_to_mtd(&chip->base);
+}
+
+static inline void *spi_nand_get_controller_data(struct spi_nand_chip *chip)
+{
+	return chip->priv;
+}
+
+static inline void spi_nand_set_controller_data(struct spi_nand_chip *chip,
+						void *priv)
+{
+	chip->priv = priv;
+}
 #endif /* __LINUX_MTD_SPI_NAND_H */
-- 
1.9.1

[PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Peter Pan]

This commit abstracts basic SPI NAND commands to
functions in spi-nand-base.c. Because command sets
have difference by vendors, we create spi-nand-cmd.c
to define this difference by command config table.

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        |   2 +
 drivers/mtd/nand/spi/spi-nand-base.c | 368 +++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/spi/spi-nand-cmd.c  |  69 +++++++
 include/linux/mtd/spi-nand.h         |  34 ++++
 7 files changed, 480 insertions(+)
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/spi-nand-base.c
 create mode 100644 drivers/mtd/nand/spi/spi-nand-cmd.c

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..04a7b71
--- /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..3c617d6
--- /dev/null
+++ b/drivers/mtd/nand/spi/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-base.o
+obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-cmd.o
diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
new file mode 100644
index 0000000..b75e5cd
--- /dev/null
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -0,0 +1,368 @@
+/**
+* spi-nand-base.c
+*
+* 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.
+*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/mtd/spi-nand.h>
+
+static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
+				struct spi_nand_cmd *cmd)
+{
+	return chip->command_fn(chip, cmd);
+}
+
+/**
+ * spi_nand_read_reg - send command 0Fh to read register
+ * @chip: SPI-NAND device structure
+ * @reg; register to read
+ * @buf: buffer to store value
+ */
+static int spi_nand_read_reg(struct spi_nand_chip *chip,
+			uint8_t reg, uint8_t *buf)
+{
+	struct spi_nand_cmd cmd;
+	int ret;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_GET_FEATURE;
+	cmd.n_addr = 1;
+	cmd.addr[0] = reg;
+	cmd.n_rx = 1;
+	cmd.rx_buf = buf;
+
+	ret = spi_nand_issue_cmd(chip, &cmd);
+	if (ret < 0)
+		pr_err("err: %d read register %d\n", ret, reg);
+
+	return ret;
+}
+
+/**
+ * spi_nand_write_reg - send command 1Fh to write register
+ * @chip: SPI-NAND device structure
+ * @reg; register to write
+ * @buf: buffer stored value
+ */
+static int spi_nand_write_reg(struct spi_nand_chip *chip,
+			uint8_t reg, uint8_t *buf)
+{
+	struct spi_nand_cmd cmd;
+	int ret;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_SET_FEATURE;
+	cmd.n_addr = 1;
+	cmd.addr[0] = reg;
+	cmd.n_tx = 1,
+	cmd.tx_buf = buf,
+
+	ret = spi_nand_issue_cmd(chip, &cmd);
+	if (ret < 0)
+		pr_err("err: %d write register %d\n", ret, reg);
+
+	return ret;
+}
+
+/**
+ * spi_nand_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.
+ *   Once the status turns to be ready, the other status bits also are
+ *   valid status bits.
+ */
+static int spi_nand_read_status(struct spi_nand_chip *chip, uint8_t *status)
+{
+	return spi_nand_read_reg(chip, REG_STATUS, status);
+}
+
+/**
+ * spi_nand_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 spi_nand_get_cfg(struct spi_nand_chip *chip, u8 *cfg)
+{
+	return spi_nand_read_reg(chip, REG_CFG, cfg);
+}
+
+/**
+ * spi_nand_set_cfg - set value to configuration register
+ * @chip: SPI-NAND device structure
+ * @cfg: buffer stored value
+ * Description:
+ *   Configuration register includes OTP config, Lock Tight enable/disable
+ *   and Internal ECC enable/disable.
+ */
+static int spi_nand_set_cfg(struct spi_nand_chip *chip, u8 *cfg)
+{
+	return spi_nand_write_reg(chip, REG_CFG, cfg);
+}
+
+/**
+ * spi_nand_enable_ecc - enable internal ECC
+ * @chip: SPI-NAND device structure
+ * Description:
+ *   There is one bit( bit 0x10 ) to set or to clear the internal ECC.
+ *   Enable chip internal ECC, set the bit to 1
+ *   Disable chip internal ECC, clear the bit to 0
+ */
+static void spi_nand_enable_ecc(struct spi_nand_chip *chip)
+{
+	u8 cfg = 0;
+
+	spi_nand_get_cfg(chip, &cfg);
+	if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE)
+		return;
+	cfg |= CFG_ECC_ENABLE;
+	spi_nand_set_cfg(chip, &cfg);
+}
+
+/**
+ * spi_nand_disable_ecc - disable internal ECC
+ * @chip: SPI-NAND device structure
+ * Description:
+ *   There is one bit( bit 0x10 ) to set or to clear the internal ECC.
+ *   Enable chip internal ECC, set the bit to 1
+ *   Disable chip internal ECC, clear the bit to 0
+ */
+static void spi_nand_disable_ecc(struct spi_nand_chip *chip)
+{
+	u8 cfg = 0;
+
+	spi_nand_get_cfg(chip, &cfg);
+	if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE) {
+		cfg &= ~CFG_ECC_ENABLE;
+		spi_nand_set_cfg(chip, &cfg);
+	}
+}
+
+/**
+ * spi_nand_write_enable - send command 06h to enable write or erase the
+ * Nand cells
+ * @chip: SPI-NAND device structure
+ * Description:
+ *   Before write and erase the Nand cells, the write enable has to be set.
+ *   After the write or erase, the write enable bit is automatically
+ *   cleared (status register bit 2)
+ *   Set the bit 2 of the status register has the same effect
+ */
+static int spi_nand_write_enable(struct spi_nand_chip *chip)
+{
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_WR_ENABLE;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+/**
+ * spi_nand_read_page_to_cache - send command 13h to read data from Nand
+ * to cache
+ * @chip: SPI-NAND device structure
+ * @page_addr: page to read
+ */
+static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
+					u32 page_addr)
+{
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_PAGE_READ;
+	cmd.n_addr = 3;
+	cmd.addr[0] = (u8)(page_addr >> 16);
+	cmd.addr[1] = (u8)(page_addr >> 8);
+	cmd.addr[2] = (u8)page_addr;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+/**
+ * spi_nand_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
+ * Description:
+ *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
+ *   The read can specify 1 to (page size + spare size) bytes of data read at
+ *   the corresponding locations.
+ *   No tRd delay.
+ */
+static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
+		u32 page_addr, u32 column, size_t len, u8 *rbuf)
+{
+	struct nand_device *nand = &chip->base;
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = chip->read_cache_op;
+	cmd.n_addr = 2;
+	cmd.addr[0] = (u8)(column >> 8);
+	if (chip->options & SPINAND_NEED_PLANE_SELECT)
+		cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
+				& 0x1) << 4);
+	cmd.addr[1] = (u8)column;
+	cmd.n_rx = len;
+	cmd.rx_buf = rbuf;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+/**
+ * spi_nand_program_data_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
+ * @clr_cache: clear cache register or not
+ * Description:
+ *   Command can be 02h, 32h, 84h, 34h
+ *   02h and 32h will clear the cache with 0xff value first
+ *   Since it is writing the data to cache, there is no tPROG time.
+ */
+static int spi_nand_program_data_to_cache(struct spi_nand_chip *chip,
+			u32 page_addr, u32 column, size_t len, const u8 *wbuf)
+{
+	struct nand_device *nand = &chip->base;
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = chip->write_cache_op;
+	cmd.n_addr = 2;
+	cmd.addr[0] = (u8)(column >> 8);
+	if (chip->options & SPINAND_NEED_PLANE_SELECT)
+		cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
+				& 0x1) << 4);
+	cmd.addr[1] = (u8)column;
+	cmd.n_tx = len;
+	cmd.tx_buf = wbuf;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+/**
+ * spi_nand_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.
+ * Description:
+ *   Need to wait for tPROG time to finish the transaction.
+ */
+static int spi_nand_program_execute(struct spi_nand_chip *chip, u32 page_addr)
+{
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_PROG_EXC;
+	cmd.n_addr = 3;
+	cmd.addr[0] = (u8)(page_addr >> 16);
+	cmd.addr[1] = (u8)(page_addr >> 8);
+	cmd.addr[2] = (u8)page_addr;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+
+/**
+ * spi_nand_erase_block_erase - send command D8h to erase a block
+ * @chip: SPI-NAND device structure
+ * @page_addr: the page to erase.
+ * Description:
+ *   Need to wait for tERS.
+ */
+static int spi_nand_erase_block(struct spi_nand_chip *chip,
+					u32 page_addr)
+{
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_BLK_ERASE;
+	cmd.n_addr = 3;
+	cmd.addr[0] = (u8)(page_addr >> 16);
+	cmd.addr[1] = (u8)(page_addr >> 8);
+	cmd.addr[2] = (u8)page_addr;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+/**
+ * spi_nand_read_id - send 9Fh command to get ID
+ * @chip: SPI-NAND device structure
+ * @buf: buffer to store id
+ */
+static int spi_nand_read_id(struct spi_nand_chip *chip, u8 *buf)
+{
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_READ_ID;
+	cmd.n_rx = 2;
+	cmd.rx_buf = buf;
+
+	return spi_nand_issue_cmd(chip, &cmd);
+}
+
+/**
+ * spi_nand_reset - send command FFh to reset chip.
+ * @chip: SPI-NAND device structure
+ */
+static int spi_nand_reset(struct spi_nand_chip *chip)
+{
+	struct spi_nand_cmd cmd;
+
+	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
+	cmd.cmd = SPINAND_CMD_RESET;
+
+	if (spi_nand_issue_cmd(chip, &cmd) < 0)
+		pr_err("spi_nand reset failed!\n");
+
+	/* elapse 2ms before issuing any other command */
+	udelay(2000);
+
+	return 0;
+}
+
+/**
+ * spi_nand_lock_block - write block lock register to
+ * lock/unlock device
+ * @spi: spi device structure
+ * @lock: value to set to block lock register
+ * Description:
+ *   After power up, all the Nand blocks are locked.  This function allows
+ *   one to unlock the blocks, and so it can be written or erased.
+ */
+static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
+{
+	return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
+}
+
+MODULE_DESCRIPTION("SPI NAND framework");
+MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mtd/nand/spi/spi-nand-cmd.c b/drivers/mtd/nand/spi/spi-nand-cmd.c
new file mode 100644
index 0000000..d63741e
--- /dev/null
+++ b/drivers/mtd/nand/spi/spi-nand-cmd.c
@@ -0,0 +1,69 @@
+/**
+* spi-nand-cmd.c
+*
+* 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/mtd/spi-nand.h>
+#include <linux/kernel.h>
+
+static struct spi_nand_cmd_cfg *cmd_table;
+
+static struct spi_nand_cmd_cfg micron_cmd_cfg_table[] = {
+/*opcode	addr_bytes	addr_bits	dummy_bytes	data_nbits*/
+	{SPINAND_CMD_GET_FEATURE,		1,	1,	0,	1},
+	{SPINAND_CMD_SET_FEATURE,		1,	1,	0,	1},
+	{SPINAND_CMD_PAGE_READ,			3,	1,	0,	0},
+	{SPINAND_CMD_READ_FROM_CACHE,		2,	1,	1,	1},
+	{SPINAND_CMD_READ_FROM_CACHE_FAST,	2,	1,	1,	1},
+	{SPINAND_CMD_READ_FROM_CACHE_X2,	2,	1,	1,	2},
+	{SPINAND_CMD_READ_FROM_CACHE_DUAL_IO,	2,	2,	1,	2},
+	{SPINAND_CMD_READ_FROM_CACHE_X4,	2,	1,	1,	4},
+	{SPINAND_CMD_READ_FROM_CACHE_QUAD_IO,	2,	4,	2,	4},
+	{SPINAND_CMD_BLK_ERASE,			3,	1,	0,	0},
+	{SPINAND_CMD_PROG_EXC,			3,	1,	0,	0},
+	{SPINAND_CMD_PROG_LOAD,			2,	1,	0,	1},
+	{SPINAND_CMD_PROG_LOAD_RDM_DATA,	2,	1,	0,	1},
+	{SPINAND_CMD_PROG_LOAD_X4,		2,	1,	0,	4},
+	{SPINAND_CMD_PROG_LOAD_RDM_DATA_X4,	2,	1,	0,	4},
+	{SPINAND_CMD_WR_ENABLE,			0,	0,	0,	0},
+	{SPINAND_CMD_WR_DISABLE,		0,	0,	0,	0},
+	{SPINAND_CMD_READ_ID,			0,	0,	1,	1},
+	{SPINAND_CMD_RESET,			0,	0,	0,	0},
+	{SPINAND_CMD_END},
+};
+
+struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode)
+{
+	struct spi_nand_cmd_cfg *index = cmd_table;
+
+	for (; index->opcode != SPINAND_CMD_END; index++) {
+		if (index->opcode == opcode)
+			return index;
+	}
+
+	pr_err("Invalid spi nand opcode %x\n", opcode);
+	BUG();
+}
+
+int spi_nand_set_cmd_cfg_table(int mfr)
+{
+	switch (mfr) {
+	case SPINAND_MFR_MICRON:
+		cmd_table = micron_cmd_cfg_table;
+		break;
+	default:
+		pr_err("Unknown device\n");
+		return -ENODEV;
+	}
+	return 0;
+}
diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
index 23b16f0..1fcbad7 100644
--- a/include/linux/mtd/spi-nand.h
+++ b/include/linux/mtd/spi-nand.h
@@ -115,6 +115,9 @@
 #define SPI_NAND_MT29F_ECC_7_8_BIT	0x50
 #define SPI_NAND_MT29F_ECC_UNCORR	0x20
 
+
+struct spi_nand_cmd;
+
 /**
  * struct spi_nand_chip - SPI-NAND Private Flash Chip Data
  * @base:		[INTERN] NAND device instance
@@ -128,6 +131,7 @@
  * @state:		[INTERN] the current state of the SPI-NAND device
  * @read_cache_op:	[REPLACEABLE] Opcode of read from cache
  * @write_cache_op:	[REPLACEABLE] Opcode of program load
+ * @command_fn:		[BOARDSPECIFIC] function to handle command transfer
  * @buf:		[INTERN] buffer for read/write data
  * @oobbuf:		[INTERN] buffer for read/write oob
  * @controller_caps:	[INTERN] capacities of SPI NAND controller
@@ -147,6 +151,8 @@ struct spi_nand_chip {
 	flstate_t	state;
 	u8		read_cache_op;
 	u8		write_cache_op;
+	int (*command_fn)(struct spi_nand_chip *this,
+			struct spi_nand_cmd *cmd);
 	u8		*buf;
 	u8		*oobbuf;
 	u32		controller_caps;
@@ -176,4 +182,32 @@ static inline void spi_nand_set_controller_data(struct spi_nand_chip *chip,
 {
 	chip->priv = priv;
 }
+
+#define SPINAND_MAX_ADDR_LEN		4
+
+struct spi_nand_cmd {
+	u8		cmd;
+	u8		n_addr;		/* Number of address */
+	u8		addr[SPINAND_MAX_ADDR_LEN];	/* Reg Offset */
+	u32		n_tx;		/* Number of tx bytes */
+	const u8	*tx_buf;	/* Tx buf */
+	u32		n_rx;		/* Number of rx bytes */
+	u8		*rx_buf;	/* Rx buf */
+};
+
+struct spi_nand_cmd_cfg {
+	u8		opcode;
+	u8		addr_bytes;
+	u8		addr_bits;
+	u8		dummy_bytes;
+	u8		data_bits;
+};
+
+/*SPI NAND chip options*/
+#define SPINAND_NEED_PLANE_SELECT	(1 << 0)
+
+/*SPI NAND manufacture ID definition*/
+#define SPINAND_MFR_MICRON		0x2C
+int spi_nand_set_cmd_cfg_table(int mfr);
+struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode);
 #endif /* __LINUX_MTD_SPI_NAND_H */
-- 
1.9.1

[PATCH 04/11] nand: spi: Add read function support

[Peter Pan]

Add read/readoob support for SPI NAND. Will be
registered under struct mtd_info later.

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

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index b75e5cd..ee94eb8 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -18,9 +18,62 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/sched.h>
 #include <linux/delay.h>
 #include <linux/mtd/spi-nand.h>
 
+/**
+ * spi_nand_get_device - [GENERIC] Get chip for selected access
+ * @mtd: MTD device structure
+ * @new_state: the state which is requested
+ *
+ * Get the device and lock it for exclusive access
+ */
+static int spi_nand_get_device(struct mtd_info *mtd, int new_state)
+{
+	struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
+	DECLARE_WAITQUEUE(wait, current);
+
+	/*
+	 * Grab the lock and see if the device is available
+	 */
+	while (1) {
+		spin_lock(&this->chip_lock);
+		if (this->state == FL_READY) {
+			this->state = new_state;
+			spin_unlock(&this->chip_lock);
+			break;
+		}
+		if (new_state == FL_PM_SUSPENDED) {
+			spin_unlock(&this->chip_lock);
+			return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
+		}
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		add_wait_queue(&this->wq, &wait);
+		spin_unlock(&this->chip_lock);
+		schedule();
+		remove_wait_queue(&this->wq, &wait);
+	}
+	return 0;
+}
+
+/**
+ * spi_nand_release_device - [GENERIC] release chip
+ * @mtd: MTD device structure
+ *
+ * Deselect, release chip lock and wake up anyone waiting on the device
+ */
+static void spi_nand_release_device(struct mtd_info *mtd)
+{
+	struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
+
+	/* Release the chip */
+	spin_lock(&this->chip_lock);
+	this->state = FL_READY;
+	wake_up(&this->wq);
+	spin_unlock(&this->chip_lock);
+}
+
 static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
 				struct spi_nand_cmd *cmd)
 {
@@ -313,6 +366,36 @@ static int spi_nand_erase_block(struct spi_nand_chip *chip,
 }
 
 /**
+ * spi_nand_wait - wait until the command is done
+ * @chip: SPI-NAND device structure
+ * @s: buffer to store status register(can be NULL)
+ */
+static int spi_nand_wait(struct spi_nand_chip *chip, u8 *s)
+{
+	unsigned long timeo = jiffies;
+	u8 status;
+	int ret = -ETIMEDOUT;
+	int count = 0;
+
+	#define MIN_TRY_COUNT 3
+	timeo += msecs_to_jiffies(20);
+
+	while (time_before(jiffies, timeo) || count < MIN_TRY_COUNT) {
+		spi_nand_read_status(chip, &status);
+		if ((status & STATUS_OIP_MASK) == STATUS_READY) {
+			ret = 0;
+			goto out;
+		}
+		count++;
+	}
+out:
+	if (s)
+		*s = status;
+
+	return ret;
+}
+
+/**
  * spi_nand_read_id - send 9Fh command to get ID
  * @chip: SPI-NAND device structure
  * @buf: buffer to store id
@@ -363,6 +446,244 @@ static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
 	return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
 }
 
+/**
+ * spi_nand_do_read_page - read page from flash to buffer
+ * @mtd: MTD device structure
+ * @page_addr: page address/raw address
+ * @column: column address
+ * @ecc_off: without ecc or not
+ * @corrected: how many bit error corrected
+ * @buf: data buffer
+ * @len: data length to read
+ */
+static int spi_nand_do_read_page(struct mtd_info *mtd, u32 page_addr,
+			bool ecc_off, int *corrected, bool oob_only)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret, ecc_error = 0;
+	u8 status;
+
+	spi_nand_read_page_to_cache(chip, page_addr);
+	ret = spi_nand_wait(chip, &status);
+	if (ret < 0) {
+		pr_err("error %d waiting page 0x%x to cache\n",
+			ret, page_addr);
+		return ret;
+	}
+	if (!oob_only)
+		spi_nand_read_from_cache(chip, page_addr, 0,
+		nand_page_size(nand) + nand_per_page_oobsize(nand), chip->buf);
+	else
+		spi_nand_read_from_cache(chip, page_addr, nand_page_size(nand),
+				nand_per_page_oobsize(nand), chip->oobbuf);
+	if (!ecc_off) {
+		chip->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) {
+			pr_err("internal ECC error reading page 0x%x\n",
+				page_addr);
+			mtd->ecc_stats.failed++;
+		} else if (*corrected) {
+			mtd->ecc_stats.corrected += *corrected;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * spi_nand_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 void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
+				  struct mtd_oob_ops *ops, size_t len)
+{
+	struct mtd_info *mtd = spi_nand_to_mtd(chip);
+	int ret;
+
+	switch (ops->mode) {
+
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_RAW:
+		memcpy(oob, chip->oobbuf + ops->ooboffs, len);
+		return;
+
+	case MTD_OPS_AUTO_OOB:
+		ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
+						  ops->ooboffs, len);
+		BUG_ON(ret);
+		return;
+
+	default:
+		BUG();
+	}
+}
+
+/**
+ * spi_nand_read_pages - read data from flash to buffer
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @ops: oob operations description structure
+ * @max_bitflips: maximum bitflip count
+ * Description:
+ *   Normal read function, read one page to buffer before issue
+ *   another.
+ */
+static int spi_nand_read_pages(struct mtd_info *mtd, loff_t from,
+			  struct mtd_oob_ops *ops, unsigned int *max_bitflips)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int page_addr, page_offset, size, ret;
+	unsigned int corrected = 0;
+	int readlen = ops->len;
+	int oobreadlen = ops->ooblen;
+	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 == NULL;
+
+	page_addr = nand_offs_to_page(nand, from);
+	page_offset = from & (nand_page_size(nand) - 1);
+	ops->retlen = 0;
+	*max_bitflips = 0;
+
+	while (1) {
+		ret = spi_nand_do_read_page(mtd, page_addr, ecc_off,
+						&corrected, oob_only);
+		if (ret)
+			break;
+		*max_bitflips = max(*max_bitflips, corrected);
+		if (ops->datbuf) {
+			size = min_t(int, readlen, nand_page_size(nand) - page_offset);
+			memcpy(ops->datbuf + ops->retlen,
+				chip->buf + page_offset, size);
+			ops->retlen += size;
+			readlen -= size;
+			page_offset = 0;
+		}
+		if (ops->oobbuf) {
+			size = min(oobreadlen, ooblen);
+			spi_nand_transfer_oob(chip,
+				ops->oobbuf + ops->oobretlen, ops, size);
+			ops->oobretlen += size;
+			oobreadlen -= size;
+		}
+		if (!readlen && !oobreadlen)
+			break;
+		page_addr++;
+	}
+
+	return ret;
+}
+
+/**
+ * spi_nand_do_read_ops - read data from flash to buffer
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @ops: oob ops structure
+ * Description:
+ *   Disable internal ECC before reading when MTD_OPS_RAW set.
+ */
+static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
+			  struct mtd_oob_ops *ops)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	int ret;
+	struct mtd_ecc_stats stats;
+	unsigned int max_bitflips = 0;
+	int oobreadlen = ops->ooblen;
+	bool ecc_off = ops->mode == MTD_OPS_RAW;
+	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
+		mtd->oobavail : mtd->oobsize;
+
+	if (oobreadlen > 0) {
+		ooblen -= ops->ooboffs;
+		ops->oobretlen = 0;
+	}
+	stats = mtd->ecc_stats;
+	if (ecc_off)
+		spi_nand_disable_ecc(chip);
+	ret = spi_nand_read_pages(mtd, from, ops, &max_bitflips);
+	if (ecc_off)
+		spi_nand_enable_ecc(chip);
+	if (ret)
+		return ret;
+
+	if (mtd->ecc_stats.failed - stats.failed)
+		return -EBADMSG;
+
+	return max_bitflips;
+}
+
+/**
+ * spi_nand_read - [MTD Interface] SPI-NAND read
+ * @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 spi_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
+	size_t *retlen, u8 *buf)
+{
+	struct mtd_oob_ops ops;
+	int ret;
+
+	spi_nand_get_device(mtd, FL_READING);
+
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ret = spi_nand_do_read_ops(mtd, from, &ops);
+
+	*retlen = ops.retlen;
+
+	spi_nand_release_device(mtd);
+
+	return ret;
+}
+
+/**
+ * spi_nand_read_oob - [MTD Interface] read data and/or out-of-band
+ * @mtd: MTD device structure
+ * @from: offset to read from
+ * @ops: oob operation description structure
+ */
+static int spi_nand_read_oob(struct mtd_info *mtd, loff_t from,
+			struct mtd_oob_ops *ops)
+{
+	int ret = -ENOTSUPP;
+
+	ops->retlen = 0;
+	spi_nand_get_device(mtd, FL_READING);
+
+	switch (ops->mode) {
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
+	case MTD_OPS_RAW:
+		break;
+
+	default:
+		goto out;
+	}
+
+	ret = spi_nand_do_read_ops(mtd, from, ops);
+
+out:
+	spi_nand_release_device(mtd);
+
+	return ret;
+}
 MODULE_DESCRIPTION("SPI NAND framework");
 MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
index 1fcbad7..b61045b 100644
--- a/include/linux/mtd/spi-nand.h
+++ b/include/linux/mtd/spi-nand.h
@@ -132,6 +132,7 @@
  * @read_cache_op:	[REPLACEABLE] Opcode of read from cache
  * @write_cache_op:	[REPLACEABLE] Opcode of program load
  * @command_fn:		[BOARDSPECIFIC] function to handle command transfer
+ * @get_ecc_status:	[REPLACEABLE] get ecc and bitflip status
  * @buf:		[INTERN] buffer for read/write data
  * @oobbuf:		[INTERN] buffer for read/write oob
  * @controller_caps:	[INTERN] capacities of SPI NAND controller
@@ -153,6 +154,8 @@ struct spi_nand_chip {
 	u8		write_cache_op;
 	int (*command_fn)(struct spi_nand_chip *this,
 			struct spi_nand_cmd *cmd);
+	void (*get_ecc_status)(struct spi_nand_chip *this, unsigned int status,
+			unsigned int *corrected, unsigned int *ecc_errors);
 	u8		*buf;
 	u8		*oobbuf;
 	u32		controller_caps;
-- 
1.9.1

[PATCH 05/11] nand: spi: Add write function support

[Peter Pan]

Add write/writeoob support for SPI NAND. Will be
registered under struct mtd_info later.

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

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index ee94eb8..d0cf552 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -497,6 +497,44 @@ static int spi_nand_do_read_page(struct mtd_info *mtd, u32 page_addr,
 }
 
 /**
+ * spi_nand_do_write_page - write data from buffer to flash
+ * @mtd: MTD device structure
+ * @page_addr: page address/raw address
+ * @column: column address
+ * @buf: data buffer
+ * @len: data length to write
+ * @clr_cache: clear cache register with 0xFF or not
+ */
+static int spi_nand_do_write_page(struct mtd_info *mtd, u32 page_addr,
+						bool oob_only)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	u8 status;
+	int ret = 0;
+
+	spi_nand_write_enable(chip);
+	if (!oob_only)
+		spi_nand_program_data_to_cache(chip, page_addr, 0,
+		nand_page_size(nand) + nand_per_page_oobsize(nand), chip->buf);
+	else
+		spi_nand_program_data_to_cache(chip, page_addr, nand_page_size(nand),
+				nand_per_page_oobsize(nand), chip->oobbuf);
+	spi_nand_program_execute(chip, page_addr);
+	ret = spi_nand_wait(chip, &status);
+	if (ret < 0) {
+		pr_err("error %d reading page 0x%x from cache\n",
+			ret, page_addr);
+		return ret;
+	}
+	if ((status & STATUS_P_FAIL_MASK) == STATUS_P_FAIL) {
+		pr_err("program page 0x%x failed\n", page_addr);
+		ret = -EIO;
+	}
+	return ret;
+}
+
+/**
  * spi_nand_transfer_oob - transfer oob to client buffer
  * @chip: SPI-NAND device structure
  * @oob: oob destination address
@@ -528,6 +566,39 @@ static void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
 }
 
 /**
+ * spi_nand_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 void spi_nand_fill_oob(struct spi_nand_chip *chip, uint8_t *oob,
+				size_t len, struct mtd_oob_ops *ops)
+{
+	struct mtd_info *mtd = spi_nand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret;
+
+	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);
+		return;
+
+	case MTD_OPS_AUTO_OOB:
+		ret = mtd_ooblayout_set_databytes(mtd, oob, chip->oobbuf,
+						  ops->ooboffs, len);
+		BUG_ON(ret);
+		return;
+
+	default:
+		BUG();
+	}
+}
+
+/**
  * spi_nand_read_pages - read data from flash to buffer
  * @mtd: MTD device structure
  * @from: offset to read from
@@ -625,6 +696,102 @@ static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 }
 
 /**
+ * spi_nand_write_pages - write data from buffer to flash
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @ops: oob operations description structure
+ * @max_bitflips: maximum bitflip count
+ * Description:
+ *   Normal read function, read one page to buffer before issue
+ *   another.
+ */
+static int spi_nand_write_pages(struct mtd_info *mtd, loff_t to,
+			  struct mtd_oob_ops *ops)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int page_addr, page_offset, size, ret;
+	int writelen = ops->len;
+	int oobwritelen = ops->ooblen;
+	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
+		mtd->oobavail : mtd->oobsize;
+	bool oob_only = ops->datbuf == NULL;
+
+	page_addr = nand_offs_to_page(nand, to);
+	page_offset = to & (nand_page_size(nand) - 1);
+	ops->retlen = 0;
+
+	while (1) {
+		memset(chip->buf, 0xff, nand_page_size(nand) + nand_per_page_oobsize(nand));
+		if (ops->oobbuf) {
+			size = min(oobwritelen, ooblen);
+			spi_nand_fill_oob(chip, ops->oobbuf + ops->oobretlen,
+					size, ops);
+			ops->oobretlen += size;
+			oobwritelen -= size;
+		}
+		if (ops->datbuf) {
+			size = min_t(int, writelen, nand_page_size(nand) - page_offset);
+			memcpy(chip->buf + page_offset,
+				ops->datbuf + ops->retlen, size);
+			ops->retlen += size;
+			writelen -= size;
+		}
+		ret = spi_nand_do_write_page(mtd, page_addr, oob_only);
+		if (ret) {
+			pr_err("error %d writing page 0x%x\n",
+				ret, page_addr);
+			return ret;
+		}
+		page_offset = 0;
+		if (!writelen && !oobwritelen)
+			break;
+		page_addr++;
+	}
+
+	return ret;
+}
+
+/**
+ * spi_nand_do_write_ops - write data from buffer to flash
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @ops: oob operations description structure
+ * Description:
+ *   Disable internal ECC before writing when MTD_OPS_RAW set.
+ */
+static int spi_nand_do_write_ops(struct mtd_info *mtd, loff_t to,
+			 struct mtd_oob_ops *ops)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int page_addr, page_offset;
+	int oobwritelen = ops->ooblen;
+	int ret = 0;
+	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
+		mtd->oobavail : mtd->oobsize;
+	bool ecc_off = ops->mode == MTD_OPS_RAW;
+
+	page_addr = nand_offs_to_page(nand, to);
+	page_offset = to & (nand_page_size(nand) - 1);
+	ops->retlen = 0;
+
+	/* for oob */
+	if (oobwritelen > 0) {
+		ooblen -= ops->ooboffs;
+		ops->oobretlen = 0;
+	}
+
+	if (ecc_off)
+		spi_nand_disable_ecc(chip);
+	ret = spi_nand_write_pages(mtd, to, ops);
+	if (ecc_off)
+		spi_nand_enable_ecc(chip);
+
+	return ret;
+}
+
+/**
  * spi_nand_read - [MTD Interface] SPI-NAND read
  * @mtd: MTD device structure
  * @from: offset to read from
@@ -654,6 +821,35 @@ static int spi_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
 }
 
 /**
+ * spi_nand_write - [MTD Interface] SPI-NAND write
+ * @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 spi_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
+	size_t *retlen, const u8 *buf)
+{
+	struct mtd_oob_ops ops;
+	int ret;
+
+	spi_nand_get_device(mtd, FL_WRITING);
+
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = (uint8_t *)buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ret =  spi_nand_do_write_ops(mtd, to, &ops);
+
+	*retlen = ops.retlen;
+
+	spi_nand_release_device(mtd);
+
+	return ret;
+}
+
+/**
  * spi_nand_read_oob - [MTD Interface] read data and/or out-of-band
  * @mtd: MTD device structure
  * @from: offset to read from
@@ -684,6 +880,38 @@ static int spi_nand_read_oob(struct mtd_info *mtd, loff_t from,
 
 	return ret;
 }
+
+/**
+ * spi_nand_write_oob - [MTD Interface] write data and/or out-of-band
+ * @mtd: MTD device structure
+ * @to: offset to write to
+ * @ops: oob operation description structure
+ */
+static int spi_nand_write_oob(struct mtd_info *mtd, loff_t to,
+			  struct mtd_oob_ops *ops)
+{
+	int ret = -ENOTSUPP;
+
+	ops->retlen = 0;
+	spi_nand_get_device(mtd, FL_WRITING);
+
+	switch (ops->mode) {
+	case MTD_OPS_PLACE_OOB:
+	case MTD_OPS_AUTO_OOB:
+	case MTD_OPS_RAW:
+		break;
+
+	default:
+		goto out;
+	}
+
+	ret = spi_nand_do_write_ops(mtd, to, ops);
+
+out:
+	spi_nand_release_device(mtd);
+
+	return ret;
+}
 MODULE_DESCRIPTION("SPI NAND framework");
 MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
 MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH 06/11] nand: spi: Add erase function support

[Peter Pan]

Add erase support for SPI NAND. Will be registered
under struct mtd_info later.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/spi/spi-nand-base.c | 85 +++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index d0cf552..5c335e2 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/delay.h>
+#include <linux/jiffies.h>
 #include <linux/mtd/spi-nand.h>
 
 /**
@@ -373,12 +374,15 @@ static int spi_nand_erase_block(struct spi_nand_chip *chip,
 static int spi_nand_wait(struct spi_nand_chip *chip, u8 *s)
 {
 	unsigned long timeo = jiffies;
-	u8 status;
+	u8 status, state = chip->state;
 	int ret = -ETIMEDOUT;
 	int count = 0;
 
 	#define MIN_TRY_COUNT 3
-	timeo += msecs_to_jiffies(20);
+	if (state == FL_ERASING)
+		timeo += msecs_to_jiffies(400);
+	else
+		timeo += msecs_to_jiffies(20);
 
 	while (time_before(jiffies, timeo) || count < MIN_TRY_COUNT) {
 		spi_nand_read_status(chip, &status);
@@ -912,6 +916,83 @@ static int spi_nand_write_oob(struct mtd_info *mtd, loff_t to,
 
 	return ret;
 }
+
+/**
+ * spi_nand_erase - [MTD Interface] erase block(s)
+ * @mtd: MTD device structure
+ * @einfo: erase instruction
+ *
+ * Erase one ore more blocks
+ */
+static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo)
+{
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	loff_t offs = einfo->addr, len = einfo->len;
+	u8 status;
+	int ret = 0;
+
+
+	/* check address align on block boundary */
+	if (offs & (nand_eraseblock_size(nand) - 1)) {
+		pr_err("%s: Unaligned address\n", __func__);
+		return -EINVAL;
+	}
+
+	if (len & (nand_eraseblock_size(nand) - 1)) {
+		pr_err("%s: Length not block aligned\n", __func__);
+		return -EINVAL;
+	}
+
+	/* Do not allow erase past end of device */
+	if ((offs + len) > chip->size) {
+		pr_err("%s: Erase past end of device\n", __func__);
+		return -EINVAL;
+	}
+
+	einfo->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+
+	/* Grab the lock and see if the device is available */
+	spi_nand_get_device(mtd, FL_ERASING);
+
+	einfo->state = MTD_ERASING;
+
+	while (len) {
+		spi_nand_write_enable(chip);
+		spi_nand_erase_block(chip, nand_offs_to_page(nand, offs));
+		ret = spi_nand_wait(chip, &status);
+		if (ret < 0) {
+			pr_err("block erase command wait failed\n");
+			einfo->state = MTD_ERASE_FAILED;
+			goto erase_exit;
+		}
+		if ((status & STATUS_E_FAIL_MASK) == STATUS_E_FAIL) {
+			pr_err("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;
+
+	spi_nand_release_device(mtd);
+
+	/* Do call back function */
+	if (!ret)
+		mtd_erase_callback(einfo);
+
+	/* Return more or less happy */
+	return ret;
+}
 MODULE_DESCRIPTION("SPI NAND framework");
 MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
 MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH 07/11] nand: spi: Add init/release function

[Peter Pan]

Add SPI NAND initialization and release functions.

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

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index 5c335e2..c77721d 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -22,6 +22,17 @@
 #include <linux/delay.h>
 #include <linux/jiffies.h>
 #include <linux/mtd/spi-nand.h>
+#include <linux/slab.h>
+
+static struct spi_nand_flash spi_nand_table[] = {
+	SPI_NAND_INFO("MT29F4G01ABAGD", 0x2C, 0x36, 2048, 128, 64, 2048,
+			2, 8, SPINAND_NEED_PLANE_SELECT,
+			SPINAND_OP_COMMON),
+	SPI_NAND_INFO("MT29F2G01ABAGD", 0x2C, 0x24, 2048, 128, 64, 2048,
+			1, 8, SPINAND_NEED_PLANE_SELECT,
+			SPINAND_OP_COMMON),
+	{.name = NULL},
+};
 
 /**
  * spi_nand_get_device - [GENERIC] Get chip for selected access
@@ -993,6 +1004,215 @@ static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo)
 	/* Return more or less happy */
 	return ret;
 }
+
+/**
+ * spi_nand_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 spi_nand_scan_id_table(struct spi_nand_chip *chip, u8 *id)
+{
+	struct nand_device *nand = &chip->base;
+	struct spi_nand_flash *type = spi_nand_table;
+	struct nand_memory_organization *memorg = &nand->memorg;
+
+	for (; type->name; type++) {
+		if (id[0] == type->mfr_id && id[1] == type->dev_id) {
+			chip->name = type->name;
+			chip->size = type->page_size * type->pages_per_blk
+				* type->blks_per_lun * type->luns_per_chip;
+			memorg->eraseblocksize = type->page_size
+					* type->pages_per_blk;
+			memorg->pagesize = type->page_size;
+			memorg->oobsize = type->oob_size;
+			memorg->diesize =
+				memorg->eraseblocksize * type->blks_per_lun;
+			memorg->ndies = type->luns_per_chip;
+			chip->ecc_strength = type->ecc_strength;
+			chip->options = type->options;
+			chip->rw_mode = type->rw_mode;
+
+			return true;
+		}
+	}
+
+	return false;
+}
+
+/**
+ * spi_nand_set_rd_wr_op - Chose the best read write command
+ * @chip: SPI-NAND device structure
+ * Description:
+ *   Chose the fastest r/w command according to spi controller's ability.
+ * Note:
+ *   If 03h/0Bh follows SPI NAND protocol, there is no difference,
+ *   while if follows SPI NOR protocol, 03h command is working under
+ *   <=20Mhz@3.3V,<=5MHz@1.8V; 0Bh command is working under
+ *   133Mhz@3.3v, 83Mhz@1.8V.
+ */
+static void spi_nand_set_rd_wr_op(struct spi_nand_chip *chip)
+{
+	u32 mode = SPINAND_OP_MASK & chip->controller_caps & chip->rw_mode;
+
+	if (mode & SPINAND_RD_QUAD)
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_QUAD_IO;
+	else if (mode & SPINAND_RD_X4)
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_X4;
+	else if (mode & SPINAND_RD_DUAL)
+		chip->read_cache_op = SPINAND_CMD_READ_FROM_CACHE_DUAL_IO;
+	else if (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 (mode & SPINAND_WR_X4)
+		chip->write_cache_op = SPINAND_CMD_PROG_LOAD_X4;
+	else
+		chip->write_cache_op = SPINAND_CMD_PROG_LOAD;
+}
+
+/**
+ * spi_nand_scan_ident - [SPI-NAND Interface] Scan for the SPI-NAND device
+ * @chip: SPI-NAND device structure
+ * Description:
+ *   This is the first phase of the initiazation. It reads the flash ID and
+ *   sets up spi_nand_chip fields accordingly.
+ */
+int spi_nand_scan_ident(struct spi_nand_chip *chip)
+{
+	struct nand_device *nand = &chip->base;
+	u8 id[SPINAND_MAX_ID_LEN] = {0};
+	int id_retry = 2;
+
+	spi_nand_reset(chip);
+read_id:
+	spi_nand_read_id(chip, id);
+	if (spi_nand_scan_id_table(chip, id))
+		goto ident_done;
+	if (id_retry--)
+		goto read_id;
+	pr_info("SPI-NAND type mfr_id: %x, dev_id: %x is not in id table.\n",
+				id[0], id[1]);
+
+	return -ENODEV;
+
+ident_done:
+	pr_info("SPI-NAND: %s is found.\n", chip->name);
+	pr_info("Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n", id[0], id[1]);
+	pr_info("%d MiB, block size: %d KiB, page size: %d, OOB size: %d\n",
+		(int)(chip->size >> 20), nand_eraseblock_size(nand) >> 10,
+		nand_page_size(nand), nand_per_page_oobsize(nand));
+
+	chip->mfr_id = id[0];
+	chip->dev_id = id[1];
+	spi_nand_set_rd_wr_op(chip);
+
+	chip->buf = kzalloc(nand_page_size(nand) + nand_per_page_oobsize(nand), GFP_KERNEL);
+	if (!chip->buf)
+		return -ENOMEM;
+
+	chip->oobbuf = chip->buf + nand_page_size(nand);
+	spi_nand_lock_block(chip, BL_ALL_UNLOCKED);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_nand_scan_ident);
+
+/**
+ * spi_nand_scan_tail - [SPI-NAND Interface] Scan for the SPI-NAND device
+ * @chip: SPI-NAND device structure
+ * Description:
+ *   This is the second phase of the initiazation. It fills out all the
+ *   uninitialized fields of spi_nand_chip and mtd fields.
+ */
+int spi_nand_scan_tail(struct spi_nand_chip *chip)
+{
+	struct mtd_info *mtd = spi_nand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	int ret;
+
+	/* Initialize state */
+	chip->state = FL_READY;
+
+	init_waitqueue_head(&chip->wq);
+	spin_lock_init(&chip->chip_lock);
+
+	mtd->name = chip->name;
+	mtd->size = chip->size;
+	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 = chip->ecc_strength ?
+				chip->ecc_strength : 1;
+
+	mtd->oobsize = nand_per_page_oobsize(nand);
+	ret = mtd_ooblayout_count_freebytes(mtd);
+	if (ret < 0)
+		ret = 0;
+	mtd->oobavail = ret;
+	mtd->_erase = spi_nand_erase;
+	mtd->_read = spi_nand_read;
+	mtd->_write = spi_nand_write;
+	mtd->_read_oob = spi_nand_read_oob;
+	mtd->_write_oob = spi_nand_write_oob;
+
+	if (!mtd->bitflip_threshold)
+		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3, 4);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_nand_scan_tail);
+
+/**
+ * spi_nand_scan_ident_release - [SPI-NAND Interface] Free resources
+ * applied by spi_nand_scan_ident
+ * @chip: SPI-NAND device structure
+ */
+int spi_nand_scan_ident_release(struct spi_nand_chip *chip)
+{
+	kfree(chip->buf);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_nand_scan_ident_release);
+
+/**
+ * spi_nand_scan_tail_release - [SPI-NAND Interface] Free resources
+ * applied by spi_nand_scan_tail
+ * @chip: SPI-NAND device structure
+ */
+int spi_nand_scan_tail_release(struct spi_nand_chip *chip)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_nand_scan_tail_release);
+
+/**
+ * spi_nand_release - [SPI-NAND Interface] Free resources held by the SPI-NAND
+ * device
+ * @chip: SPI-NAND device structure
+ */
+int spi_nand_release(struct spi_nand_chip *chip)
+{
+	struct mtd_info *mtd = spi_nand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+	struct nand_bbt_descr *bd = nand->bbt.bbp;
+
+	mtd_device_unregister(mtd);
+	kfree(chip->buf);
+	kfree(nand->bbt.bbt);
+	if (bd->options & NAND_BBT_DYNAMICSTRUCT)
+		kfree(bd);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_nand_release);
+
 MODULE_DESCRIPTION("SPI NAND framework");
 MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
index b61045b..cdacec4 100644
--- a/include/linux/mtd/spi-nand.h
+++ b/include/linux/mtd/spi-nand.h
@@ -116,6 +116,25 @@
 #define SPI_NAND_MT29F_ECC_UNCORR	0x20
 
 
+/* SPI NAND controller and chip supported OP mode */
+#define SPINAND_OP_MASK		0x0000ffff
+#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 spi_nand_cmd;
 
 /**
@@ -136,6 +155,7 @@
  * @buf:		[INTERN] buffer for read/write data
  * @oobbuf:		[INTERN] buffer for read/write oob
  * @controller_caps:	[INTERN] capacities of SPI NAND controller
+ * @rw_mode:		[BOARDSPECIFIC] read/write mode of SPI NAND chip
  * @size:		[INTERN] the size of chip
  * @options:		[BOARDSPECIFIC] various chip options. They can partly
  *			be set to inform nand_scan about special functionality.
@@ -159,6 +179,7 @@ struct spi_nand_chip {
 	u8		*buf;
 	u8		*oobbuf;
 	u32		controller_caps;
+	u32		rw_mode;
 	u64		size;
 	u32		options;
 	u32		ecc_strength;
@@ -186,6 +207,20 @@ static inline void spi_nand_set_controller_data(struct spi_nand_chip *chip,
 	chip->priv = priv;
 }
 
+struct spi_nand_flash {
+	char		*name;
+	u8		mfr_id;
+	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		options;
+	u32		rw_mode;
+};
+
 #define SPINAND_MAX_ADDR_LEN		4
 
 struct spi_nand_cmd {
@@ -206,11 +241,28 @@ struct spi_nand_cmd_cfg {
 	u8		data_bits;
 };
 
+#define SPI_NAND_INFO(nm, mid, did, pagesz, oobsz, pg_per_blk,\
+	blk_per_lun, lun_per_chip, ecc_stren, opts, rwmode)	\
+	{ .name = (nm), .mfr_id = (mid), .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),\
+	.options = (opts), .rw_mode = (rwmode) }
+
 /*SPI NAND chip options*/
 #define SPINAND_NEED_PLANE_SELECT	(1 << 0)
 
 /*SPI NAND manufacture ID definition*/
 #define SPINAND_MFR_MICRON		0x2C
+
+#define SPINAND_MAX_ID_LEN		2
+
+
+int spi_nand_scan_ident(struct spi_nand_chip *chip);
+int spi_nand_scan_tail(struct spi_nand_chip *chip);
+int spi_nand_scan_ident_release(struct spi_nand_chip *chip);
+int spi_nand_scan_tail_release(struct spi_nand_chip *chip);
+int spi_nand_release(struct spi_nand_chip *chip);
 int spi_nand_set_cmd_cfg_table(int mfr);
 struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode);
 #endif /* __LINUX_MTD_SPI_NAND_H */
-- 
1.9.1

[PATCH 08/11] 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.

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

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index c77721d..57986d5 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -34,6 +34,8 @@
 	{.name = NULL},
 };
 
+static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo);
+
 /**
  * spi_nand_get_device - [GENERIC] Get chip for selected access
  * @mtd: MTD device structure
@@ -929,6 +931,115 @@ static int spi_nand_write_oob(struct mtd_info *mtd, loff_t to,
 }
 
 /**
+ * spi_nand_block_bad - Check if block at offset is bad
+ * @mtd: MTD device structure
+ * @offs: offset relative to mtd start
+ * @getchip: 0, if the chip is already selected
+ */
+static int spi_nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
+{
+	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;
+
+	block_addr = nand_offs_to_eraseblock(nand, ofs);
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ops.ooblen = 2;
+	ops.oobbuf = bad;
+
+	if (getchip)
+		spi_nand_get_device(mtd, FL_READING);
+	spi_nand_do_read_ops(mtd, nand_eraseblock_to_offs(nand, block_addr), &ops);
+	if (getchip)
+		spi_nand_release_device(mtd);
+	if (bad[0] != 0xFF || bad[1] != 0xFF)
+		ret =  1;
+
+	return ret;
+}
+
+
+/**
+ * spi_nand_block_isbad - [MTD Interface] Check if block at offset is bad
+ * @mtd: MTD device structure
+ * @offs: offset relative to mtd start
+ */
+static int spi_nand_block_isbad(struct mtd_info *mtd, loff_t offs)
+{
+	return spi_nand_block_bad(mtd, offs, 1);
+}
+
+/**
+ * spi_nand_block_markbad_lowlevel - mark a block bad
+ * @mtd: MTD device structure
+ * @ofs: offset from device start
+ *
+ * This function performs the generic bad block marking steps (i.e., bad
+ * block table(s) and/or marker(s)). We only allow the hardware driver to
+ * specify how to write bad block markers to OOB (chip->block_markbad).
+ *
+ * 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
+ * Note that we retain the first error encountered in (2) or (3), finish the
+ * procedures, and dump the error in the end.
+*/
+static int spi_nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
+{
+	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 ret = 0;
+
+	/*erase bad block before mark bad block*/
+	einfo.mtd = mtd;
+	einfo.addr = ofs;
+	einfo.len = nand_eraseblock_size(nand);
+	spi_nand_erase(mtd, &einfo);
+
+	block_addr = nand_offs_to_eraseblock(nand, ofs);
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ops.ooblen = 2;
+	ops.oobbuf = buf;
+	spi_nand_get_device(mtd, FL_WRITING);
+	ret = spi_nand_do_write_ops(mtd,
+		nand_eraseblock_to_offs(nand, block_addr), &ops);
+	spi_nand_release_device(mtd);
+
+	if (!ret)
+		mtd->ecc_stats.badblocks++;
+
+	return ret;
+}
+
+/**
+ * spi_nand_block_markbad - [MTD Interface] Mark block at the given offset
+ * as bad
+ * @mtd: MTD device structure
+ * @ofs: offset relative to mtd start
+ */
+static int spi_nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+	int ret;
+
+	ret = spi_nand_block_isbad(mtd, ofs);
+	if (ret) {
+		/* If it was bad already, return success and do nothing */
+		if (ret > 0)
+			return 0;
+		return ret;
+	}
+
+	return spi_nand_block_markbad_lowlevel(mtd, ofs);
+}
+
+/**
  * spi_nand_erase - [MTD Interface] erase block(s)
  * @mtd: MTD device structure
  * @einfo: erase instruction
@@ -969,6 +1080,13 @@ static int spi_nand_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 (spi_nand_block_bad(mtd, offs, 0)) {
+			pr_warn("%s: attempt to erase a bad block at 0x%012llx\n",
+			__func__, offs);
+			einfo->state = MTD_ERASE_FAILED;
+			goto erase_exit;
+		}
 		spi_nand_write_enable(chip);
 		spi_nand_erase_block(chip, nand_offs_to_page(nand, offs));
 		ret = spi_nand_wait(chip, &status);
@@ -1161,6 +1279,8 @@ int spi_nand_scan_tail(struct spi_nand_chip *chip)
 	mtd->_write = spi_nand_write;
 	mtd->_read_oob = spi_nand_read_oob;
 	mtd->_write_oob = spi_nand_write_oob;
+	mtd->_block_isbad = spi_nand_block_isbad;
+	mtd->_block_markbad = spi_nand_block_markbad;
 
 	if (!mtd->bitflip_threshold)
 		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3, 4);
-- 
1.9.1

[PATCH 09/11] nand: spi: Add BBT support

[Peter Pan]

This commit enables BBT for SPI NAND. We also define
SPINAND_SKIP_BBTSCAN to skip to use BBT for some
chipset vendors' request.

Signed-off-by: Peter Pan <peterpandong@micron.com>
---
 drivers/mtd/nand/spi/spi-nand-base.c | 195 +++++++++++++++++++++++++++++++----
 include/linux/mtd/spi-nand.h         |   1 +
 2 files changed, 176 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index 57986d5..5ac4b26 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -960,6 +960,27 @@ static int spi_nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
 	return ret;
 }
 
+/**
+ * spi_nand_block_checkbad - Check if a block is marked bad
+ * @mtd: MTD device structure
+ * @ofs: offset from device start
+ * @getchip: 0, if the chip is already selected
+ * @allowbbt: 1, if its allowed to access the bbt area
+ *
+ * Check, if the block is bad. Either by reading the bad block table or
+ * calling of the scan function.
+ */
+static int spi_nand_block_checkbad(struct mtd_info *mtd, loff_t ofs,
+			int getchip, int allowbbt)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+
+	if (!nand->bbt.bbt)
+		return spi_nand_block_bad(mtd, ofs, getchip);
+
+	/* Return info from the table */
+	return nand_isbad_bbt(nand, ofs, allowbbt);
+}
 
 /**
  * spi_nand_block_isbad - [MTD Interface] Check if block at offset is bad
@@ -968,7 +989,7 @@ static int spi_nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
  */
 static int spi_nand_block_isbad(struct mtd_info *mtd, loff_t offs)
 {
-	return spi_nand_block_bad(mtd, offs, 1);
+	return spi_nand_block_checkbad(mtd, offs, 1, 0);
 }
 
 /**
@@ -995,22 +1016,31 @@ static int spi_nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
 	struct erase_info einfo = {0};
 	u32 block_addr;
 	u8 buf[2] = {0, 0};
-	int ret = 0;
-
-	/*erase bad block before mark bad block*/
-	einfo.mtd = mtd;
-	einfo.addr = ofs;
-	einfo.len = nand_eraseblock_size(nand);
-	spi_nand_erase(mtd, &einfo);
+	int res, ret = 0;
+
+	if (!nand->bbt.bbt || !(nand->bbt.options & NAND_BBT_NO_OOB_BBM)) {
+		/*erase bad block before mark bad block*/
+		einfo.mtd = mtd;
+		einfo.addr = ofs;
+		einfo.len = nand_eraseblock_size(nand);
+		spi_nand_erase(mtd, &einfo);
+
+		block_addr = nand_offs_to_eraseblock(nand, ofs);
+		ops.mode = MTD_OPS_PLACE_OOB;
+		ops.ooblen = 2;
+		ops.oobbuf = buf;
+		spi_nand_get_device(mtd, FL_WRITING);
+		ret = spi_nand_do_write_ops(mtd,
+			nand_eraseblock_to_offs(nand, block_addr), &ops);
+		spi_nand_release_device(mtd);
+	}
 
-	block_addr = nand_offs_to_eraseblock(nand, ofs);
-	ops.mode = MTD_OPS_PLACE_OOB;
-	ops.ooblen = 2;
-	ops.oobbuf = buf;
-	spi_nand_get_device(mtd, FL_WRITING);
-	ret = spi_nand_do_write_ops(mtd,
-		nand_eraseblock_to_offs(nand, block_addr), &ops);
-	spi_nand_release_device(mtd);
+	/* Mark block bad in BBT */
+	if (nand->bbt.bbt) {
+		res = nand_markbad_bbt(nand, ofs);
+		if (!ret)
+			ret = res;
+	}
 
 	if (!ret)
 		mtd->ecc_stats.badblocks++;
@@ -1040,13 +1070,15 @@ static int spi_nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
 }
 
 /**
- * spi_nand_erase - [MTD Interface] erase block(s)
+ * __spi_nand_erase - erase block(s)
  * @mtd: MTD device structure
  * @einfo: erase instruction
+ * @allowbbt: allow to access bbt
  *
  * Erase one ore more blocks
  */
-static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo)
+static int __spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo,
+			int allowbbt)
 {
 	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
 	struct nand_device *nand = mtd_to_nand(mtd);
@@ -1081,7 +1113,7 @@ static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo)
 
 	while (len) {
 		/* Check if we have a bad block, we do not erase bad blocks! */
-		if (spi_nand_block_bad(mtd, offs, 0)) {
+		if (spi_nand_block_checkbad(mtd, offs, 0, allowbbt)) {
 			pr_warn("%s: attempt to erase a bad block at 0x%012llx\n",
 			__func__, offs);
 			einfo->state = MTD_ERASE_FAILED;
@@ -1124,6 +1156,55 @@ static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo)
 }
 
 /**
+ * spi_nand_erase - [MTD Interface] erase block(s)
+ * @mtd: MTD device structure
+ * @einfo: erase instruction
+ *
+ * Erase one ore more blocks
+ */
+static int spi_nand_erase(struct mtd_info *mtd, struct erase_info *einfo)
+{
+	return __spi_nand_erase(mtd, einfo, 0);
+}
+
+
+static int spi_nand_erase_bbt(struct nand_device *nand, struct erase_info *einfo)
+{
+	return __spi_nand_erase(nand_to_mtd(nand), einfo, 1);
+}
+
+static int spi_nand_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 spi_nand_do_write_ops(nand_to_mtd(nand),
+		nand_eraseblock_to_offs(nand, block), &ops);
+
+}
+
+/**
+ * spi_nand_block_isreserved - [MTD Interface] Check if a block is
+ * marked reserved.
+ * @mtd: MTD device structure
+ * @ofs: offset from device start
+ */
+static int spi_nand_block_isreserved(struct mtd_info *mtd, loff_t ofs)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+
+	if (!nand->bbt.bbt)
+		return 0;
+	/* Return info from the table */
+	return nand_isreserved_bbt(nand, ofs);
+}
+
+/**
  * spi_nand_scan_id_table - scan chip info in id table
  * @chip: SPI-NAND device structure
  * @id: point to manufacture id and device id
@@ -1191,6 +1272,74 @@ static void spi_nand_set_rd_wr_op(struct spi_nand_chip *chip)
 		chip->write_cache_op = SPINAND_CMD_PROG_LOAD;
 }
 
+static const struct nand_ops spi_nand_ops = {
+	.erase = spi_nand_erase_bbt,
+	.markbad = spi_nand_markbad,
+};
+
+/*
+ * Define some generic bad / good block scan pattern which are used
+ * while scanning a device for factory marked good / bad blocks.
+ */
+static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
+
+#define BADBLOCK_SCAN_MASK (~NAND_BBT_NO_OOB)
+
+/**
+ * nand_create_badblock_pattern - [INTERN] Creates a BBT descriptor structure
+ * @this: NAND chip to create descriptor for
+ * @chip: SPI-NAND device structure
+ *
+ * This function allocates and initializes a nand_bbt_descr for BBM detection
+ * based on the properties of @this. The new descriptor is stored in
+ * this->badblock_pattern. Thus, this->badblock_pattern should be NULL when
+ * passed to this function.
+ */
+static int spi_nand_create_badblock_pattern(struct spi_nand_chip *chip)
+{
+	struct nand_device *nand = &chip->base;
+	struct nand_bbt_descr *bd;
+
+	if (nand->bbt.bbp) {
+		pr_warn("Bad block pattern already allocated; not replacing\n");
+		return -EINVAL;
+	}
+	bd = kzalloc(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;
+}
+
+static int spi_nand_default_bbt(struct mtd_info *mtd)
+{
+	struct nand_device *nand = mtd_to_nand(mtd);
+	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	int ret;
+
+	nand->bbt.options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
+	nand->bbt.td = NULL;
+	nand->bbt.md = NULL;
+
+	ret = spi_nand_create_badblock_pattern(chip);
+	if (ret)
+		return ret;
+
+	return nand_scan_bbt(nand);
+}
+
+static void spi_nand_fill_nandd(struct spi_nand_chip *chip)
+{
+	struct nand_device *nand = &chip->base;
+
+	nand->ops = &spi_nand_ops;
+}
+
 /**
  * spi_nand_scan_ident - [SPI-NAND Interface] Scan for the SPI-NAND device
  * @chip: SPI-NAND device structure
@@ -1232,6 +1381,7 @@ int spi_nand_scan_ident(struct spi_nand_chip *chip)
 		return -ENOMEM;
 
 	chip->oobbuf = chip->buf + nand_page_size(nand);
+	spi_nand_fill_nandd(chip);
 	spi_nand_lock_block(chip, BL_ALL_UNLOCKED);
 
 	return 0;
@@ -1281,11 +1431,16 @@ int spi_nand_scan_tail(struct spi_nand_chip *chip)
 	mtd->_write_oob = spi_nand_write_oob;
 	mtd->_block_isbad = spi_nand_block_isbad;
 	mtd->_block_markbad = spi_nand_block_markbad;
+	mtd->_block_isreserved = spi_nand_block_isreserved;
 
 	if (!mtd->bitflip_threshold)
 		mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3, 4);
 
-	return 0;
+	/* Check, if we should skip the bad block table scan */
+	if (chip->options & SPINAND_SKIP_BBTSCAN)
+		return 0;
+	/* Build bad block table */
+	return spi_nand_default_bbt(mtd);
 }
 EXPORT_SYMBOL_GPL(spi_nand_scan_tail);
 
diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
index cdacec4..5f362b6 100644
--- a/include/linux/mtd/spi-nand.h
+++ b/include/linux/mtd/spi-nand.h
@@ -251,6 +251,7 @@ struct spi_nand_cmd_cfg {
 
 /*SPI NAND chip options*/
 #define SPINAND_NEED_PLANE_SELECT	(1 << 0)
+#define SPINAND_SKIP_BBTSCAN		(1 << 3)
 
 /*SPI NAND manufacture ID definition*/
 #define SPINAND_MFR_MICRON		0x2C
-- 
1.9.1

[PATCH 10/11] 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/chips/Kconfig       |   5 +
 drivers/mtd/nand/spi/chips/Makefile      |   1 +
 drivers/mtd/nand/spi/chips/generic_spi.c | 269 +++++++++++++++++++++++++++++++
 5 files changed, 278 insertions(+)
 create mode 100644 drivers/mtd/nand/spi/chips/Kconfig
 create mode 100644 drivers/mtd/nand/spi/chips/Makefile
 create mode 100644 drivers/mtd/nand/spi/chips/generic_spi.c

diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
index 04a7b71..07ebbb0 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/chips/Kconfig"
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index 3c617d6..f0775d2 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-base.o
 obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-cmd.o
+obj-$(CONFIG_MTD_SPI_NAND) += chips/
diff --git a/drivers/mtd/nand/spi/chips/Kconfig b/drivers/mtd/nand/spi/chips/Kconfig
new file mode 100644
index 0000000..337a94f
--- /dev/null
+++ b/drivers/mtd/nand/spi/chips/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/chips/Makefile b/drivers/mtd/nand/spi/chips/Makefile
new file mode 100644
index 0000000..2cb7763
--- /dev/null
+++ b/drivers/mtd/nand/spi/chips/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_GENERIC_SPI_NAND) += generic_spi.o
diff --git a/drivers/mtd/nand/spi/chips/generic_spi.c b/drivers/mtd/nand/spi/chips/generic_spi.c
new file mode 100644
index 0000000..38d8613
--- /dev/null
+++ b/drivers/mtd/nand/spi/chips/generic_spi.c
@@ -0,0 +1,269 @@
+/*
+ * 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/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/spi/spi.h>
+#include <linux/mtd/spi-nand.h>
+
+static int micron_ooblayout_ecc_64(struct mtd_info *mtd, int section,
+				 struct mtd_oob_region *oobregion)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	oobregion->length = 8;
+	oobregion->offset = (section * 2 + 1) * 8;
+
+	return 0;
+}
+
+static int micron_ooblayout_free_64(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *oobregion)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	oobregion->length = section ? 8 : 6;
+	oobregion->offset = section ? section * 16 : 2;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops micron_ooblayout_64_ops = {
+	.ecc = micron_ooblayout_ecc_64,
+	.free = micron_ooblayout_free_64,
+};
+
+static int micron_ooblayout_ecc_128(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 micron_ooblayout_free_128(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 micron_ooblayout_128_ops = {
+	.ecc = micron_ooblayout_ecc_128,
+	.free = micron_ooblayout_free_128,
+};
+
+static void spi_nand_mt29f_ecc_status(struct spi_nand_chip *chip,
+					unsigned int status,
+					unsigned int *corrected,
+					unsigned int *ecc_error)
+{
+	unsigned int ecc_status = status & SPI_NAND_MT29F_ECC_MASK;
+
+	*ecc_error = (ecc_status == SPI_NAND_MT29F_ECC_UNCORR);
+	switch (ecc_status) {
+	case SPI_NAND_MT29F_ECC_0_BIT:
+		*corrected = 0;
+		break;
+	case SPI_NAND_MT29F_ECC_1_3_BIT:
+		*corrected = 3;
+		break;
+	case SPI_NAND_MT29F_ECC_4_6_BIT:
+		*corrected = 6;
+		break;
+	case SPI_NAND_MT29F_ECC_7_8_BIT:
+		*corrected = 8;
+		break;
+	}
+}
+
+static int spi_nand_manufacture_init(struct spi_nand_chip *chip)
+{
+	struct mtd_info *mtd = spi_nand_to_mtd(chip);
+	struct nand_device *nand = mtd_to_nand(mtd);
+
+	switch (chip->mfr_id) {
+	case SPINAND_MFR_MICRON:
+		chip->get_ecc_status = spi_nand_mt29f_ecc_status;
+		if (nand_per_page_oobsize(nand) == 64)
+			mtd_set_ooblayout(mtd, &micron_ooblayout_64_ops);
+		else if (nand_per_page_oobsize(nand) == 128)
+			mtd_set_ooblayout(mtd, &micron_ooblayout_128_ops);
+
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+/*
+ * generic_spi_nand_cmd_fn - to process a command to send to the SPI-NAND
+ * by generic SPI bus
+ * @chip: SPI-NAND device structure
+ * @cmd: command structure
+ * Description:
+ *   Set up the command buffer to send to the SPI controller.
+ *   The command buffer has to initialized to 0.
+ */
+static int generic_spi_nand_cmd_fn(struct spi_nand_chip *chip,
+				struct spi_nand_cmd *cmd)
+{
+	struct spi_nand_cmd_cfg *cmd_cfg = spi_nand_get_cmd_cfg(cmd->cmd);
+	struct spi_message message;
+	struct spi_transfer x[3];
+	struct spi_device *spi = chip->priv;
+	u8 buf[SPINAND_MAX_ADDR_LEN];
+
+	spi_message_init(&message);
+	memset(x, 0, sizeof(x));
+	x[0].len = 1;
+	x[0].tx_nbits = 1;
+	x[0].tx_buf = &cmd->cmd;
+	spi_message_add_tail(&x[0], &message);
+
+	if (cmd_cfg->addr_bytes || cmd_cfg->dummy_bytes) {
+		if (cmd_cfg->addr_bytes > cmd->n_addr) {
+			memcpy(buf, cmd->addr, cmd->n_addr);
+			memset(cmd->addr, 0, cmd->n_addr);
+			memcpy(cmd->addr + cmd_cfg->addr_bytes - cmd->n_addr,
+				buf, cmd->n_addr);
+		}
+		x[1].len = cmd_cfg->addr_bytes + cmd_cfg->dummy_bytes;
+		x[1].tx_nbits = cmd_cfg->addr_bits;
+		x[1].tx_buf = cmd->addr;
+		spi_message_add_tail(&x[1], &message);
+	}
+	if (cmd->n_tx) {
+		x[2].len = cmd->n_tx;
+		x[2].tx_nbits = cmd_cfg->data_bits;
+		x[2].tx_buf = cmd->tx_buf;
+		spi_message_add_tail(&x[2], &message);
+	} else if (cmd->n_rx) {
+		x[2].len = cmd->n_rx;
+		x[2].rx_nbits = cmd_cfg->data_bits;
+		x[2].rx_buf = cmd->rx_buf;
+		spi_message_add_tail(&x[2], &message);
+	}
+	return spi_sync(spi, &message);
+}
+
+static int generic_spi_nand_probe(struct spi_device *spi)
+{
+	struct spi_nand_chip *chip;
+	int mfr;
+	struct mtd_info *mtd;
+	int ret;
+	u32 max_speed_hz = spi->max_speed_hz;
+
+	chip = kzalloc(sizeof(struct spi_nand_chip), GFP_KERNEL);
+	if (!chip) {
+		ret = -ENOMEM;
+		goto err1;
+	}
+	mtd = spi_nand_to_mtd(chip);
+	chip->command_fn = generic_spi_nand_cmd_fn;
+	if (spi->mode & SPI_RX_QUAD)
+		chip->controller_caps |= SPINAND_RD_QUAD | SPINAND_RD_X4;
+	if (spi->mode & SPI_RX_DUAL)
+		chip->controller_caps |= SPINAND_RD_DUAL | SPINAND_RD_X2;
+	if (spi->mode & SPI_TX_QUAD)
+		chip->controller_caps |= SPINAND_WR_QUAD | SPINAND_WR_X4;
+	if (spi->mode & SPI_TX_DUAL)
+		chip->controller_caps |= SPINAND_WR_DUAL | SPINAND_WR_X2;
+	chip->priv = spi;
+	spi_set_drvdata(spi, chip);
+	/*
+	 * read ID command format might be different for different manufactory
+	 * such as, Micron SPI NAND need extra one dummy byte after perform
+	 * read ID command but Giga device don't need.
+	 *
+	 * So, specify manufactory of device in device tree is obligatory
+	 */
+	mfr = spi_get_device_id(spi)->driver_data;
+	ret = spi_nand_set_cmd_cfg_table(mfr);
+	if (ret)
+		goto err2;
+
+	spi->max_speed_hz = min_t(int, 25000000, max_speed_hz);
+	ret = spi_nand_scan_ident(chip);
+	if (ret)
+		goto err2;
+
+	spi_nand_manufacture_init(chip);
+
+	spi->max_speed_hz = max_speed_hz;
+	ret = spi_nand_scan_tail(chip);
+	if (ret)
+		goto err3;
+
+	mtd_set_of_node(mtd, spi->dev.of_node);
+
+	ret = mtd_device_parse_register(mtd, NULL, NULL, NULL, 0);
+	if (!ret)
+		return 0;
+
+	spi_nand_scan_tail_release(chip);
+err3:
+	spi_nand_scan_ident_release(chip);
+err2:
+	kfree(chip);
+err1:
+	return ret;
+}
+
+static int generic_spi_nand_remove(struct spi_device *spi)
+{
+	struct spi_nand_chip *chip = spi_get_drvdata(spi);
+
+	spi_nand_release(chip);
+	kfree(chip);
+
+	return 0;
+}
+
+
+static const struct spi_device_id spi_nand_id_table[] = {
+	{ "mt29f", SPINAND_MFR_MICRON },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, spi_nand_id_table);
+
+static struct spi_driver generic_spi_nand_driver = {
+	.driver = {
+		.name	= "generic_spi_nand",
+		.owner	= THIS_MODULE,
+	},
+	.id_table = spi_nand_id_table,
+	.probe	= generic_spi_nand_probe,
+	.remove	= generic_spi_nand_remove,
+};
+module_spi_driver(generic_spi_nand_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 11/11] nand: spi: Add arguments check for read/write

[Peter Pan]

Check offset and length in spi_nand_do_read_ops and
spi_nand_do_write_ops.

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

diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
index 5ac4b26..c7616d6 100644
--- a/drivers/mtd/nand/spi/spi-nand-base.c
+++ b/drivers/mtd/nand/spi/spi-nand-base.c
@@ -685,6 +685,7 @@ static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 			  struct mtd_oob_ops *ops)
 {
 	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
+	struct nand_device *nand = mtd_to_nand(mtd);
 	int ret;
 	struct mtd_ecc_stats stats;
 	unsigned int max_bitflips = 0;
@@ -693,7 +694,24 @@ static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
 		mtd->oobavail : mtd->oobsize;
 
+	if (unlikely(from >= mtd->size)) {
+		pr_err("%s: attempt to read beyond end of device\n",
+				__func__);
+		return -EINVAL;
+	}
 	if (oobreadlen > 0) {
+		if (unlikely(ops->ooboffs >= ooblen)) {
+			pr_err("%s: attempt to start read outside oob\n",
+					__func__);
+			return -EINVAL;
+		}
+		if (unlikely(ops->ooboffs + oobreadlen >
+		(nand_len_to_pages(nand, mtd->size) - nand_offs_to_page(nand, from))
+		* ooblen)) {
+			pr_err("%s: attempt to read beyond end of device\n",
+					__func__);
+			return -EINVAL;
+		}
 		ooblen -= ops->ooboffs;
 		ops->oobretlen = 0;
 	}
@@ -789,12 +807,38 @@ static int spi_nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 		mtd->oobavail : mtd->oobsize;
 	bool ecc_off = ops->mode == MTD_OPS_RAW;
 
+	/* Do not allow reads past end of device */
+	if (unlikely(to >= mtd->size)) {
+		pr_err("%s: attempt to write beyond end of device\n",
+				__func__);
+		return -EINVAL;
+	}
+
 	page_addr = nand_offs_to_page(nand, to);
 	page_offset = to & (nand_page_size(nand) - 1);
 	ops->retlen = 0;
 
 	/* for oob */
 	if (oobwritelen > 0) {
+		/* Do not allow write past end of page */
+		if ((ops->ooboffs + oobwritelen) > ooblen) {
+			pr_err("%s: attempt to write past end of page\n",
+					__func__);
+			return -EINVAL;
+		}
+
+		if (unlikely(ops->ooboffs >= ooblen)) {
+			pr_err("%s: attempt to start write outside oob\n",
+					__func__);
+			return -EINVAL;
+		}
+		if (unlikely(ops->ooboffs + oobwritelen >
+		(nand_len_to_pages(nand, mtd->size) - nand_offs_to_page(nand, to))
+			* ooblen)) {
+			pr_err("%s: attempt to write beyond end of device\n",
+					__func__);
+			return -EINVAL;
+		}
 		ooblen -= ops->ooboffs;
 		ops->oobretlen = 0;
 	}
-- 
1.9.1

Re: [PATCH 10/11] nand: spi: Add generic SPI controller support

[Richard Weinberger]

Peter,

Am 21.02.2017 um 09:00 schrieb Peter Pan:
> This commit supports to use generic spi controller
> as SPI NAND controller.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>

...

> +static void spi_nand_mt29f_ecc_status(struct spi_nand_chip *chip,
> +					unsigned int status,
> +					unsigned int *corrected,
> +					unsigned int *ecc_error)
> +{
> +	unsigned int ecc_status = status & SPI_NAND_MT29F_ECC_MASK;
> +
> +	*ecc_error = (ecc_status == SPI_NAND_MT29F_ECC_UNCORR);
> +	switch (ecc_status) {
> +	case SPI_NAND_MT29F_ECC_0_BIT:
> +		*corrected = 0;
> +		break;
> +	case SPI_NAND_MT29F_ECC_1_3_BIT:
> +		*corrected = 3;
> +		break;
> +	case SPI_NAND_MT29F_ECC_4_6_BIT:
> +		*corrected = 6;
> +		break;
> +	case SPI_NAND_MT29F_ECC_7_8_BIT:
> +		*corrected = 8;
> +		break;
> +	}
> +}

I'm confused. What value will the corrected value be when only 1 bit flipped and got corrected?

Thanks,
//richard

Re: [PATCH 00/11] Introduction to SPI NAND framework

[Richard Weinberger]

Peter,

Am 21.02.2017 um 08:59 schrieb Peter Pan:
> This serie 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 serie is based on Boris's nand/generic branch[2], which
> is on 4.9-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 serie 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.

Can you please share on which SPI NAND chips this got tested?

Thanks,
//richard

Re: [PATCH 00/11] Introduction to SPI NAND framework

[Peter Pan]

Hi Richard,


On Tue, Feb 21, 2017 at 4:05 PM, Richard Weinberger <richard@nod.at> wrote:
> Peter,
>
> Am 21.02.2017 um 08:59 schrieb Peter Pan:
>> This serie 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 serie is based on Boris's nand/generic branch[2], which
>> is on 4.9-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 serie 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.
>
> Can you please share on which SPI NAND chips this got tested?

Sorry for missing this part in cover letter... This series is tested on
Xilinx Zedboard with Micron MT29F2G01AAAED SPI NAND chip.

Thanks,
Peter Pan

Re: [PATCH 10/11] nand: spi: Add generic SPI controller support

[Peter Pan]

Hi Richard,


On Tue, Feb 21, 2017 at 4:03 PM, Richard Weinberger <richard@nod.at> wrote:
> Peter,
>
> Am 21.02.2017 um 09:00 schrieb Peter Pan:
>> This commit supports to use generic spi controller
>> as SPI NAND controller.
>>
>> Signed-off-by: Peter Pan <peterpandong@micron.com>
>
> ...
>
>> +static void spi_nand_mt29f_ecc_status(struct spi_nand_chip *chip,
>> +                                     unsigned int status,
>> +                                     unsigned int *corrected,
>> +                                     unsigned int *ecc_error)
>> +{
>> +     unsigned int ecc_status = status & SPI_NAND_MT29F_ECC_MASK;
>> +
>> +     *ecc_error = (ecc_status == SPI_NAND_MT29F_ECC_UNCORR);
>> +     switch (ecc_status) {
>> +     case SPI_NAND_MT29F_ECC_0_BIT:
>> +             *corrected = 0;
>> +             break;
>> +     case SPI_NAND_MT29F_ECC_1_3_BIT:
>> +             *corrected = 3;
>> +             break;
>> +     case SPI_NAND_MT29F_ECC_4_6_BIT:
>> +             *corrected = 6;
>> +             break;
>> +     case SPI_NAND_MT29F_ECC_7_8_BIT:
>> +             *corrected = 8;
>> +             break;
>> +     }
>> +}
>
> I'm confused. What value will the corrected value be when only 1 bit flipped and got corrected?

According to Micron data sheet, Micron MT29F SPI NAND series only
report 5 level ECC status:
     No errors
     1-3 bit errors detected and corrected
     4-6 bit errors detected and corrected. This indicates data
refreshment might be taken
     7-8 bit errors detected and corrected. This indicates data
refreshment must be taken to guarantee the data retention
     Bit errors greater than 8 bits detected and not corrected

So I report 3 when only 1 bit flipped.

Thanks,
Peter Pan

Re: [PATCH 00/11] Introduction to SPI NAND framework

[Peter Pan]

On Tue, Feb 21, 2017 at 4:11 PM, Peter Pan <peterpansjtu@gmail.com> wrote:
> Hi Richard,
>
>
> On Tue, Feb 21, 2017 at 4:05 PM, Richard Weinberger <richard@nod.at> wrote:
>> Peter,
>>
>> Am 21.02.2017 um 08:59 schrieb Peter Pan:
>>> This serie 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 serie is based on Boris's nand/generic branch[2], which
>>> is on 4.9-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 serie 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.
>>
>> Can you please share on which SPI NAND chips this got tested?
>
> Sorry for missing this part in cover letter... This series is tested on
> Xilinx Zedboard with Micron MT29F2G01AAAED SPI NAND chip.

Sorry for mistake, the chip should be MT29F2G01ABAGDSF

Thanks,
Peter Pan

Re: [PATCH 02/11] nand: spi: create spi_nand_chip struct

[Boris Brezillon]

On Tue, 21 Feb 2017 16:00:01 +0800
Peter Pan <peterpandong@micron.com> wrote:

> Create spi_nand_chip struct and helper functions.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  include/linux/mtd/spi-nand.h | 67 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
> index 68442e08..23b16f0 100644
> --- a/include/linux/mtd/spi-nand.h
> +++ b/include/linux/mtd/spi-nand.h
> @@ -16,6 +16,12 @@
>  #ifndef __LINUX_MTD_SPI_NAND_H
>  #define __LINUX_MTD_SPI_NAND_H
>  
> +#include <linux/wait.h>
> +#include <linux/spinlock.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/flashchip.h>
> +#include <linux/mtd/nand.h>
> +
>  /*
>   * Standard SPI-NAND flash commands
>   */
> @@ -109,4 +115,65 @@
>  #define SPI_NAND_MT29F_ECC_7_8_BIT	0x50
>  #define SPI_NAND_MT29F_ECC_UNCORR	0x20
>  
> +/**
> + * struct spi_nand_chip - SPI-NAND Private Flash Chip Data
> + * @base:		[INTERN] NAND device instance

Drop extra tabs after the field name, and please drop these
[INTERN/BOARDSPECIFIC] specifiers.

> + * @chip_lock:		[INTERN] protection lock
> + * @name:		name of the chip
> + * @wq:			[INTERN] wait queue to sleep on if a SPI-NAND operation
> + *			is in progress used instead of the per chip wait queue
> + *			when a hw controller is available.
> + * @mfr_id:		[BOARDSPECIFIC] manufacture id
> + * @dev_id:		[BOARDSPECIFIC] device id
> + * @state:		[INTERN] the current state of the SPI-NAND device
> + * @read_cache_op:	[REPLACEABLE] Opcode of read from cache
> + * @write_cache_op:	[REPLACEABLE] Opcode of program load
> + * @buf:		[INTERN] buffer for read/write data
> + * @oobbuf:		[INTERN] buffer for read/write oob
> + * @controller_caps:	[INTERN] capacities of SPI NAND controller
> + * @size:		[INTERN] the size of chip
> + * @options:		[BOARDSPECIFIC] various chip options. They can partly
> + *			be set to inform nand_scan about special functionality.
> + * @ecc_strength:	[INTERN] ECC correctability from the datasheet.
> + * @priv:		[BOARDSPECIFIC] pointer to controller data
> + */
> +struct spi_nand_chip {

s/spi_nand_chip/spinand_device/

Let's try to be consistent with the rawnand_device naming scheme. So in
general s/spi_nand/spinand/

> +	struct nand_device	base;

No tabs, just a single space.

> +	spinlock_t	chip_lock;

s/chip_lock/lock/, why do you need a spinlock here? 

> +	char		*name;
> +	wait_queue_head_t wq;
> +	u8		mfr_id;
> +	u8		dev_id;

Can we make it an array of u8 and then have pre-defined indexes like

#define SPINAND_MFR_ID		0
#define SPINAND_DEV_ID		1
...
	
> +	flstate_t	state;
> +	u8		read_cache_op;
> +	u8		write_cache_op;

Hm, so these operations are manufacturer specific.

> +	u8		*buf;
> +	u8		*oobbuf;
> +	u32		controller_caps;

What flags do you plan to put here ^?

> +	u64		size;

Isn't it already defined in nand_device?

> +	u32		options;

Again, what do you plan to put in this field?

> +	u32		ecc_strength;
> +	void		*priv;

Ok, it seems that ->priv is actually used to store controller specific
data. Please rename it or do something like:

	struct {
		void *priv;
	} controller;

It will avoid the kind of confusion we have in the rawnand framework.

Since you also have controller caps, you could do:

	struct {
		u32 caps;
		void *priv;
		/*
		 * whatever is controller specific. probably a pointer
		 * to the SPI controller or something like that.
		 */
	} controller;

> +};
> +
> +static inline struct spi_nand_chip *mtd_to_spi_nand(struct mtd_info *mtd)
> +{
> +	return container_of(mtd_to_nand(mtd), struct spi_nand_chip, base);
> +}
> +
> +static inline struct mtd_info *spi_nand_to_mtd(struct spi_nand_chip *chip)
> +{
> +	return nand_to_mtd(&chip->base);
> +}
> +
> +static inline void *spi_nand_get_controller_data(struct spi_nand_chip *chip)
> +{
> +	return chip->priv;
> +}
> +
> +static inline void spi_nand_set_controller_data(struct spi_nand_chip *chip,
> +						void *priv)
> +{
> +	chip->priv = priv;
> +}
>  #endif /* __LINUX_MTD_SPI_NAND_H */

Re: [PATCH 00/11] Introduction to SPI NAND framework

[Boris Brezillon]

On Tue, 21 Feb 2017 15:59:59 +0800
Peter Pan <peterpandong@micron.com> wrote:

> This serie 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 serie is based on Boris's nand/generic branch[2], which
> is on 4.9-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.

Thanks for doing that.

> 
> This serie 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.

Okay, that's actually a good start. We should definitely take support
for external ECC engine into account when designing the framework, but
we don't need to support it right now.

> 
> 
> [1]http://lists.infradead.org/pipermail/linux-mtd/2015-January/057223.html
> [2]https://github.com/bbrezillon/linux-0day/tree/nand/generic
> 
> 
> Peter Pan (11):
>   nand: Add SPI NAND cmd set and register definition
>   nand: spi: create spi_nand_chip struct
>   nand: spi: Abstract SPI NAND cmd set to functions
>   nand: spi: Add read function support
>   nand: spi: Add write function support
>   nand: spi: Add erase function support
>   nand: spi: Add init/release function
>   nand: spi: Add bad block support
>   nand: spi: Add BBT support
>   nand: spi: Add generic SPI controller support
>   nand: spi: Add arguments check for read/write

First comment: I think you're trying to separate things that should
not be. I mean, adding support for spi-nand means adding support for all
basic features at once (read/write/erase). Please squash patches 1 to 6
into a single patch. I'm not sure yet about patch 7 and 8, but according
to the title, they should also be part of this "add basic support for
SPI NANDs" commit.
Then, BBT is clearly an optional feature that should be added in a
separate patch.

> 
>  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/chips/Kconfig       |    5 +
>  drivers/mtd/nand/spi/chips/Makefile      |    1 +
>  drivers/mtd/nand/spi/chips/generic_spi.c |  269 ++++++
>  drivers/mtd/nand/spi/spi-nand-base.c     | 1537 ++++++++++++++++++++++++++++++
>  drivers/mtd/nand/spi/spi-nand-cmd.c      |   69 ++
>  include/linux/mtd/spi-nand.h             |  269 ++++++
>  10 files changed, 2162 insertions(+)
>  create mode 100644 drivers/mtd/nand/spi/Kconfig
>  create mode 100644 drivers/mtd/nand/spi/Makefile
>  create mode 100644 drivers/mtd/nand/spi/chips/Kconfig
>  create mode 100644 drivers/mtd/nand/spi/chips/Makefile
>  create mode 100644 drivers/mtd/nand/spi/chips/generic_spi.c
>  create mode 100644 drivers/mtd/nand/spi/spi-nand-base.c
>  create mode 100644 drivers/mtd/nand/spi/spi-nand-cmd.c
>  create mode 100644 include/linux/mtd/spi-nand.h
> 

Re: [PATCH 01/11] nand: Add SPI NAND cmd set and register definition

[Boris Brezillon]

On Tue, 21 Feb 2017 16:00:00 +0800
Peter Pan <peterpandong@micron.com> wrote:

> This commit adds SPI NAND command set and register
> definition according Micron SPI NAND data sheet.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  include/linux/mtd/spi-nand.h | 112 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100644 include/linux/mtd/spi-nand.h
> 
> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
> new file mode 100644
> index 0000000..68442e08
> --- /dev/null
> +++ b/include/linux/mtd/spi-nand.h
> @@ -0,0 +1,112 @@
> +/**
> +* spi-nand.h
> +*
> +* 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_SPI_NAND_H
> +#define __LINUX_MTD_SPI_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_PAGE_CACHE_RDM		0x30
> +#define SPINAND_CMD_READ_PAGE_CACHE_LAST	0x3f
> +#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
> +#define SPINAND_CMD_END				0x0
> +
> +
> +/* feature registers */
> +#define REG_BLOCK_LOCK		0xa0
> +#define REG_CFG			0xb0
> +#define REG_STATUS		0xc0
> +#define REG_DIE_SELECT		0xd0
> +
> +/* status */
> +#define STATUS_OIP_MASK		0x01
> +#define STATUS_CRBSY_MASK	0x80
> +#define STATUS_READY		(0 << 0)
> +#define STATUS_BUSY		(1 << 0)
> +
> +#define STATUS_E_FAIL_MASK	0x04
> +#define STATUS_E_FAIL		(1 << 2)
> +
> +#define STATUS_P_FAIL_MASK	0x08
> +#define STATUS_P_FAIL		(1 << 3)
> +
> +
> +/*Configuration register defines*/
> +#define CFG_QE_MASK		0x01
> +#define CFG_QE_ENABLE		0x01
> +#define CFG_ECC_MASK		0x10
> +#define CFG_ECC_ENABLE		0x10
> +#define CFG_LOT_MASK		0x20
> +#define CFG_LOT_ENABLE		0x20
> +#define CFG_OTP_MASK		0xc2
> +#define CFG_OTP_ENTER		0x40
> +#define CFG_OTP_EXIT		0x00
> +
> +/* block lock */
> +#define BL_ALL_LOCKED		0x7c
> +#define BL_U_1_1024_LOCKED		0x08
> +#define BL_U_1_512_LOCKED		0x10
> +#define BL_U_1_256_LOCKED		0x18
> +#define BL_U_1_128_LOCKED		0x20
> +#define BL_U_1_64_LOCKED		0x28
> +#define BL_U_1_32_LOCKED		0x30
> +#define BL_U_1_16_LOCKED		0x38
> +#define BL_U_1_8_LOCKED		0x40
> +#define BL_U_1_4_LOCKED		0x48
> +#define BL_U_1_2_LOCKED		0x50
> +#define BL_L_1_1024_LOCKED		0x0c
> +#define BL_L_1_512_LOCKED		0x14
> +#define BL_L_1_256_LOCKED		0x1c
> +#define BL_L_1_128_LOCKED		0x24
> +#define BL_L_1_64_LOCKED		0x2c
> +#define BL_L_1_32_LOCKED		0x34
> +#define BL_L_1_16_LOCKED		0x3c
> +#define BL_L_1_8_LOCKED		0x44
> +#define BL_L_1_4_LOCKED		0x4c
> +#define BL_L_1_2_LOCKED		0x54
> +#define BL_ALL_UNLOCKED		0X00
> +
> +/* die select */
> +#define DIE_SELECT_MASK		0x40
> +#define DIE_SELECT_DS0		0x00
> +#define DIE_SELECT_DS1		0x40
> +
> +#define SPI_NAND_MT29F_ECC_MASK		0x70
> +#define SPI_NAND_MT29F_ECC_0_BIT	0x00
> +#define SPI_NAND_MT29F_ECC_1_3_BIT	0x10
> +#define SPI_NAND_MT29F_ECC_4_6_BIT	0x30
> +#define SPI_NAND_MT29F_ECC_7_8_BIT	0x50
> +#define SPI_NAND_MT29F_ECC_UNCORR	0x20

These are manufacturer specific definitions, and I don't think they
should be exposed in the generic header.

Re: [PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Arnaud Mouiche]

Hello Peter.
First thank you for this effort to finally bring the spinand framework 
back on stage.

On 21/02/2017 09:00, Peter Pan wrote:
> This commit abstracts basic SPI NAND commands to
> functions in spi-nand-base.c. Because command sets
> have difference by vendors, we create spi-nand-cmd.c
> to define this difference by command config table.
>
> [...]
> +
> +/**
> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
> + * to cache
> + * @chip: SPI-NAND device structure
> + * @page_addr: page to read
> + */
> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
> +					u32 page_addr)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_PAGE_READ;
> +	cmd.n_addr = 3;
> +	cmd.addr[0] = (u8)(page_addr >> 16);
> +	cmd.addr[1] = (u8)(page_addr >> 8);
> +	cmd.addr[2] = (u8)page_addr;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}

Just a question. Don't you try to map too exactly the spi_nand_cmd to 
the internal micron SPI command ?
Why "cmd.addr" should be a byte array, and not a u32, leaving each chip 
to details to handle the u32 to SPI byte stream mapping.


> +
> +/**
> + * spi_nand_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
> + * Description:
> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
> + *   The read can specify 1 to (page size + spare size) bytes of data read at
> + *   the corresponding locations.
> + *   No tRd delay.
> + */
> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
> +		u32 page_addr, u32 column, size_t len, u8 *rbuf)
> +{
> +	struct nand_device *nand = &chip->base;
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = chip->read_cache_op;
> +	cmd.n_addr = 2;
> +	cmd.addr[0] = (u8)(column >> 8);
> +	if (chip->options & SPINAND_NEED_PLANE_SELECT)
> +		cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
> +				& 0x1) << 4);
> +	cmd.addr[1] = (u8)column;
> +	cmd.n_rx = len;
> +	cmd.rx_buf = rbuf;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
Some reasons to handle the plane concept (Micron specific), and even 
move the fact that it touch the bit 4 of cdm.addr[0], in common code ?

Regards,
Arnaud

Re: [PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Boris Brezillon]

On Tue, 21 Feb 2017 16:00:02 +0800
Peter Pan <peterpandong@micron.com> wrote:

> This commit abstracts basic SPI NAND commands to
> functions in spi-nand-base.c. Because command sets
> have difference by vendors, we create spi-nand-cmd.c
> to define this difference by command config table.
> 
> 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        |   2 +
>  drivers/mtd/nand/spi/spi-nand-base.c | 368 +++++++++++++++++++++++++++++++++++
>  drivers/mtd/nand/spi/spi-nand-cmd.c  |  69 +++++++
>  include/linux/mtd/spi-nand.h         |  34 ++++
>  7 files changed, 480 insertions(+)
>  create mode 100644 drivers/mtd/nand/spi/Kconfig
>  create mode 100644 drivers/mtd/nand/spi/Makefile
>  create mode 100644 drivers/mtd/nand/spi/spi-nand-base.c
>  create mode 100644 drivers/mtd/nand/spi/spi-nand-cmd.c
> 
> 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..04a7b71
> --- /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..3c617d6
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-base.o
> +obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-cmd.o
> diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
> new file mode 100644
> index 0000000..b75e5cd
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/spi-nand-base.c
> @@ -0,0 +1,368 @@
> +/**
> +* spi-nand-base.c
> +*
> +* 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.
> +*/
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/mtd/spi-nand.h>
> +
> +static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
> +				struct spi_nand_cmd *cmd)
> +{
> +	return chip->command_fn(chip, cmd);
> +}
> +
> +/**
> + * spi_nand_read_reg - send command 0Fh to read register
> + * @chip: SPI-NAND device structure
> + * @reg; register to read
> + * @buf: buffer to store value
> + */
> +static int spi_nand_read_reg(struct spi_nand_chip *chip,
> +			uint8_t reg, uint8_t *buf)
> +{
> +	struct spi_nand_cmd cmd;
> +	int ret;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_GET_FEATURE;
> +	cmd.n_addr = 1;
> +	cmd.addr[0] = reg;
> +	cmd.n_rx = 1;
> +	cmd.rx_buf = buf;
> +
> +	ret = spi_nand_issue_cmd(chip, &cmd);
> +	if (ret < 0)
> +		pr_err("err: %d read register %d\n", ret, reg);
> +
> +	return ret;
> +}
> +
> +/**
> + * spi_nand_write_reg - send command 1Fh to write register
> + * @chip: SPI-NAND device structure
> + * @reg; register to write
> + * @buf: buffer stored value
> + */
> +static int spi_nand_write_reg(struct spi_nand_chip *chip,
> +			uint8_t reg, uint8_t *buf)
> +{
> +	struct spi_nand_cmd cmd;
> +	int ret;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_SET_FEATURE;
> +	cmd.n_addr = 1;
> +	cmd.addr[0] = reg;
> +	cmd.n_tx = 1,
> +	cmd.tx_buf = buf,
> +
> +	ret = spi_nand_issue_cmd(chip, &cmd);
> +	if (ret < 0)
> +		pr_err("err: %d write register %d\n", ret, reg);
> +
> +	return ret;
> +}
> +
> +/**
> + * spi_nand_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.
> + *   Once the status turns to be ready, the other status bits also are
> + *   valid status bits.
> + */
> +static int spi_nand_read_status(struct spi_nand_chip *chip, uint8_t *status)
> +{
> +	return spi_nand_read_reg(chip, REG_STATUS, status);
> +}
> +
> +/**
> + * spi_nand_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 spi_nand_get_cfg(struct spi_nand_chip *chip, u8 *cfg)
> +{
> +	return spi_nand_read_reg(chip, REG_CFG, cfg);
> +}
> +
> +/**
> + * spi_nand_set_cfg - set value to configuration register
> + * @chip: SPI-NAND device structure
> + * @cfg: buffer stored value
> + * Description:
> + *   Configuration register includes OTP config, Lock Tight enable/disable
> + *   and Internal ECC enable/disable.
> + */
> +static int spi_nand_set_cfg(struct spi_nand_chip *chip, u8 *cfg)
> +{
> +	return spi_nand_write_reg(chip, REG_CFG, cfg);
> +}
> +
> +/**
> + * spi_nand_enable_ecc - enable internal ECC
> + * @chip: SPI-NAND device structure
> + * Description:
> + *   There is one bit( bit 0x10 ) to set or to clear the internal ECC.
> + *   Enable chip internal ECC, set the bit to 1
> + *   Disable chip internal ECC, clear the bit to 0
> + */
> +static void spi_nand_enable_ecc(struct spi_nand_chip *chip)
> +{
> +	u8 cfg = 0;
> +
> +	spi_nand_get_cfg(chip, &cfg);
> +	if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE)
> +		return;
> +	cfg |= CFG_ECC_ENABLE;
> +	spi_nand_set_cfg(chip, &cfg);
> +}
> +
> +/**
> + * spi_nand_disable_ecc - disable internal ECC
> + * @chip: SPI-NAND device structure
> + * Description:
> + *   There is one bit( bit 0x10 ) to set or to clear the internal ECC.
> + *   Enable chip internal ECC, set the bit to 1
> + *   Disable chip internal ECC, clear the bit to 0
> + */
> +static void spi_nand_disable_ecc(struct spi_nand_chip *chip)
> +{
> +	u8 cfg = 0;
> +
> +	spi_nand_get_cfg(chip, &cfg);
> +	if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE) {
> +		cfg &= ~CFG_ECC_ENABLE;
> +		spi_nand_set_cfg(chip, &cfg);
> +	}
> +}
> +
> +/**
> + * spi_nand_write_enable - send command 06h to enable write or erase the
> + * Nand cells
> + * @chip: SPI-NAND device structure
> + * Description:
> + *   Before write and erase the Nand cells, the write enable has to be set.
> + *   After the write or erase, the write enable bit is automatically
> + *   cleared (status register bit 2)
> + *   Set the bit 2 of the status register has the same effect
> + */
> +static int spi_nand_write_enable(struct spi_nand_chip *chip)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_WR_ENABLE;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +/**
> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
> + * to cache
> + * @chip: SPI-NAND device structure
> + * @page_addr: page to read
> + */
> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
> +					u32 page_addr)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_PAGE_READ;
> +	cmd.n_addr = 3;
> +	cmd.addr[0] = (u8)(page_addr >> 16);
> +	cmd.addr[1] = (u8)(page_addr >> 8);
> +	cmd.addr[2] = (u8)page_addr;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +/**
> + * spi_nand_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
> + * Description:
> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
> + *   The read can specify 1 to (page size + spare size) bytes of data read at
> + *   the corresponding locations.
> + *   No tRd delay.
> + */
> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
> +		u32 page_addr, u32 column, size_t len, u8 *rbuf)
> +{
> +	struct nand_device *nand = &chip->base;
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = chip->read_cache_op;
> +	cmd.n_addr = 2;
> +	cmd.addr[0] = (u8)(column >> 8);
> +	if (chip->options & SPINAND_NEED_PLANE_SELECT)
> +		cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
> +				& 0x1) << 4);
> +	cmd.addr[1] = (u8)column;
> +	cmd.n_rx = len;
> +	cmd.rx_buf = rbuf;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +/**
> + * spi_nand_program_data_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
> + * @clr_cache: clear cache register or not
> + * Description:
> + *   Command can be 02h, 32h, 84h, 34h
> + *   02h and 32h will clear the cache with 0xff value first
> + *   Since it is writing the data to cache, there is no tPROG time.
> + */
> +static int spi_nand_program_data_to_cache(struct spi_nand_chip *chip,
> +			u32 page_addr, u32 column, size_t len, const u8 *wbuf)
> +{
> +	struct nand_device *nand = &chip->base;
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = chip->write_cache_op;
> +	cmd.n_addr = 2;
> +	cmd.addr[0] = (u8)(column >> 8);
> +	if (chip->options & SPINAND_NEED_PLANE_SELECT)
> +		cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
> +				& 0x1) << 4);
> +	cmd.addr[1] = (u8)column;
> +	cmd.n_tx = len;
> +	cmd.tx_buf = wbuf;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +/**
> + * spi_nand_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.
> + * Description:
> + *   Need to wait for tPROG time to finish the transaction.
> + */
> +static int spi_nand_program_execute(struct spi_nand_chip *chip, u32 page_addr)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_PROG_EXC;
> +	cmd.n_addr = 3;
> +	cmd.addr[0] = (u8)(page_addr >> 16);
> +	cmd.addr[1] = (u8)(page_addr >> 8);
> +	cmd.addr[2] = (u8)page_addr;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +
> +/**
> + * spi_nand_erase_block_erase - send command D8h to erase a block
> + * @chip: SPI-NAND device structure
> + * @page_addr: the page to erase.
> + * Description:
> + *   Need to wait for tERS.
> + */
> +static int spi_nand_erase_block(struct spi_nand_chip *chip,
> +					u32 page_addr)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_BLK_ERASE;
> +	cmd.n_addr = 3;
> +	cmd.addr[0] = (u8)(page_addr >> 16);
> +	cmd.addr[1] = (u8)(page_addr >> 8);
> +	cmd.addr[2] = (u8)page_addr;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +/**
> + * spi_nand_read_id - send 9Fh command to get ID
> + * @chip: SPI-NAND device structure
> + * @buf: buffer to store id
> + */
> +static int spi_nand_read_id(struct spi_nand_chip *chip, u8 *buf)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_READ_ID;
> +	cmd.n_rx = 2;
> +	cmd.rx_buf = buf;
> +
> +	return spi_nand_issue_cmd(chip, &cmd);
> +}
> +
> +/**
> + * spi_nand_reset - send command FFh to reset chip.
> + * @chip: SPI-NAND device structure
> + */
> +static int spi_nand_reset(struct spi_nand_chip *chip)
> +{
> +	struct spi_nand_cmd cmd;
> +
> +	memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> +	cmd.cmd = SPINAND_CMD_RESET;
> +
> +	if (spi_nand_issue_cmd(chip, &cmd) < 0)
> +		pr_err("spi_nand reset failed!\n");
> +
> +	/* elapse 2ms before issuing any other command */
> +	udelay(2000);

2ms looks huge. Are you sure you can't poll the status register before
that?

> +
> +	return 0;
> +}
> +
> +/**
> + * spi_nand_lock_block - write block lock register to
> + * lock/unlock device
> + * @spi: spi device structure
> + * @lock: value to set to block lock register
> + * Description:
> + *   After power up, all the Nand blocks are locked.  This function allows
> + *   one to unlock the blocks, and so it can be written or erased.
> + */
> +static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
> +{
> +	return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
> +}
> +
> +MODULE_DESCRIPTION("SPI NAND framework");
> +MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mtd/nand/spi/spi-nand-cmd.c b/drivers/mtd/nand/spi/spi-nand-cmd.c
> new file mode 100644
> index 0000000..d63741e
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/spi-nand-cmd.c
> @@ -0,0 +1,69 @@
> +/**
> +* spi-nand-cmd.c
> +*
> +* 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/mtd/spi-nand.h>
> +#include <linux/kernel.h>
> +
> +static struct spi_nand_cmd_cfg *cmd_table;
> +
> +static struct spi_nand_cmd_cfg micron_cmd_cfg_table[] = {

static const

> +/*opcode	addr_bytes	addr_bits	dummy_bytes	data_nbits*/
> +	{SPINAND_CMD_GET_FEATURE,		1,	1,	0,	1},
> +	{SPINAND_CMD_SET_FEATURE,		1,	1,	0,	1},
> +	{SPINAND_CMD_PAGE_READ,			3,	1,	0,	0},
> +	{SPINAND_CMD_READ_FROM_CACHE,		2,	1,	1,	1},
> +	{SPINAND_CMD_READ_FROM_CACHE_FAST,	2,	1,	1,	1},
> +	{SPINAND_CMD_READ_FROM_CACHE_X2,	2,	1,	1,	2},
> +	{SPINAND_CMD_READ_FROM_CACHE_DUAL_IO,	2,	2,	1,	2},
> +	{SPINAND_CMD_READ_FROM_CACHE_X4,	2,	1,	1,	4},
> +	{SPINAND_CMD_READ_FROM_CACHE_QUAD_IO,	2,	4,	2,	4},
> +	{SPINAND_CMD_BLK_ERASE,			3,	1,	0,	0},
> +	{SPINAND_CMD_PROG_EXC,			3,	1,	0,	0},
> +	{SPINAND_CMD_PROG_LOAD,			2,	1,	0,	1},
> +	{SPINAND_CMD_PROG_LOAD_RDM_DATA,	2,	1,	0,	1},
> +	{SPINAND_CMD_PROG_LOAD_X4,		2,	1,	0,	4},
> +	{SPINAND_CMD_PROG_LOAD_RDM_DATA_X4,	2,	1,	0,	4},
> +	{SPINAND_CMD_WR_ENABLE,			0,	0,	0,	0},
> +	{SPINAND_CMD_WR_DISABLE,		0,	0,	0,	0},
> +	{SPINAND_CMD_READ_ID,			0,	0,	1,	1},
> +	{SPINAND_CMD_RESET,			0,	0,	0,	0},
> +	{SPINAND_CMD_END},
> +};
> +
> +struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode)
> +{
> +	struct spi_nand_cmd_cfg *index = cmd_table;
> +
> +	for (; index->opcode != SPINAND_CMD_END; index++) {
> +		if (index->opcode == opcode)
> +			return index;
> +	}
> +
> +	pr_err("Invalid spi nand opcode %x\n", opcode);
> +	BUG();
> +}
> +
> +int spi_nand_set_cmd_cfg_table(int mfr)
> +{
> +	switch (mfr) {
> +	case SPINAND_MFR_MICRON:
> +		cmd_table = micron_cmd_cfg_table;
> +		break;

As said earlier, I really don't want to reproduce the errors done in
the rawnand framework, so I'd like to separate manufacturer specific
things into separate source files, and let this manufacturer-drivers
initialize things in a dedicated ->init() function (see the rework I've
done here [1] for the // NAND framework).

> +	default:
> +		pr_err("Unknown device\n");
> +		return -ENODEV;
> +	}
> +	return 0;
> +}
> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
> index 23b16f0..1fcbad7 100644
> --- a/include/linux/mtd/spi-nand.h
> +++ b/include/linux/mtd/spi-nand.h
> @@ -115,6 +115,9 @@
>  #define SPI_NAND_MT29F_ECC_7_8_BIT	0x50
>  #define SPI_NAND_MT29F_ECC_UNCORR	0x20
>  
> +
> +struct spi_nand_cmd;
> +
>  /**
>   * struct spi_nand_chip - SPI-NAND Private Flash Chip Data
>   * @base:		[INTERN] NAND device instance
> @@ -128,6 +131,7 @@
>   * @state:		[INTERN] the current state of the SPI-NAND device
>   * @read_cache_op:	[REPLACEABLE] Opcode of read from cache
>   * @write_cache_op:	[REPLACEABLE] Opcode of program load
> + * @command_fn:		[BOARDSPECIFIC] function to handle command transfer
>   * @buf:		[INTERN] buffer for read/write data
>   * @oobbuf:		[INTERN] buffer for read/write oob
>   * @controller_caps:	[INTERN] capacities of SPI NAND controller
> @@ -147,6 +151,8 @@ struct spi_nand_chip {
>  	flstate_t	state;
>  	u8		read_cache_op;
>  	u8		write_cache_op;
> +	int (*command_fn)(struct spi_nand_chip *this,
> +			struct spi_nand_cmd *cmd);

Nope, let's try to not do the same mistake we did in the raw (parallel)
NAND framework.

This seems to be a controller specific hook. You should define

struct spinand_controller_ops {
	int (*exec_op)(struct spinand_device *nand,
		       struct spinand_op *op);
};

Then have a

struct spinand_controller {
	struct spinand_controller_ops *ops;
	u32 caps;
	/* other controller fields */
};

and inside you spinand_device struct:

struct spinand_device {
	struct spinand_controller *controller;
	void *controller_priv;
};

I know it goes against my suggestion to put controller caps and priv
into a sub-struct inside the spinand_device struct, but it seems that
you really need to clearly separate the spinand_device and
spinand_controller objects.

>  	u8		*buf;
>  	u8		*oobbuf;
>  	u32		controller_caps;
> @@ -176,4 +182,32 @@ static inline void spi_nand_set_controller_data(struct spi_nand_chip *chip,
>  {
>  	chip->priv = priv;
>  }
> +
> +#define SPINAND_MAX_ADDR_LEN		4
> +
> +struct spi_nand_cmd {

struct spinand_op (op stands for operation), to avoid confusion with the
cmd field in this structure.

> +	u8		cmd;
> +	u8		n_addr;		/* Number of address */
> +	u8		addr[SPINAND_MAX_ADDR_LEN];	/* Reg Offset */
> +	u32		n_tx;		/* Number of tx bytes */
> +	const u8	*tx_buf;	/* Tx buf */
> +	u32		n_rx;		/* Number of rx bytes */
> +	u8		*rx_buf;	/* Rx buf */
> +};
> +
> +struct spi_nand_cmd_cfg {

spinand_op_def?

> +	u8		opcode;
> +	u8		addr_bytes;
> +	u8		addr_bits;
> +	u8		dummy_bytes;
> +	u8		data_bits;
> +};
> +
> +/*SPI NAND chip options*/
> +#define SPINAND_NEED_PLANE_SELECT	(1 << 0)
> +
> +/*SPI NAND manufacture ID definition*/
> +#define SPINAND_MFR_MICRON		0x2C
> +int spi_nand_set_cmd_cfg_table(int mfr);
> +struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode);
>  #endif /* __LINUX_MTD_SPI_NAND_H */

[1]https://lkml.org/lkml/2017/1/9/169

Re: [PATCH 02/11] nand: spi: create spi_nand_chip struct

[Peter Pan]

Hi Boris,

First, thanks for you comments. I regret I send it out too late!!

On Tue, Feb 21, 2017 at 4:34 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 16:00:01 +0800
> Peter Pan <peterpandong@micron.com> wrote:
>
>> Create spi_nand_chip struct and helper functions.
>>
>> Signed-off-by: Peter Pan <peterpandong@micron.com>
>> ---
>>  include/linux/mtd/spi-nand.h | 67 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
>> index 68442e08..23b16f0 100644
>> --- a/include/linux/mtd/spi-nand.h
>> +++ b/include/linux/mtd/spi-nand.h
>> @@ -16,6 +16,12 @@
>>  #ifndef __LINUX_MTD_SPI_NAND_H
>>  #define __LINUX_MTD_SPI_NAND_H
>>
>> +#include <linux/wait.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/flashchip.h>
>> +#include <linux/mtd/nand.h>
>> +
>>  /*
>>   * Standard SPI-NAND flash commands
>>   */
>> @@ -109,4 +115,65 @@
>>  #define SPI_NAND_MT29F_ECC_7_8_BIT   0x50
>>  #define SPI_NAND_MT29F_ECC_UNCORR    0x20
>>
>> +/**
>> + * struct spi_nand_chip - SPI-NAND Private Flash Chip Data
>> + * @base:            [INTERN] NAND device instance
>
> Drop extra tabs after the field name, and please drop these
> [INTERN/BOARDSPECIFIC] specifiers.

Fix this in v2.

>
>> + * @chip_lock:               [INTERN] protection lock
>> + * @name:            name of the chip
>> + * @wq:                      [INTERN] wait queue to sleep on if a SPI-NAND operation
>> + *                   is in progress used instead of the per chip wait queue
>> + *                   when a hw controller is available.
>> + * @mfr_id:          [BOARDSPECIFIC] manufacture id
>> + * @dev_id:          [BOARDSPECIFIC] device id
>> + * @state:           [INTERN] the current state of the SPI-NAND device
>> + * @read_cache_op:   [REPLACEABLE] Opcode of read from cache
>> + * @write_cache_op:  [REPLACEABLE] Opcode of program load
>> + * @buf:             [INTERN] buffer for read/write data
>> + * @oobbuf:          [INTERN] buffer for read/write oob
>> + * @controller_caps: [INTERN] capacities of SPI NAND controller
>> + * @size:            [INTERN] the size of chip
>> + * @options:         [BOARDSPECIFIC] various chip options. They can partly
>> + *                   be set to inform nand_scan about special functionality.
>> + * @ecc_strength:    [INTERN] ECC correctability from the datasheet.
>> + * @priv:            [BOARDSPECIFIC] pointer to controller data
>> + */
>> +struct spi_nand_chip {
>
> s/spi_nand_chip/spinand_device/
>
> Let's try to be consistent with the rawnand_device naming scheme. So in
> general s/spi_nand/spinand/

Fix this in v2.

>
>> +     struct nand_device      base;
>
> No tabs, just a single space.

Fix this in v2.

>
>> +     spinlock_t      chip_lock;
>
> s/chip_lock/lock/, why do you need a spinlock here?

Fix this in v2. The spinlock is used for get/put device. nand_get_device also
has a lock. I'll put this lock in controller struct, is this OK?

>
>> +     char            *name;
>> +     wait_queue_head_t wq;
>> +     u8              mfr_id;
>> +     u8              dev_id;
>
> Can we make it an array of u8 and then have pre-defined indexes like

Fix this in v2.

>
> #define SPINAND_MFR_ID          0
> #define SPINAND_DEV_ID          1
> ...
>
>> +     flstate_t       state;
>> +     u8              read_cache_op;
>> +     u8              write_cache_op;
>
> Hm, so these operations are manufacturer specific.

SPI NAND has x1/x2/x4/dual IO/quad IO modes for read/write. These two
are used to store opcode for read and write. The opcode for read and write
are uniform for vendors.

>
>> +     u8              *buf;
>> +     u8              *oobbuf;
>> +     u32             controller_caps;
>
> What flags do you plan to put here ^?

RIght now, the flags include support read/write mode(x1/x2/x4/dual/quad).

>
>> +     u64             size;
>
> Isn't it already defined in nand_device?

Yes you're right. Fix this in v2.

>
>> +     u32             options;
>
> Again, what do you plan to put in this field?

Some Micron chips need to select plane and die when read/write.

>
>> +     u32             ecc_strength;
>> +     void            *priv;
>
> Ok, it seems that ->priv is actually used to store controller specific
> data. Please rename it or do something like:
>
>         struct {
>                 void *priv;
>         } controller;
>
> It will avoid the kind of confusion we have in the rawnand framework.
>
> Since you also have controller caps, you could do:
>
>         struct {
>                 u32 caps;
>                 void *priv;
>                 /*
>                  * whatever is controller specific. probably a pointer
>                  * to the SPI controller or something like that.
>                  */
>         } controller;
>

Really valuable comment. Fix this in v2.

Thanks,
Peter Pan

Re: [PATCH 00/11] Introduction to SPI NAND framework

[Peter Pan]

Hi Boris,

On Tue, Feb 21, 2017 at 4:43 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 15:59:59 +0800
> Peter Pan <peterpandong@micron.com> wrote:
>
>> This serie 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 serie is based on Boris's nand/generic branch[2], which
>> is on 4.9-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.
>
> Thanks for doing that.
>
>>
>> This serie 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.
>
> Okay, that's actually a good start. We should definitely take support
> for external ECC engine into account when designing the framework, but
> we don't need to support it right now.

I totally agree with you.

>
>>
>>
>> [1]http://lists.infradead.org/pipermail/linux-mtd/2015-January/057223.html
>> [2]https://github.com/bbrezillon/linux-0day/tree/nand/generic
>>
>>
>> Peter Pan (11):
>>   nand: Add SPI NAND cmd set and register definition
>>   nand: spi: create spi_nand_chip struct
>>   nand: spi: Abstract SPI NAND cmd set to functions
>>   nand: spi: Add read function support
>>   nand: spi: Add write function support
>>   nand: spi: Add erase function support
>>   nand: spi: Add init/release function
>>   nand: spi: Add bad block support
>>   nand: spi: Add BBT support
>>   nand: spi: Add generic SPI controller support
>>   nand: spi: Add arguments check for read/write
>
> First comment: I think you're trying to separate things that should
> not be. I mean, adding support for spi-nand means adding support for all
> basic features at once (read/write/erase). Please squash patches 1 to 6
> into a single patch. I'm not sure yet about patch 7 and 8, but according
> to the title, they should also be part of this "add basic support for
> SPI NANDs" commit.
> Then, BBT is clearly an optional feature that should be added in a
> separate patch.

The reason I split patch like this is the patch is too long for reviewing.
Maybe I should remove some from framework :). I will follow you suggestion
in v2.

Thanks
Peter Pan

Re: [PATCH 01/11] nand: Add SPI NAND cmd set and register definition

[Peter Pan]

Hi Boris,

On Tue, Feb 21, 2017 at 4:44 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 16:00:00 +0800
> Peter Pan <peterpandong@micron.com> wrote:
>
>> This commit adds SPI NAND command set and register
>> definition according Micron SPI NAND data sheet.
>>
>> Signed-off-by: Peter Pan <peterpandong@micron.com>
>> ---
>>  include/linux/mtd/spi-nand.h | 112 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 112 insertions(+)
>>  create mode 100644 include/linux/mtd/spi-nand.h
>>
>> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
>> new file mode 100644
>> index 0000000..68442e08
>> --- /dev/null
>> +++ b/include/linux/mtd/spi-nand.h
>> @@ -0,0 +1,112 @@
>> +/**
>> +* spi-nand.h
>> +*
>> +* 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_SPI_NAND_H
>> +#define __LINUX_MTD_SPI_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_PAGE_CACHE_RDM              0x30
>> +#define SPINAND_CMD_READ_PAGE_CACHE_LAST     0x3f
>> +#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
>> +#define SPINAND_CMD_END                              0x0
>> +
>> +
>> +/* feature registers */
>> +#define REG_BLOCK_LOCK               0xa0
>> +#define REG_CFG                      0xb0
>> +#define REG_STATUS           0xc0
>> +#define REG_DIE_SELECT               0xd0
>> +
>> +/* status */
>> +#define STATUS_OIP_MASK              0x01
>> +#define STATUS_CRBSY_MASK    0x80
>> +#define STATUS_READY         (0 << 0)
>> +#define STATUS_BUSY          (1 << 0)
>> +
>> +#define STATUS_E_FAIL_MASK   0x04
>> +#define STATUS_E_FAIL                (1 << 2)
>> +
>> +#define STATUS_P_FAIL_MASK   0x08
>> +#define STATUS_P_FAIL                (1 << 3)
>> +
>> +
>> +/*Configuration register defines*/
>> +#define CFG_QE_MASK          0x01
>> +#define CFG_QE_ENABLE                0x01
>> +#define CFG_ECC_MASK         0x10
>> +#define CFG_ECC_ENABLE               0x10
>> +#define CFG_LOT_MASK         0x20
>> +#define CFG_LOT_ENABLE               0x20
>> +#define CFG_OTP_MASK         0xc2
>> +#define CFG_OTP_ENTER                0x40
>> +#define CFG_OTP_EXIT         0x00
>> +
>> +/* block lock */
>> +#define BL_ALL_LOCKED                0x7c
>> +#define BL_U_1_1024_LOCKED           0x08
>> +#define BL_U_1_512_LOCKED            0x10
>> +#define BL_U_1_256_LOCKED            0x18
>> +#define BL_U_1_128_LOCKED            0x20
>> +#define BL_U_1_64_LOCKED             0x28
>> +#define BL_U_1_32_LOCKED             0x30
>> +#define BL_U_1_16_LOCKED             0x38
>> +#define BL_U_1_8_LOCKED              0x40
>> +#define BL_U_1_4_LOCKED              0x48
>> +#define BL_U_1_2_LOCKED              0x50
>> +#define BL_L_1_1024_LOCKED           0x0c
>> +#define BL_L_1_512_LOCKED            0x14
>> +#define BL_L_1_256_LOCKED            0x1c
>> +#define BL_L_1_128_LOCKED            0x24
>> +#define BL_L_1_64_LOCKED             0x2c
>> +#define BL_L_1_32_LOCKED             0x34
>> +#define BL_L_1_16_LOCKED             0x3c
>> +#define BL_L_1_8_LOCKED              0x44
>> +#define BL_L_1_4_LOCKED              0x4c
>> +#define BL_L_1_2_LOCKED              0x54
>> +#define BL_ALL_UNLOCKED              0X00
>> +
>> +/* die select */
>> +#define DIE_SELECT_MASK              0x40
>> +#define DIE_SELECT_DS0               0x00
>> +#define DIE_SELECT_DS1               0x40
>> +
>> +#define SPI_NAND_MT29F_ECC_MASK              0x70
>> +#define SPI_NAND_MT29F_ECC_0_BIT     0x00
>> +#define SPI_NAND_MT29F_ECC_1_3_BIT   0x10
>> +#define SPI_NAND_MT29F_ECC_4_6_BIT   0x30
>> +#define SPI_NAND_MT29F_ECC_7_8_BIT   0x50
>> +#define SPI_NAND_MT29F_ECC_UNCORR    0x20
>
> These are manufacturer specific definitions, and I don't think they
> should be exposed in the generic header.

Totally agree. Fix this in v2

Thanks,
Peter Pan

Re: [PATCH 10/11] nand: spi: Add generic SPI controller support

[Arnaud Mouiche]

On 21/02/2017 09:00, 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/chips/Kconfig       |   5 +
>   drivers/mtd/nand/spi/chips/Makefile      |   1 +
>   drivers/mtd/nand/spi/chips/generic_spi.c | 269 +++++++++++++++++++++++++++++++
>   5 files changed, 278 insertions(+)
>   create mode 100644 drivers/mtd/nand/spi/chips/Kconfig
>   create mode 100644 drivers/mtd/nand/spi/chips/Makefile
>   create mode 100644 drivers/mtd/nand/spi/chips/generic_spi.c
[...]
> +/*
> + * generic_spi_nand_cmd_fn - to process a command to send to the SPI-NAND
> + * by generic SPI bus
> + * @chip: SPI-NAND device structure
> + * @cmd: command structure
> + * Description:
> + *   Set up the command buffer to send to the SPI controller.
> + *   The command buffer has to initialized to 0.
> + */
> +static int generic_spi_nand_cmd_fn(struct spi_nand_chip *chip,
> +				struct spi_nand_cmd *cmd)
> +{
> +	struct spi_nand_cmd_cfg *cmd_cfg = spi_nand_get_cmd_cfg(cmd->cmd);
> +	struct spi_message message;
> +	struct spi_transfer x[3];
> +	struct spi_device *spi = chip->priv;
> +	u8 buf[SPINAND_MAX_ADDR_LEN];
> +
> +	spi_message_init(&message);
> +	memset(x, 0, sizeof(x));
> +	x[0].len = 1;
> +	x[0].tx_nbits = 1;
> +	x[0].tx_buf = &cmd->cmd;
> +	spi_message_add_tail(&x[0], &message);
> +
> +	if (cmd_cfg->addr_bytes || cmd_cfg->dummy_bytes) {
> +		if (cmd_cfg->addr_bytes > cmd->n_addr) {
> +			memcpy(buf, cmd->addr, cmd->n_addr);
> +			memset(cmd->addr, 0, cmd->n_addr);
> +			memcpy(cmd->addr + cmd_cfg->addr_bytes - cmd->n_addr,
> +				buf, cmd->n_addr);
> +		}
> +		x[1].len = cmd_cfg->addr_bytes + cmd_cfg->dummy_bytes;
> +		x[1].tx_nbits = cmd_cfg->addr_bits;
> +		x[1].tx_buf = cmd->addr;
> +		spi_message_add_tail(&x[1], &message);
> +	}
> +	if (cmd->n_tx) {
> +		x[2].len = cmd->n_tx;
> +		x[2].tx_nbits = cmd_cfg->data_bits;
> +		x[2].tx_buf = cmd->tx_buf;
> +		spi_message_add_tail(&x[2], &message);
> +	} else if (cmd->n_rx) {
> +		x[2].len = cmd->n_rx;
> +		x[2].rx_nbits = cmd_cfg->data_bits;
> +		x[2].rx_buf = cmd->rx_buf;
> +		spi_message_add_tail(&x[2], &message);
> +	}
> +	return spi_sync(spi, &message);
> +}
> +
>
Just a spi speed consideration.
All the spi drivers I've work with are not very good for scatter/gather, 
and performance are dropping especially when you gather only 1 to 3 
bytes parts.
But we can manage this optimization later anyway, with proper speed 
figures, once the spinand framework will be merged.

Arnaud

Re: [PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Peter Pan]

Hi Boris,

On Tue, Feb 21, 2017 at 5:06 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 16:00:02 +0800
> Peter Pan <peterpandong@micron.com> wrote:
>
>> This commit abstracts basic SPI NAND commands to
>> functions in spi-nand-base.c. Because command sets
>> have difference by vendors, we create spi-nand-cmd.c
>> to define this difference by command config table.
>>
>> 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        |   2 +
>>  drivers/mtd/nand/spi/spi-nand-base.c | 368 +++++++++++++++++++++++++++++++++++
>>  drivers/mtd/nand/spi/spi-nand-cmd.c  |  69 +++++++
>>  include/linux/mtd/spi-nand.h         |  34 ++++
>>  7 files changed, 480 insertions(+)
>>  create mode 100644 drivers/mtd/nand/spi/Kconfig
>>  create mode 100644 drivers/mtd/nand/spi/Makefile
>>  create mode 100644 drivers/mtd/nand/spi/spi-nand-base.c
>>  create mode 100644 drivers/mtd/nand/spi/spi-nand-cmd.c
>>
>> 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..04a7b71
>> --- /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..3c617d6
>> --- /dev/null
>> +++ b/drivers/mtd/nand/spi/Makefile
>> @@ -0,0 +1,2 @@
>> +obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-base.o
>> +obj-$(CONFIG_MTD_SPI_NAND) += spi-nand-cmd.o
>> diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
>> new file mode 100644
>> index 0000000..b75e5cd
>> --- /dev/null
>> +++ b/drivers/mtd/nand/spi/spi-nand-base.c
>> @@ -0,0 +1,368 @@
>> +/**
>> +* spi-nand-base.c
>> +*
>> +* 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.
>> +*/
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/mtd/spi-nand.h>
>> +
>> +static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
>> +                             struct spi_nand_cmd *cmd)
>> +{
>> +     return chip->command_fn(chip, cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_read_reg - send command 0Fh to read register
>> + * @chip: SPI-NAND device structure
>> + * @reg; register to read
>> + * @buf: buffer to store value
>> + */
>> +static int spi_nand_read_reg(struct spi_nand_chip *chip,
>> +                     uint8_t reg, uint8_t *buf)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +     int ret;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_GET_FEATURE;
>> +     cmd.n_addr = 1;
>> +     cmd.addr[0] = reg;
>> +     cmd.n_rx = 1;
>> +     cmd.rx_buf = buf;
>> +
>> +     ret = spi_nand_issue_cmd(chip, &cmd);
>> +     if (ret < 0)
>> +             pr_err("err: %d read register %d\n", ret, reg);
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * spi_nand_write_reg - send command 1Fh to write register
>> + * @chip: SPI-NAND device structure
>> + * @reg; register to write
>> + * @buf: buffer stored value
>> + */
>> +static int spi_nand_write_reg(struct spi_nand_chip *chip,
>> +                     uint8_t reg, uint8_t *buf)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +     int ret;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_SET_FEATURE;
>> +     cmd.n_addr = 1;
>> +     cmd.addr[0] = reg;
>> +     cmd.n_tx = 1,
>> +     cmd.tx_buf = buf,
>> +
>> +     ret = spi_nand_issue_cmd(chip, &cmd);
>> +     if (ret < 0)
>> +             pr_err("err: %d write register %d\n", ret, reg);
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * spi_nand_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.
>> + *   Once the status turns to be ready, the other status bits also are
>> + *   valid status bits.
>> + */
>> +static int spi_nand_read_status(struct spi_nand_chip *chip, uint8_t *status)
>> +{
>> +     return spi_nand_read_reg(chip, REG_STATUS, status);
>> +}
>> +
>> +/**
>> + * spi_nand_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 spi_nand_get_cfg(struct spi_nand_chip *chip, u8 *cfg)
>> +{
>> +     return spi_nand_read_reg(chip, REG_CFG, cfg);
>> +}
>> +
>> +/**
>> + * spi_nand_set_cfg - set value to configuration register
>> + * @chip: SPI-NAND device structure
>> + * @cfg: buffer stored value
>> + * Description:
>> + *   Configuration register includes OTP config, Lock Tight enable/disable
>> + *   and Internal ECC enable/disable.
>> + */
>> +static int spi_nand_set_cfg(struct spi_nand_chip *chip, u8 *cfg)
>> +{
>> +     return spi_nand_write_reg(chip, REG_CFG, cfg);
>> +}
>> +
>> +/**
>> + * spi_nand_enable_ecc - enable internal ECC
>> + * @chip: SPI-NAND device structure
>> + * Description:
>> + *   There is one bit( bit 0x10 ) to set or to clear the internal ECC.
>> + *   Enable chip internal ECC, set the bit to 1
>> + *   Disable chip internal ECC, clear the bit to 0
>> + */
>> +static void spi_nand_enable_ecc(struct spi_nand_chip *chip)
>> +{
>> +     u8 cfg = 0;
>> +
>> +     spi_nand_get_cfg(chip, &cfg);
>> +     if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE)
>> +             return;
>> +     cfg |= CFG_ECC_ENABLE;
>> +     spi_nand_set_cfg(chip, &cfg);
>> +}
>> +
>> +/**
>> + * spi_nand_disable_ecc - disable internal ECC
>> + * @chip: SPI-NAND device structure
>> + * Description:
>> + *   There is one bit( bit 0x10 ) to set or to clear the internal ECC.
>> + *   Enable chip internal ECC, set the bit to 1
>> + *   Disable chip internal ECC, clear the bit to 0
>> + */
>> +static void spi_nand_disable_ecc(struct spi_nand_chip *chip)
>> +{
>> +     u8 cfg = 0;
>> +
>> +     spi_nand_get_cfg(chip, &cfg);
>> +     if ((cfg & CFG_ECC_MASK) == CFG_ECC_ENABLE) {
>> +             cfg &= ~CFG_ECC_ENABLE;
>> +             spi_nand_set_cfg(chip, &cfg);
>> +     }
>> +}
>> +
>> +/**
>> + * spi_nand_write_enable - send command 06h to enable write or erase the
>> + * Nand cells
>> + * @chip: SPI-NAND device structure
>> + * Description:
>> + *   Before write and erase the Nand cells, the write enable has to be set.
>> + *   After the write or erase, the write enable bit is automatically
>> + *   cleared (status register bit 2)
>> + *   Set the bit 2 of the status register has the same effect
>> + */
>> +static int spi_nand_write_enable(struct spi_nand_chip *chip)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_WR_ENABLE;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
>> + * to cache
>> + * @chip: SPI-NAND device structure
>> + * @page_addr: page to read
>> + */
>> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
>> +                                     u32 page_addr)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_PAGE_READ;
>> +     cmd.n_addr = 3;
>> +     cmd.addr[0] = (u8)(page_addr >> 16);
>> +     cmd.addr[1] = (u8)(page_addr >> 8);
>> +     cmd.addr[2] = (u8)page_addr;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_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
>> + * Description:
>> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
>> + *   The read can specify 1 to (page size + spare size) bytes of data read at
>> + *   the corresponding locations.
>> + *   No tRd delay.
>> + */
>> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
>> +             u32 page_addr, u32 column, size_t len, u8 *rbuf)
>> +{
>> +     struct nand_device *nand = &chip->base;
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = chip->read_cache_op;
>> +     cmd.n_addr = 2;
>> +     cmd.addr[0] = (u8)(column >> 8);
>> +     if (chip->options & SPINAND_NEED_PLANE_SELECT)
>> +             cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
>> +                             & 0x1) << 4);
>> +     cmd.addr[1] = (u8)column;
>> +     cmd.n_rx = len;
>> +     cmd.rx_buf = rbuf;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_program_data_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
>> + * @clr_cache: clear cache register or not
>> + * Description:
>> + *   Command can be 02h, 32h, 84h, 34h
>> + *   02h and 32h will clear the cache with 0xff value first
>> + *   Since it is writing the data to cache, there is no tPROG time.
>> + */
>> +static int spi_nand_program_data_to_cache(struct spi_nand_chip *chip,
>> +                     u32 page_addr, u32 column, size_t len, const u8 *wbuf)
>> +{
>> +     struct nand_device *nand = &chip->base;
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = chip->write_cache_op;
>> +     cmd.n_addr = 2;
>> +     cmd.addr[0] = (u8)(column >> 8);
>> +     if (chip->options & SPINAND_NEED_PLANE_SELECT)
>> +             cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand, page_addr)
>> +                             & 0x1) << 4);
>> +     cmd.addr[1] = (u8)column;
>> +     cmd.n_tx = len;
>> +     cmd.tx_buf = wbuf;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_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.
>> + * Description:
>> + *   Need to wait for tPROG time to finish the transaction.
>> + */
>> +static int spi_nand_program_execute(struct spi_nand_chip *chip, u32 page_addr)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_PROG_EXC;
>> +     cmd.n_addr = 3;
>> +     cmd.addr[0] = (u8)(page_addr >> 16);
>> +     cmd.addr[1] = (u8)(page_addr >> 8);
>> +     cmd.addr[2] = (u8)page_addr;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +
>> +/**
>> + * spi_nand_erase_block_erase - send command D8h to erase a block
>> + * @chip: SPI-NAND device structure
>> + * @page_addr: the page to erase.
>> + * Description:
>> + *   Need to wait for tERS.
>> + */
>> +static int spi_nand_erase_block(struct spi_nand_chip *chip,
>> +                                     u32 page_addr)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_BLK_ERASE;
>> +     cmd.n_addr = 3;
>> +     cmd.addr[0] = (u8)(page_addr >> 16);
>> +     cmd.addr[1] = (u8)(page_addr >> 8);
>> +     cmd.addr[2] = (u8)page_addr;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_read_id - send 9Fh command to get ID
>> + * @chip: SPI-NAND device structure
>> + * @buf: buffer to store id
>> + */
>> +static int spi_nand_read_id(struct spi_nand_chip *chip, u8 *buf)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_READ_ID;
>> +     cmd.n_rx = 2;
>> +     cmd.rx_buf = buf;
>> +
>> +     return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>> +/**
>> + * spi_nand_reset - send command FFh to reset chip.
>> + * @chip: SPI-NAND device structure
>> + */
>> +static int spi_nand_reset(struct spi_nand_chip *chip)
>> +{
>> +     struct spi_nand_cmd cmd;
>> +
>> +     memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +     cmd.cmd = SPINAND_CMD_RESET;
>> +
>> +     if (spi_nand_issue_cmd(chip, &cmd) < 0)
>> +             pr_err("spi_nand reset failed!\n");
>> +
>> +     /* elapse 2ms before issuing any other command */
>> +     udelay(2000);
>
> 2ms looks huge. Are you sure you can't poll the status register before
> that?

I just checked with data sheet. We can poll the status register, Thanks
for you comments. Fix this in v2.

>
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * spi_nand_lock_block - write block lock register to
>> + * lock/unlock device
>> + * @spi: spi device structure
>> + * @lock: value to set to block lock register
>> + * Description:
>> + *   After power up, all the Nand blocks are locked.  This function allows
>> + *   one to unlock the blocks, and so it can be written or erased.
>> + */
>> +static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
>> +{
>> +     return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
>> +}
>> +
>> +MODULE_DESCRIPTION("SPI NAND framework");
>> +MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mtd/nand/spi/spi-nand-cmd.c b/drivers/mtd/nand/spi/spi-nand-cmd.c
>> new file mode 100644
>> index 0000000..d63741e
>> --- /dev/null
>> +++ b/drivers/mtd/nand/spi/spi-nand-cmd.c
>> @@ -0,0 +1,69 @@
>> +/**
>> +* spi-nand-cmd.c
>> +*
>> +* 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/mtd/spi-nand.h>
>> +#include <linux/kernel.h>
>> +
>> +static struct spi_nand_cmd_cfg *cmd_table;
>> +
>> +static struct spi_nand_cmd_cfg micron_cmd_cfg_table[] = {
>
> static const

Fix this in v2.

>
>> +/*opcode     addr_bytes      addr_bits       dummy_bytes     data_nbits*/
>> +     {SPINAND_CMD_GET_FEATURE,               1,      1,      0,      1},
>> +     {SPINAND_CMD_SET_FEATURE,               1,      1,      0,      1},
>> +     {SPINAND_CMD_PAGE_READ,                 3,      1,      0,      0},
>> +     {SPINAND_CMD_READ_FROM_CACHE,           2,      1,      1,      1},
>> +     {SPINAND_CMD_READ_FROM_CACHE_FAST,      2,      1,      1,      1},
>> +     {SPINAND_CMD_READ_FROM_CACHE_X2,        2,      1,      1,      2},
>> +     {SPINAND_CMD_READ_FROM_CACHE_DUAL_IO,   2,      2,      1,      2},
>> +     {SPINAND_CMD_READ_FROM_CACHE_X4,        2,      1,      1,      4},
>> +     {SPINAND_CMD_READ_FROM_CACHE_QUAD_IO,   2,      4,      2,      4},
>> +     {SPINAND_CMD_BLK_ERASE,                 3,      1,      0,      0},
>> +     {SPINAND_CMD_PROG_EXC,                  3,      1,      0,      0},
>> +     {SPINAND_CMD_PROG_LOAD,                 2,      1,      0,      1},
>> +     {SPINAND_CMD_PROG_LOAD_RDM_DATA,        2,      1,      0,      1},
>> +     {SPINAND_CMD_PROG_LOAD_X4,              2,      1,      0,      4},
>> +     {SPINAND_CMD_PROG_LOAD_RDM_DATA_X4,     2,      1,      0,      4},
>> +     {SPINAND_CMD_WR_ENABLE,                 0,      0,      0,      0},
>> +     {SPINAND_CMD_WR_DISABLE,                0,      0,      0,      0},
>> +     {SPINAND_CMD_READ_ID,                   0,      0,      1,      1},
>> +     {SPINAND_CMD_RESET,                     0,      0,      0,      0},
>> +     {SPINAND_CMD_END},
>> +};
>> +
>> +struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode)
>> +{
>> +     struct spi_nand_cmd_cfg *index = cmd_table;
>> +
>> +     for (; index->opcode != SPINAND_CMD_END; index++) {
>> +             if (index->opcode == opcode)
>> +                     return index;
>> +     }
>> +
>> +     pr_err("Invalid spi nand opcode %x\n", opcode);
>> +     BUG();
>> +}
>> +
>> +int spi_nand_set_cmd_cfg_table(int mfr)
>> +{
>> +     switch (mfr) {
>> +     case SPINAND_MFR_MICRON:
>> +             cmd_table = micron_cmd_cfg_table;
>> +             break;
>
> As said earlier, I really don't want to reproduce the errors done in
> the rawnand framework, so I'd like to separate manufacturer specific
> things into separate source files, and let this manufacturer-drivers
> initialize things in a dedicated ->init() function (see the rework I've
> done here [1] for the // NAND framework).

Really valuable comment. I will check with your rework.

>
>> +     default:
>> +             pr_err("Unknown device\n");
>> +             return -ENODEV;
>> +     }
>> +     return 0;
>> +}
>> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
>> index 23b16f0..1fcbad7 100644
>> --- a/include/linux/mtd/spi-nand.h
>> +++ b/include/linux/mtd/spi-nand.h
>> @@ -115,6 +115,9 @@
>>  #define SPI_NAND_MT29F_ECC_7_8_BIT   0x50
>>  #define SPI_NAND_MT29F_ECC_UNCORR    0x20
>>
>> +
>> +struct spi_nand_cmd;
>> +
>>  /**
>>   * struct spi_nand_chip - SPI-NAND Private Flash Chip Data
>>   * @base:            [INTERN] NAND device instance
>> @@ -128,6 +131,7 @@
>>   * @state:           [INTERN] the current state of the SPI-NAND device
>>   * @read_cache_op:   [REPLACEABLE] Opcode of read from cache
>>   * @write_cache_op:  [REPLACEABLE] Opcode of program load
>> + * @command_fn:              [BOARDSPECIFIC] function to handle command transfer
>>   * @buf:             [INTERN] buffer for read/write data
>>   * @oobbuf:          [INTERN] buffer for read/write oob
>>   * @controller_caps: [INTERN] capacities of SPI NAND controller
>> @@ -147,6 +151,8 @@ struct spi_nand_chip {
>>       flstate_t       state;
>>       u8              read_cache_op;
>>       u8              write_cache_op;
>> +     int (*command_fn)(struct spi_nand_chip *this,
>> +                     struct spi_nand_cmd *cmd);
>
> Nope, let's try to not do the same mistake we did in the raw (parallel)
> NAND framework.
>
> This seems to be a controller specific hook. You should define
>
> struct spinand_controller_ops {
>         int (*exec_op)(struct spinand_device *nand,
>                        struct spinand_op *op);
> };
>
> Then have a
>
> struct spinand_controller {
>         struct spinand_controller_ops *ops;
>         u32 caps;
>         /* other controller fields */
> };
>
> and inside you spinand_device struct:
>
> struct spinand_device {
>         struct spinand_controller *controller;
>         void *controller_priv;
> };
>
> I know it goes against my suggestion to put controller caps and priv
> into a sub-struct inside the spinand_device struct, but it seems that
> you really need to clearly separate the spinand_device and
> spinand_controller objects.

Really valuable comment. Again, I should send this out earlier!!! I also
realized I didn't separate controller with device. I think I need to review
your NAND framework again before making v2.

>
>>       u8              *buf;
>>       u8              *oobbuf;
>>       u32             controller_caps;
>> @@ -176,4 +182,32 @@ static inline void spi_nand_set_controller_data(struct spi_nand_chip *chip,
>>  {
>>       chip->priv = priv;
>>  }
>> +
>> +#define SPINAND_MAX_ADDR_LEN         4
>> +
>> +struct spi_nand_cmd {
>
> struct spinand_op (op stands for operation), to avoid confusion with the
> cmd field in this structure.

Good suggestion.

>
>> +     u8              cmd;
>> +     u8              n_addr;         /* Number of address */
>> +     u8              addr[SPINAND_MAX_ADDR_LEN];     /* Reg Offset */
>> +     u32             n_tx;           /* Number of tx bytes */
>> +     const u8        *tx_buf;        /* Tx buf */
>> +     u32             n_rx;           /* Number of rx bytes */
>> +     u8              *rx_buf;        /* Rx buf */
>> +};
>> +
>> +struct spi_nand_cmd_cfg {
>
> spinand_op_def?

Better  name.

>
>> +     u8              opcode;
>> +     u8              addr_bytes;
>> +     u8              addr_bits;
>> +     u8              dummy_bytes;
>> +     u8              data_bits;
>> +};
>> +
>> +/*SPI NAND chip options*/
>> +#define SPINAND_NEED_PLANE_SELECT    (1 << 0)
>> +
>> +/*SPI NAND manufacture ID definition*/
>> +#define SPINAND_MFR_MICRON           0x2C
>> +int spi_nand_set_cmd_cfg_table(int mfr);
>> +struct spi_nand_cmd_cfg *spi_nand_get_cmd_cfg(u8 opcode);
>>  #endif /* __LINUX_MTD_SPI_NAND_H */
>
> [1]https://lkml.org/lkml/2017/1/9/169

Re: [PATCH 10/11] nand: spi: Add generic SPI controller support

[Peter Pan]

Hi Arnaud,

On Tue, Feb 21, 2017 at 5:25 PM, Arnaud Mouiche
<arnaud.mouiche@gmail.com> wrote:
>
>
> On 21/02/2017 09:00, 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/chips/Kconfig       |   5 +
>>   drivers/mtd/nand/spi/chips/Makefile      |   1 +
>>   drivers/mtd/nand/spi/chips/generic_spi.c | 269
>> +++++++++++++++++++++++++++++++
>>   5 files changed, 278 insertions(+)
>>   create mode 100644 drivers/mtd/nand/spi/chips/Kconfig
>>   create mode 100644 drivers/mtd/nand/spi/chips/Makefile
>>   create mode 100644 drivers/mtd/nand/spi/chips/generic_spi.c
>
> [...]
>
>> +/*
>> + * generic_spi_nand_cmd_fn - to process a command to send to the SPI-NAND
>> + * by generic SPI bus
>> + * @chip: SPI-NAND device structure
>> + * @cmd: command structure
>> + * Description:
>> + *   Set up the command buffer to send to the SPI controller.
>> + *   The command buffer has to initialized to 0.
>> + */
>> +static int generic_spi_nand_cmd_fn(struct spi_nand_chip *chip,
>> +                               struct spi_nand_cmd *cmd)
>> +{
>> +       struct spi_nand_cmd_cfg *cmd_cfg = spi_nand_get_cmd_cfg(cmd->cmd);
>> +       struct spi_message message;
>> +       struct spi_transfer x[3];
>> +       struct spi_device *spi = chip->priv;
>> +       u8 buf[SPINAND_MAX_ADDR_LEN];
>> +
>> +       spi_message_init(&message);
>> +       memset(x, 0, sizeof(x));
>> +       x[0].len = 1;
>> +       x[0].tx_nbits = 1;
>> +       x[0].tx_buf = &cmd->cmd;
>> +       spi_message_add_tail(&x[0], &message);
>> +
>> +       if (cmd_cfg->addr_bytes || cmd_cfg->dummy_bytes) {
>> +               if (cmd_cfg->addr_bytes > cmd->n_addr) {
>> +                       memcpy(buf, cmd->addr, cmd->n_addr);
>> +                       memset(cmd->addr, 0, cmd->n_addr);
>> +                       memcpy(cmd->addr + cmd_cfg->addr_bytes -
>> cmd->n_addr,
>> +                               buf, cmd->n_addr);
>> +               }
>> +               x[1].len = cmd_cfg->addr_bytes + cmd_cfg->dummy_bytes;
>> +               x[1].tx_nbits = cmd_cfg->addr_bits;
>> +               x[1].tx_buf = cmd->addr;
>> +               spi_message_add_tail(&x[1], &message);
>> +       }
>> +       if (cmd->n_tx) {
>> +               x[2].len = cmd->n_tx;
>> +               x[2].tx_nbits = cmd_cfg->data_bits;
>> +               x[2].tx_buf = cmd->tx_buf;
>> +               spi_message_add_tail(&x[2], &message);
>> +       } else if (cmd->n_rx) {
>> +               x[2].len = cmd->n_rx;
>> +               x[2].rx_nbits = cmd_cfg->data_bits;
>> +               x[2].rx_buf = cmd->rx_buf;
>> +               spi_message_add_tail(&x[2], &message);
>> +       }
>> +       return spi_sync(spi, &message);
>> +}
>> +
>>
> Just a spi speed consideration.
> All the spi drivers I've work with are not very good for scatter/gather, and
> performance are dropping especially when you gather only 1 to 3 bytes parts.
> But we can manage this optimization later anyway, with proper speed figures,
> once the spinand framework will be merged.

I also found the speed issue when debugging. For opcode and address transfer
and get/set feature, CPU polling is better than DMA while CPU resource
is wasted.

Thanks,
Peter Pan

Re: [PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Peter Pan]

Hi Arnaud,

On Tue, Feb 21, 2017 at 5:01 PM, Arnaud Mouiche
<arnaud.mouiche@gmail.com> wrote:
> Hello Peter.
> First thank you for this effort to finally bring the spinand framework back
> on stage.
>
> On 21/02/2017 09:00, Peter Pan wrote:
>>
>> This commit abstracts basic SPI NAND commands to
>> functions in spi-nand-base.c. Because command sets
>> have difference by vendors, we create spi-nand-cmd.c
>> to define this difference by command config table.
>>
>> [...]
>> +
>> +/**
>> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
>> + * to cache
>> + * @chip: SPI-NAND device structure
>> + * @page_addr: page to read
>> + */
>> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
>> +                                       u32 page_addr)
>> +{
>> +       struct spi_nand_cmd cmd;
>> +
>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +       cmd.cmd = SPINAND_CMD_PAGE_READ;
>> +       cmd.n_addr = 3;
>> +       cmd.addr[0] = (u8)(page_addr >> 16);
>> +       cmd.addr[1] = (u8)(page_addr >> 8);
>> +       cmd.addr[2] = (u8)page_addr;
>> +
>> +       return spi_nand_issue_cmd(chip, &cmd);
>> +}
>
>
> Just a question. Don't you try to map too exactly the spi_nand_cmd to the
> internal micron SPI command ?
> Why "cmd.addr" should be a byte array, and not a u32, leaving each chip to
> details to handle the u32 to SPI byte stream mapping.
>
>
>
>> +
>> +/**
>> + * spi_nand_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
>> + * Description:
>> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
>> + *   The read can specify 1 to (page size + spare size) bytes of data
>> read at
>> + *   the corresponding locations.
>> + *   No tRd delay.
>> + */
>> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
>> +               u32 page_addr, u32 column, size_t len, u8 *rbuf)
>> +{
>> +       struct nand_device *nand = &chip->base;
>> +       struct spi_nand_cmd cmd;
>> +
>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +       cmd.cmd = chip->read_cache_op;
>> +       cmd.n_addr = 2;
>> +       cmd.addr[0] = (u8)(column >> 8);
>> +       if (chip->options & SPINAND_NEED_PLANE_SELECT)
>> +               cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand,
>> page_addr)
>> +                               & 0x1) << 4);
>> +       cmd.addr[1] = (u8)column;
>> +       cmd.n_rx = len;
>> +       cmd.rx_buf = rbuf;
>> +
>> +       return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>
> Some reasons to handle the plane concept (Micron specific), and even move
> the fact that it touch the bit 4 of cdm.addr[0], in common code ?

Let me try to remove this from framework when separate controller and device
related code. Any suggestion on this?

Thanks,
Peter Pan

Re: [PATCH 04/11] nand: spi: Add read function support

[Boris Brezillon]

On Tue, 21 Feb 2017 16:00:03 +0800
Peter Pan <peterpandong@micron.com> wrote:

> Add read/readoob support for SPI NAND. Will be
> registered under struct mtd_info later.
> 
> Signed-off-by: Peter Pan <peterpandong@micron.com>
> ---
>  drivers/mtd/nand/spi/spi-nand-base.c | 321 +++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nand.h         |   3 +
>  2 files changed, 324 insertions(+)
> 
> diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
> index b75e5cd..ee94eb8 100644
> --- a/drivers/mtd/nand/spi/spi-nand-base.c
> +++ b/drivers/mtd/nand/spi/spi-nand-base.c
> @@ -18,9 +18,62 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/sched.h>
>  #include <linux/delay.h>
>  #include <linux/mtd/spi-nand.h>
>  
> +/**
> + * spi_nand_get_device - [GENERIC] Get chip for selected access
> + * @mtd: MTD device structure
> + * @new_state: the state which is requested
> + *
> + * Get the device and lock it for exclusive access
> + */
> +static int spi_nand_get_device(struct mtd_info *mtd, int new_state)
> +{
> +	struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
> +	DECLARE_WAITQUEUE(wait, current);
> +
> +	/*
> +	 * Grab the lock and see if the device is available
> +	 */
> +	while (1) {
> +		spin_lock(&this->chip_lock);
> +		if (this->state == FL_READY) {
> +			this->state = new_state;
> +			spin_unlock(&this->chip_lock);
> +			break;
> +		}
> +		if (new_state == FL_PM_SUSPENDED) {
> +			spin_unlock(&this->chip_lock);
> +			return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
> +		}
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		add_wait_queue(&this->wq, &wait);
> +		spin_unlock(&this->chip_lock);
> +		schedule();
> +		remove_wait_queue(&this->wq, &wait);
> +	}

Do we really need this fl_state dance? It could be replaced with a
simple mutex that is taken for the whole flash operation. This leaves
the suspended case which could be handled with:

static void spinand_suspend(struct mtd_info *mtd)
{
	struct spi_nand_chip *this = mtd_to_spinand(mtd);

	if (!mutex_trylock(&this->lock))
		return -EAGAIN;

	return 0;
}

For other operations (read, write, lock, unlock, ...), you just have to
take the lock before accessing the NAND and release it when you're done.

Again, I'm saying that because I don't want to end up with something
that is overly complex for no real reason, but maybe I'm missing
something. 

> +	return 0;
> +}
> +
> +/**
> + * spi_nand_release_device - [GENERIC] release chip
> + * @mtd: MTD device structure
> + *
> + * Deselect, release chip lock and wake up anyone waiting on the device
> + */
> +static void spi_nand_release_device(struct mtd_info *mtd)
> +{
> +	struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
> +
> +	/* Release the chip */
> +	spin_lock(&this->chip_lock);
> +	this->state = FL_READY;
> +	wake_up(&this->wq);
> +	spin_unlock(&this->chip_lock);
> +}
> +
>  static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
>  				struct spi_nand_cmd *cmd)
>  {
> @@ -313,6 +366,36 @@ static int spi_nand_erase_block(struct spi_nand_chip *chip,
>  }
>  
>  /**
> + * spi_nand_wait - wait until the command is done
> + * @chip: SPI-NAND device structure
> + * @s: buffer to store status register(can be NULL)
> + */
> +static int spi_nand_wait(struct spi_nand_chip *chip, u8 *s)
> +{
> +	unsigned long timeo = jiffies;
> +	u8 status;
> +	int ret = -ETIMEDOUT;
> +	int count = 0;
> +
> +	#define MIN_TRY_COUNT 3

Please, no macro definitions inside functions.

> +	timeo += msecs_to_jiffies(20);
> +
> +	while (time_before(jiffies, timeo) || count < MIN_TRY_COUNT) {
> +		spi_nand_read_status(chip, &status);
> +		if ((status & STATUS_OIP_MASK) == STATUS_READY) {
> +			ret = 0;
> +			goto out;
> +		}
> +		count++;
> +	}


Hm, if your SPI controller is not able to read 3 times the status reg
in 20ms it means it's really slow.

I'd recommend doing something like:

	do {
		spi_nand_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 out last check
	 */
	spi_nand_read_status(chip, &status);

> +out:
> +	if (s)
> +		*s = status;
> +
> +	return ret;

	return (status & STATUS_OIP_MASK) == STATUS_READY ? 0 :	-ETIMEDOUT;

> +}
> +
> +/**
>   * spi_nand_read_id - send 9Fh command to get ID
>   * @chip: SPI-NAND device structure
>   * @buf: buffer to store id
> @@ -363,6 +446,244 @@ static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
>  	return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
>  }
>  
> +/**
> + * spi_nand_do_read_page - read page from flash to buffer
> + * @mtd: MTD device structure
> + * @page_addr: page address/raw address
> + * @column: column address
> + * @ecc_off: without ecc or not
> + * @corrected: how many bit error corrected
> + * @buf: data buffer
> + * @len: data length to read
> + */
> +static int spi_nand_do_read_page(struct mtd_info *mtd, u32 page_addr,
> +			bool ecc_off, int *corrected, bool oob_only)
> +{
> +	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	int ret, ecc_error = 0;
> +	u8 status;
> +
> +	spi_nand_read_page_to_cache(chip, page_addr);
> +	ret = spi_nand_wait(chip, &status);
> +	if (ret < 0) {
> +		pr_err("error %d waiting page 0x%x to cache\n",
> +			ret, page_addr);
> +		return ret;
> +	}
> +	if (!oob_only)
> +		spi_nand_read_from_cache(chip, page_addr, 0,
> +		nand_page_size(nand) + nand_per_page_oobsize(nand), chip->buf);
> +	else
> +		spi_nand_read_from_cache(chip, page_addr, nand_page_size(nand),
> +				nand_per_page_oobsize(nand), chip->oobbuf);
> +	if (!ecc_off) {
> +		chip->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) {
> +			pr_err("internal ECC error reading page 0x%x\n",
> +				page_addr);
> +			mtd->ecc_stats.failed++;
> +		} else if (*corrected) {
> +			mtd->ecc_stats.corrected += *corrected;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * spi_nand_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 void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
> +				  struct mtd_oob_ops *ops, size_t len)
> +{
> +	struct mtd_info *mtd = spi_nand_to_mtd(chip);
> +	int ret;
> +
> +	switch (ops->mode) {
> +
> +	case MTD_OPS_PLACE_OOB:
> +	case MTD_OPS_RAW:
> +		memcpy(oob, chip->oobbuf + ops->ooboffs, len);
> +		return;
> +
> +	case MTD_OPS_AUTO_OOB:
> +		ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
> +						  ops->ooboffs, len);
> +		BUG_ON(ret);
> +		return;
> +
> +	default:
> +		BUG();

Let's try to avoid these BUG_ON() calls. If you want transfer_oob() to
report an error, change the function prototype and check the ret code
in the caller.

> +	}
> +}
> +
> +/**
> + * spi_nand_read_pages - read data from flash to buffer
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob operations description structure
> + * @max_bitflips: maximum bitflip count
> + * Description:
> + *   Normal read function, read one page to buffer before issue
> + *   another.
> + */
> +static int spi_nand_read_pages(struct mtd_info *mtd, loff_t from,
> +			  struct mtd_oob_ops *ops, unsigned int *max_bitflips)
> +{
> +	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
> +	struct nand_device *nand = mtd_to_nand(mtd);
> +	int page_addr, page_offset, size, ret;
> +	unsigned int corrected = 0;
> +	int readlen = ops->len;
> +	int oobreadlen = ops->ooblen;
> +	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 == NULL;
> +
> +	page_addr = nand_offs_to_page(nand, from);
> +	page_offset = from & (nand_page_size(nand) - 1);
> +	ops->retlen = 0;
> +	*max_bitflips = 0;
> +

Comment for future reworks: we're doing the same thing in the // NAND
framework, maybe we should let the generic NAND layer do this
transformation for us (1 mtd request -> X page read/write requests).

> +	while (1) {
> +		ret = spi_nand_do_read_page(mtd, page_addr, ecc_off,
> +						&corrected, oob_only);
> +		if (ret)
> +			break;
> +		*max_bitflips = max(*max_bitflips, corrected);
> +		if (ops->datbuf) {
> +			size = min_t(int, readlen, nand_page_size(nand) - page_offset);
> +			memcpy(ops->datbuf + ops->retlen,
> +				chip->buf + page_offset, size);
> +			ops->retlen += size;
> +			readlen -= size;
> +			page_offset = 0;
> +		}
> +		if (ops->oobbuf) {
> +			size = min(oobreadlen, ooblen);
> +			spi_nand_transfer_oob(chip,
> +				ops->oobbuf + ops->oobretlen, ops, size);
> +			ops->oobretlen += size;
> +			oobreadlen -= size;
> +		}
> +		if (!readlen && !oobreadlen)
> +			break;
> +		page_addr++;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * spi_nand_do_read_ops - read data from flash to buffer
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob ops structure
> + * Description:
> + *   Disable internal ECC before reading when MTD_OPS_RAW set.
> + */
> +static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
> +			  struct mtd_oob_ops *ops)
> +{
> +	struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
> +	int ret;
> +	struct mtd_ecc_stats stats;
> +	unsigned int max_bitflips = 0;
> +	int oobreadlen = ops->ooblen;
> +	bool ecc_off = ops->mode == MTD_OPS_RAW;
> +	int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
> +		mtd->oobavail : mtd->oobsize;
> +
> +	if (oobreadlen > 0) {
> +		ooblen -= ops->ooboffs;
> +		ops->oobretlen = 0;
> +	}
> +	stats = mtd->ecc_stats;
> +	if (ecc_off)
> +		spi_nand_disable_ecc(chip);
> +	ret = spi_nand_read_pages(mtd, from, ops, &max_bitflips);
> +	if (ecc_off)
> +		spi_nand_enable_ecc(chip);
> +	if (ret)
> +		return ret;
> +
> +	if (mtd->ecc_stats.failed - stats.failed)
> +		return -EBADMSG;
> +
> +	return max_bitflips;
> +}
> +
> +/**
> + * spi_nand_read - [MTD Interface] SPI-NAND read
> + * @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 spi_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
> +	size_t *retlen, u8 *buf)
> +{
> +	struct mtd_oob_ops ops;
> +	int ret;
> +
> +	spi_nand_get_device(mtd, FL_READING);
> +
> +	memset(&ops, 0, sizeof(ops));
> +	ops.len = len;
> +	ops.datbuf = buf;
> +	ops.mode = MTD_OPS_PLACE_OOB;
> +	ret = spi_nand_do_read_ops(mtd, from, &ops);
> +
> +	*retlen = ops.retlen;
> +
> +	spi_nand_release_device(mtd);
> +
> +	return ret;
> +}
> +
> +/**
> + * spi_nand_read_oob - [MTD Interface] read data and/or out-of-band
> + * @mtd: MTD device structure
> + * @from: offset to read from
> + * @ops: oob operation description structure
> + */
> +static int spi_nand_read_oob(struct mtd_info *mtd, loff_t from,
> +			struct mtd_oob_ops *ops)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	ops->retlen = 0;
> +	spi_nand_get_device(mtd, FL_READING);
> +
> +	switch (ops->mode) {
> +	case MTD_OPS_PLACE_OOB:
> +	case MTD_OPS_AUTO_OOB:
> +	case MTD_OPS_RAW:
> +		break;
> +
> +	default:
> +		goto out;
> +	}
> +
> +	ret = spi_nand_do_read_ops(mtd, from, ops);
> +
> +out:
> +	spi_nand_release_device(mtd);
> +
> +	return ret;
> +}
>  MODULE_DESCRIPTION("SPI NAND framework");
>  MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
>  MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
> index 1fcbad7..b61045b 100644
> --- a/include/linux/mtd/spi-nand.h
> +++ b/include/linux/mtd/spi-nand.h
> @@ -132,6 +132,7 @@
>   * @read_cache_op:	[REPLACEABLE] Opcode of read from cache
>   * @write_cache_op:	[REPLACEABLE] Opcode of program load
>   * @command_fn:		[BOARDSPECIFIC] function to handle command transfer
> + * @get_ecc_status:	[REPLACEABLE] get ecc and bitflip status
>   * @buf:		[INTERN] buffer for read/write data
>   * @oobbuf:		[INTERN] buffer for read/write oob
>   * @controller_caps:	[INTERN] capacities of SPI NAND controller
> @@ -153,6 +154,8 @@ struct spi_nand_chip {
>  	u8		write_cache_op;
>  	int (*command_fn)(struct spi_nand_chip *this,
>  			struct spi_nand_cmd *cmd);
> +	void (*get_ecc_status)(struct spi_nand_chip *this, unsigned int status,
> +			unsigned int *corrected, unsigned int *ecc_errors);

As for the ->command_fn(), put that in a different structure (not sure
it is NAND controller related, so maybe a spinand_ecc_engine and
spinand_ecc_engine_ops structures).

>  	u8		*buf;
>  	u8		*oobbuf;
>  	u32		controller_caps;

Re: [PATCH 04/11] nand: spi: Add read function support

[Peter Pan]

Hi Boris,

On Tue, Feb 21, 2017 at 5:51 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 21 Feb 2017 16:00:03 +0800
> Peter Pan <peterpandong@micron.com> wrote:
>
>> Add read/readoob support for SPI NAND. Will be
>> registered under struct mtd_info later.
>>
>> Signed-off-by: Peter Pan <peterpandong@micron.com>
>> ---
>>  drivers/mtd/nand/spi/spi-nand-base.c | 321 +++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spi-nand.h         |   3 +
>>  2 files changed, 324 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/spi/spi-nand-base.c b/drivers/mtd/nand/spi/spi-nand-base.c
>> index b75e5cd..ee94eb8 100644
>> --- a/drivers/mtd/nand/spi/spi-nand-base.c
>> +++ b/drivers/mtd/nand/spi/spi-nand-base.c
>> @@ -18,9 +18,62 @@
>>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>> +#include <linux/sched.h>
>>  #include <linux/delay.h>
>>  #include <linux/mtd/spi-nand.h>
>>
>> +/**
>> + * spi_nand_get_device - [GENERIC] Get chip for selected access
>> + * @mtd: MTD device structure
>> + * @new_state: the state which is requested
>> + *
>> + * Get the device and lock it for exclusive access
>> + */
>> +static int spi_nand_get_device(struct mtd_info *mtd, int new_state)
>> +{
>> +     struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
>> +     DECLARE_WAITQUEUE(wait, current);
>> +
>> +     /*
>> +      * Grab the lock and see if the device is available
>> +      */
>> +     while (1) {
>> +             spin_lock(&this->chip_lock);
>> +             if (this->state == FL_READY) {
>> +                     this->state = new_state;
>> +                     spin_unlock(&this->chip_lock);
>> +                     break;
>> +             }
>> +             if (new_state == FL_PM_SUSPENDED) {
>> +                     spin_unlock(&this->chip_lock);
>> +                     return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
>> +             }
>> +             set_current_state(TASK_UNINTERRUPTIBLE);
>> +             add_wait_queue(&this->wq, &wait);
>> +             spin_unlock(&this->chip_lock);
>> +             schedule();
>> +             remove_wait_queue(&this->wq, &wait);
>> +     }
>
> Do we really need this fl_state dance? It could be replaced with a
> simple mutex that is taken for the whole flash operation. This leaves
> the suspended case which could be handled with:
>
> static void spinand_suspend(struct mtd_info *mtd)
> {
>         struct spi_nand_chip *this = mtd_to_spinand(mtd);
>
>         if (!mutex_trylock(&this->lock))
>                 return -EAGAIN;
>
>         return 0;
> }
>
> For other operations (read, write, lock, unlock, ...), you just have to
> take the lock before accessing the NAND and release it when you're done.
>
> Again, I'm saying that because I don't want to end up with something
> that is overly complex for no real reason, but maybe I'm missing
> something.

This code refers old NAND framework two years ago. I just check the latest
nand_base.c you already remove it. Let's make it simple. Fix this in v2.

>
>> +     return 0;
>> +}
>> +
>> +/**
>> + * spi_nand_release_device - [GENERIC] release chip
>> + * @mtd: MTD device structure
>> + *
>> + * Deselect, release chip lock and wake up anyone waiting on the device
>> + */
>> +static void spi_nand_release_device(struct mtd_info *mtd)
>> +{
>> +     struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
>> +
>> +     /* Release the chip */
>> +     spin_lock(&this->chip_lock);
>> +     this->state = FL_READY;
>> +     wake_up(&this->wq);
>> +     spin_unlock(&this->chip_lock);
>> +}
>> +
>>  static inline int spi_nand_issue_cmd(struct spi_nand_chip *chip,
>>                               struct spi_nand_cmd *cmd)
>>  {
>> @@ -313,6 +366,36 @@ static int spi_nand_erase_block(struct spi_nand_chip *chip,
>>  }
>>
>>  /**
>> + * spi_nand_wait - wait until the command is done
>> + * @chip: SPI-NAND device structure
>> + * @s: buffer to store status register(can be NULL)
>> + */
>> +static int spi_nand_wait(struct spi_nand_chip *chip, u8 *s)
>> +{
>> +     unsigned long timeo = jiffies;
>> +     u8 status;
>> +     int ret = -ETIMEDOUT;
>> +     int count = 0;
>> +
>> +     #define MIN_TRY_COUNT 3
>
> Please, no macro definitions inside functions.

Fix this in v2.

>
>> +     timeo += msecs_to_jiffies(20);
>> +
>> +     while (time_before(jiffies, timeo) || count < MIN_TRY_COUNT) {
>> +             spi_nand_read_status(chip, &status);
>> +             if ((status & STATUS_OIP_MASK) == STATUS_READY) {
>> +                     ret = 0;
>> +                     goto out;
>> +             }
>> +             count++;
>> +     }
>
>
> Hm, if your SPI controller is not able to read 3 times the status reg
> in 20ms it means it's really slow.

I met this case when using generic spi controller.

>
> I'd recommend doing something like:
>
>         do {
>                 spi_nand_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 out last check
>          */
>         spi_nand_read_status(chip, &status);

Recommendation accepted.

>
>> +out:
>> +     if (s)
>> +             *s = status;
>> +
>> +     return ret;
>
>         return (status & STATUS_OIP_MASK) == STATUS_READY ? 0 : -ETIMEDOUT;
>
>> +}
>> +
>> +/**
>>   * spi_nand_read_id - send 9Fh command to get ID
>>   * @chip: SPI-NAND device structure
>>   * @buf: buffer to store id
>> @@ -363,6 +446,244 @@ static int spi_nand_lock_block(struct spi_nand_chip *chip, u8 lock)
>>       return spi_nand_write_reg(chip, REG_BLOCK_LOCK, &lock);
>>  }
>>
>> +/**
>> + * spi_nand_do_read_page - read page from flash to buffer
>> + * @mtd: MTD device structure
>> + * @page_addr: page address/raw address
>> + * @column: column address
>> + * @ecc_off: without ecc or not
>> + * @corrected: how many bit error corrected
>> + * @buf: data buffer
>> + * @len: data length to read
>> + */
>> +static int spi_nand_do_read_page(struct mtd_info *mtd, u32 page_addr,
>> +                     bool ecc_off, int *corrected, bool oob_only)
>> +{
>> +     struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
>> +     struct nand_device *nand = mtd_to_nand(mtd);
>> +     int ret, ecc_error = 0;
>> +     u8 status;
>> +
>> +     spi_nand_read_page_to_cache(chip, page_addr);
>> +     ret = spi_nand_wait(chip, &status);
>> +     if (ret < 0) {
>> +             pr_err("error %d waiting page 0x%x to cache\n",
>> +                     ret, page_addr);
>> +             return ret;
>> +     }
>> +     if (!oob_only)
>> +             spi_nand_read_from_cache(chip, page_addr, 0,
>> +             nand_page_size(nand) + nand_per_page_oobsize(nand), chip->buf);
>> +     else
>> +             spi_nand_read_from_cache(chip, page_addr, nand_page_size(nand),
>> +                             nand_per_page_oobsize(nand), chip->oobbuf);
>> +     if (!ecc_off) {
>> +             chip->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) {
>> +                     pr_err("internal ECC error reading page 0x%x\n",
>> +                             page_addr);
>> +                     mtd->ecc_stats.failed++;
>> +             } else if (*corrected) {
>> +                     mtd->ecc_stats.corrected += *corrected;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * spi_nand_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 void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
>> +                               struct mtd_oob_ops *ops, size_t len)
>> +{
>> +     struct mtd_info *mtd = spi_nand_to_mtd(chip);
>> +     int ret;
>> +
>> +     switch (ops->mode) {
>> +
>> +     case MTD_OPS_PLACE_OOB:
>> +     case MTD_OPS_RAW:
>> +             memcpy(oob, chip->oobbuf + ops->ooboffs, len);
>> +             return;
>> +
>> +     case MTD_OPS_AUTO_OOB:
>> +             ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
>> +                                               ops->ooboffs, len);
>> +             BUG_ON(ret);
>> +             return;
>> +
>> +     default:
>> +             BUG();
>
> Let's try to avoid these BUG_ON() calls. If you want transfer_oob() to
> report an error, change the function prototype and check the ret code
> in the caller.

I will remove the BUG_ON()s in v2. Boris, I think this part can be put in
new NAND core, what do you think?

>
>> +     }
>> +}
>> +
>> +/**
>> + * spi_nand_read_pages - read data from flash to buffer
>> + * @mtd: MTD device structure
>> + * @from: offset to read from
>> + * @ops: oob operations description structure
>> + * @max_bitflips: maximum bitflip count
>> + * Description:
>> + *   Normal read function, read one page to buffer before issue
>> + *   another.
>> + */
>> +static int spi_nand_read_pages(struct mtd_info *mtd, loff_t from,
>> +                       struct mtd_oob_ops *ops, unsigned int *max_bitflips)
>> +{
>> +     struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
>> +     struct nand_device *nand = mtd_to_nand(mtd);
>> +     int page_addr, page_offset, size, ret;
>> +     unsigned int corrected = 0;
>> +     int readlen = ops->len;
>> +     int oobreadlen = ops->ooblen;
>> +     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 == NULL;
>> +
>> +     page_addr = nand_offs_to_page(nand, from);
>> +     page_offset = from & (nand_page_size(nand) - 1);
>> +     ops->retlen = 0;
>> +     *max_bitflips = 0;
>> +
>
> Comment for future reworks: we're doing the same thing in the // NAND
> framework, maybe we should let the generic NAND layer do this
> transformation for us (1 mtd request -> X page read/write requests).

Totally agree with you.

>
>> +     while (1) {
>> +             ret = spi_nand_do_read_page(mtd, page_addr, ecc_off,
>> +                                             &corrected, oob_only);
>> +             if (ret)
>> +                     break;
>> +             *max_bitflips = max(*max_bitflips, corrected);
>> +             if (ops->datbuf) {
>> +                     size = min_t(int, readlen, nand_page_size(nand) - page_offset);
>> +                     memcpy(ops->datbuf + ops->retlen,
>> +                             chip->buf + page_offset, size);
>> +                     ops->retlen += size;
>> +                     readlen -= size;
>> +                     page_offset = 0;
>> +             }
>> +             if (ops->oobbuf) {
>> +                     size = min(oobreadlen, ooblen);
>> +                     spi_nand_transfer_oob(chip,
>> +                             ops->oobbuf + ops->oobretlen, ops, size);
>> +                     ops->oobretlen += size;
>> +                     oobreadlen -= size;
>> +             }
>> +             if (!readlen && !oobreadlen)
>> +                     break;
>> +             page_addr++;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * spi_nand_do_read_ops - read data from flash to buffer
>> + * @mtd: MTD device structure
>> + * @from: offset to read from
>> + * @ops: oob ops structure
>> + * Description:
>> + *   Disable internal ECC before reading when MTD_OPS_RAW set.
>> + */
>> +static int spi_nand_do_read_ops(struct mtd_info *mtd, loff_t from,
>> +                       struct mtd_oob_ops *ops)
>> +{
>> +     struct spi_nand_chip *chip = mtd_to_spi_nand(mtd);
>> +     int ret;
>> +     struct mtd_ecc_stats stats;
>> +     unsigned int max_bitflips = 0;
>> +     int oobreadlen = ops->ooblen;
>> +     bool ecc_off = ops->mode == MTD_OPS_RAW;
>> +     int ooblen = ops->mode == MTD_OPS_AUTO_OOB ?
>> +             mtd->oobavail : mtd->oobsize;
>> +
>> +     if (oobreadlen > 0) {
>> +             ooblen -= ops->ooboffs;
>> +             ops->oobretlen = 0;
>> +     }
>> +     stats = mtd->ecc_stats;
>> +     if (ecc_off)
>> +             spi_nand_disable_ecc(chip);
>> +     ret = spi_nand_read_pages(mtd, from, ops, &max_bitflips);
>> +     if (ecc_off)
>> +             spi_nand_enable_ecc(chip);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (mtd->ecc_stats.failed - stats.failed)
>> +             return -EBADMSG;
>> +
>> +     return max_bitflips;
>> +}
>> +
>> +/**
>> + * spi_nand_read - [MTD Interface] SPI-NAND read
>> + * @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 spi_nand_read(struct mtd_info *mtd, loff_t from, size_t len,
>> +     size_t *retlen, u8 *buf)
>> +{
>> +     struct mtd_oob_ops ops;
>> +     int ret;
>> +
>> +     spi_nand_get_device(mtd, FL_READING);
>> +
>> +     memset(&ops, 0, sizeof(ops));
>> +     ops.len = len;
>> +     ops.datbuf = buf;
>> +     ops.mode = MTD_OPS_PLACE_OOB;
>> +     ret = spi_nand_do_read_ops(mtd, from, &ops);
>> +
>> +     *retlen = ops.retlen;
>> +
>> +     spi_nand_release_device(mtd);
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * spi_nand_read_oob - [MTD Interface] read data and/or out-of-band
>> + * @mtd: MTD device structure
>> + * @from: offset to read from
>> + * @ops: oob operation description structure
>> + */
>> +static int spi_nand_read_oob(struct mtd_info *mtd, loff_t from,
>> +                     struct mtd_oob_ops *ops)
>> +{
>> +     int ret = -ENOTSUPP;
>> +
>> +     ops->retlen = 0;
>> +     spi_nand_get_device(mtd, FL_READING);
>> +
>> +     switch (ops->mode) {
>> +     case MTD_OPS_PLACE_OOB:
>> +     case MTD_OPS_AUTO_OOB:
>> +     case MTD_OPS_RAW:
>> +             break;
>> +
>> +     default:
>> +             goto out;
>> +     }
>> +
>> +     ret = spi_nand_do_read_ops(mtd, from, ops);
>> +
>> +out:
>> +     spi_nand_release_device(mtd);
>> +
>> +     return ret;
>> +}
>>  MODULE_DESCRIPTION("SPI NAND framework");
>>  MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
>>  MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/mtd/spi-nand.h b/include/linux/mtd/spi-nand.h
>> index 1fcbad7..b61045b 100644
>> --- a/include/linux/mtd/spi-nand.h
>> +++ b/include/linux/mtd/spi-nand.h
>> @@ -132,6 +132,7 @@
>>   * @read_cache_op:   [REPLACEABLE] Opcode of read from cache
>>   * @write_cache_op:  [REPLACEABLE] Opcode of program load
>>   * @command_fn:              [BOARDSPECIFIC] function to handle command transfer
>> + * @get_ecc_status:  [REPLACEABLE] get ecc and bitflip status
>>   * @buf:             [INTERN] buffer for read/write data
>>   * @oobbuf:          [INTERN] buffer for read/write oob
>>   * @controller_caps: [INTERN] capacities of SPI NAND controller
>> @@ -153,6 +154,8 @@ struct spi_nand_chip {
>>       u8              write_cache_op;
>>       int (*command_fn)(struct spi_nand_chip *this,
>>                       struct spi_nand_cmd *cmd);
>> +     void (*get_ecc_status)(struct spi_nand_chip *this, unsigned int status,
>> +                     unsigned int *corrected, unsigned int *ecc_errors);
>
> As for the ->command_fn(), put that in a different structure (not sure
> it is NAND controller related, so maybe a spinand_ecc_engine and
> spinand_ecc_engine_ops structures).

spinand_ecc_engine is better I think. Fix this in v2,

Thanks
Peter Pan

Re: [PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Arnaud Mouiche]

On 21/02/2017 10:40, Peter Pan wrote:
> Hi Arnaud,
>
> On Tue, Feb 21, 2017 at 5:01 PM, Arnaud Mouiche
> <arnaud.mouiche@gmail.com> wrote:
>> Hello Peter.
>> First thank you for this effort to finally bring the spinand framework back
>> on stage.
>>
>> On 21/02/2017 09:00, Peter Pan wrote:
>>> This commit abstracts basic SPI NAND commands to
>>> functions in spi-nand-base.c. Because command sets
>>> have difference by vendors, we create spi-nand-cmd.c
>>> to define this difference by command config table.
>>>
>>> [...]
>>> +
>>> +/**
>>> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
>>> + * to cache
>>> + * @chip: SPI-NAND device structure
>>> + * @page_addr: page to read
>>> + */
>>> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
>>> +                                       u32 page_addr)
>>> +{
>>> +       struct spi_nand_cmd cmd;
>>> +
>>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>>> +       cmd.cmd = SPINAND_CMD_PAGE_READ;
>>> +       cmd.n_addr = 3;
>>> +       cmd.addr[0] = (u8)(page_addr >> 16);
>>> +       cmd.addr[1] = (u8)(page_addr >> 8);
>>> +       cmd.addr[2] = (u8)page_addr;
>>> +
>>> +       return spi_nand_issue_cmd(chip, &cmd);
>>> +}
>>
>> Just a question. Don't you try to map too exactly the spi_nand_cmd to the
>> internal micron SPI command ?
>> Why "cmd.addr" should be a byte array, and not a u32, leaving each chip to
>> details to handle the u32 to SPI byte stream mapping.
>>
>>
>>
>>> +
>>> +/**
>>> + * spi_nand_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
>>> + * Description:
>>> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
>>> + *   The read can specify 1 to (page size + spare size) bytes of data
>>> read at
>>> + *   the corresponding locations.
>>> + *   No tRd delay.
>>> + */
>>> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
>>> +               u32 page_addr, u32 column, size_t len, u8 *rbuf)
>>> +{
>>> +       struct nand_device *nand = &chip->base;
>>> +       struct spi_nand_cmd cmd;
>>> +
>>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>>> +       cmd.cmd = chip->read_cache_op;
>>> +       cmd.n_addr = 2;
>>> +       cmd.addr[0] = (u8)(column >> 8);
>>> +       if (chip->options & SPINAND_NEED_PLANE_SELECT)
>>> +               cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand,
>>> page_addr)
>>> +                               & 0x1) << 4);
>>> +       cmd.addr[1] = (u8)column;
>>> +       cmd.n_rx = len;
>>> +       cmd.rx_buf = rbuf;
>>> +
>>> +       return spi_nand_issue_cmd(chip, &cmd);
>>> +}
>>> +
>> Some reasons to handle the plane concept (Micron specific), and even move
>> the fact that it touch the bit 4 of cdm.addr[0], in common code ?
> Let me try to remove this from framework when separate controller and device
> related code. Any suggestion on this?
I know, Micron was the first spinand chipset available, will be the 
first chipset supported by this framework, but it is already not so 
generic regarding to the page addressing compared with winbond, esmt, 
gigadevice or micronix...

But may be this the sign that "generic_spi_nand_cmd_fn" if "too" 
straight in your implementation and just try to map the "struct 
spi_nand_cmd" into the SPI frame without any more place for chip 
specialization.

I guess I would prefer to see multiple 'command primitive' inside each 
chip structures (eg. chip->read_from_cache() and so on).
But in the meantime, spinand chipset are made to be generic and behave 
mostly the same way, which would make this primitive declaration a 
little bit redundant.

The other way is to already have a "Micron_spi_nand_cmd_fn" that will 
patch the address fields for "read cache op", and then call the 
"generic_spi_nand_cmd_fn" to finalize the job.

Arnaud

>
> Thanks,
> Peter Pan

Re: [PATCH 04/11] nand: spi: Add read function support

[Boris Brezillon]

On Tue, 21 Feb 2017 18:06:03 +0800
Peter Pan <peterpansjtu@gmail.com> wrote:

[...]

> >> +static int spi_nand_get_device(struct mtd_info *mtd, int new_state)
> >> +{
> >> +     struct spi_nand_chip *this = mtd_to_spi_nand(mtd);
> >> +     DECLARE_WAITQUEUE(wait, current);
> >> +
> >> +     /*
> >> +      * Grab the lock and see if the device is available
> >> +      */
> >> +     while (1) {
> >> +             spin_lock(&this->chip_lock);
> >> +             if (this->state == FL_READY) {
> >> +                     this->state = new_state;
> >> +                     spin_unlock(&this->chip_lock);
> >> +                     break;
> >> +             }
> >> +             if (new_state == FL_PM_SUSPENDED) {
> >> +                     spin_unlock(&this->chip_lock);
> >> +                     return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
> >> +             }
> >> +             set_current_state(TASK_UNINTERRUPTIBLE);
> >> +             add_wait_queue(&this->wq, &wait);
> >> +             spin_unlock(&this->chip_lock);
> >> +             schedule();
> >> +             remove_wait_queue(&this->wq, &wait);
> >> +     }  
> >
> > Do we really need this fl_state dance? It could be replaced with a
> > simple mutex that is taken for the whole flash operation. This leaves
> > the suspended case which could be handled with:
> >
> > static void spinand_suspend(struct mtd_info *mtd)
> > {
> >         struct spi_nand_chip *this = mtd_to_spinand(mtd);
> >
> >         if (!mutex_trylock(&this->lock))
> >                 return -EAGAIN;
> >
> >         return 0;
> > }
> >
> > For other operations (read, write, lock, unlock, ...), you just have to
> > take the lock before accessing the NAND and release it when you're done.
> >
> > Again, I'm saying that because I don't want to end up with something
> > that is overly complex for no real reason, but maybe I'm missing
> > something.  
> 
> This code refers old NAND framework two years ago. I just check the latest
> nand_base.c you already remove it. Let's make it simple. Fix this in v2.

Nope, it's still here [1], but I'm pretty sure this complexity is
actually not needed, and anyway, I plan to delegate this serialization
to the NAND controller in the future (some operations can be done
in //, for example, on multi-die NANDs, you can read something on a die,
while you're programming something on the other die).

[1]http://lxr.free-electrons.com/source/drivers/mtd/nand/nand_base.c#L851

Re: [PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Boris Brezillon]

On Tue, 21 Feb 2017 11:22:52 +0100
Arnaud Mouiche <arnaud.mouiche@gmail.com> wrote:

> On 21/02/2017 10:40, Peter Pan wrote:
> > Hi Arnaud,
> >
> > On Tue, Feb 21, 2017 at 5:01 PM, Arnaud Mouiche
> > <arnaud.mouiche@gmail.com> wrote:  
> >> Hello Peter.
> >> First thank you for this effort to finally bring the spinand framework back
> >> on stage.
> >>
> >> On 21/02/2017 09:00, Peter Pan wrote:  
> >>> This commit abstracts basic SPI NAND commands to
> >>> functions in spi-nand-base.c. Because command sets
> >>> have difference by vendors, we create spi-nand-cmd.c
> >>> to define this difference by command config table.
> >>>
> >>> [...]
> >>> +
> >>> +/**
> >>> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
> >>> + * to cache
> >>> + * @chip: SPI-NAND device structure
> >>> + * @page_addr: page to read
> >>> + */
> >>> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
> >>> +                                       u32 page_addr)
> >>> +{
> >>> +       struct spi_nand_cmd cmd;
> >>> +
> >>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> >>> +       cmd.cmd = SPINAND_CMD_PAGE_READ;
> >>> +       cmd.n_addr = 3;
> >>> +       cmd.addr[0] = (u8)(page_addr >> 16);
> >>> +       cmd.addr[1] = (u8)(page_addr >> 8);
> >>> +       cmd.addr[2] = (u8)page_addr;
> >>> +
> >>> +       return spi_nand_issue_cmd(chip, &cmd);
> >>> +}  
> >>
> >> Just a question. Don't you try to map too exactly the spi_nand_cmd to the
> >> internal micron SPI command ?
> >> Why "cmd.addr" should be a byte array, and not a u32, leaving each chip to
> >> details to handle the u32 to SPI byte stream mapping.
> >>
> >>
> >>  
> >>> +
> >>> +/**
> >>> + * spi_nand_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
> >>> + * Description:
> >>> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
> >>> + *   The read can specify 1 to (page size + spare size) bytes of data
> >>> read at
> >>> + *   the corresponding locations.
> >>> + *   No tRd delay.
> >>> + */
> >>> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
> >>> +               u32 page_addr, u32 column, size_t len, u8 *rbuf)
> >>> +{
> >>> +       struct nand_device *nand = &chip->base;
> >>> +       struct spi_nand_cmd cmd;
> >>> +
> >>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
> >>> +       cmd.cmd = chip->read_cache_op;
> >>> +       cmd.n_addr = 2;
> >>> +       cmd.addr[0] = (u8)(column >> 8);
> >>> +       if (chip->options & SPINAND_NEED_PLANE_SELECT)
> >>> +               cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand,
> >>> page_addr)
> >>> +                               & 0x1) << 4);
> >>> +       cmd.addr[1] = (u8)column;
> >>> +       cmd.n_rx = len;
> >>> +       cmd.rx_buf = rbuf;
> >>> +
> >>> +       return spi_nand_issue_cmd(chip, &cmd);
> >>> +}
> >>> +  
> >> Some reasons to handle the plane concept (Micron specific), and even move
> >> the fact that it touch the bit 4 of cdm.addr[0], in common code ?  
> > Let me try to remove this from framework when separate controller and device
> > related code. Any suggestion on this?  
> I know, Micron was the first spinand chipset available, will be the 
> first chipset supported by this framework, but it is already not so 
> generic regarding to the page addressing compared with winbond, esmt, 
> gigadevice or micronix...
> 
> But may be this the sign that "generic_spi_nand_cmd_fn" if "too" 
> straight in your implementation and just try to map the "struct 
> spi_nand_cmd" into the SPI frame without any more place for chip 
> specialization.
> 
> I guess I would prefer to see multiple 'command primitive' inside each 
> chip structures (eg. chip->read_from_cache() and so on).

I'm not against the idea, but I'd prefer to see these operations in a
separate struct:

struct spinand_manufacturer_ops {
	int (*read_from_cache)();
	/* ... */
};

struct spinand_device {
	/* ... */

	struct {
		struct spinand_manufacturer_ops *ops;
		void *priv;
		/* ... */
	} manufacturer;
};

> But in the meantime, spinand chipset are made to be generic and behave 
> mostly the same way, which would make this primitive declaration a 
> little bit redundant.

Well, for the generic case, you could either provide generic helpers
that could be used by NAND manufacturer drivers in their
spinand_manufacturer_ops definition, or you could let the core do

	if (!nand->manufacturer->ops || !nand->manufacturer->ops->func)
		default_func();
		

Re: [PATCH 04/11] nand: spi: Add read function support

[Boris Brezillon]

On Tue, 21 Feb 2017 18:06:03 +0800
Peter Pan <peterpansjtu@gmail.com> wrote:



> >> +/**
> >> + * spi_nand_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 void spi_nand_transfer_oob(struct spi_nand_chip *chip, u8 *oob,
> >> +                               struct mtd_oob_ops *ops, size_t len)
> >> +{
> >> +     struct mtd_info *mtd = spi_nand_to_mtd(chip);
> >> +     int ret;
> >> +
> >> +     switch (ops->mode) {
> >> +
> >> +     case MTD_OPS_PLACE_OOB:
> >> +     case MTD_OPS_RAW:
> >> +             memcpy(oob, chip->oobbuf + ops->ooboffs, len);
> >> +             return;
> >> +
> >> +     case MTD_OPS_AUTO_OOB:
> >> +             ret = mtd_ooblayout_get_databytes(mtd, oob, chip->oobbuf,
> >> +                                               ops->ooboffs, len);
> >> +             BUG_ON(ret);
> >> +             return;
> >> +
> >> +     default:
> >> +             BUG();  
> >
> > Let's try to avoid these BUG_ON() calls. If you want transfer_oob() to
> > report an error, change the function prototype and check the ret code
> > in the caller.  
> 
> I will remove the BUG_ON()s in v2. Boris, I think this part can be put in
> new NAND core, what do you think?

Well, assuming we move the ->oob buffer in nand_device struct, then
yes. That's exactly for this reason I wanted to see a version of the
SPI NAND framework before merging the "interface-agnostic NAND layer".
I guess we'll find other candidates for factorization, but I also think
we should not try to get the perfect implementation before merging
something (IOW, let's keep this improvement on our TODO list).

> 
> >  
> >> +     }
> >> +}