[PATCH v6 0/4] Tango NAND Flash controller driver (and dependencies)

[PATCH v6 0/4] Tango NAND Flash controller driver (and dependencies)

[Marc Gonzalez]

This patch series imports a driver for the Tango NAND Flash controller
and a few dependent patches, included for easy reference.

Boris Brezillon (2):
  mtd: nand: Add a few more timings to nand_sdr_timings
  mtd: nand: Wait tCCS after a column change

Marc Gonzalez (2):
  mtd: nand: Actually apply the optimal timings
  mtd: nand: tango: import driver for tango chips

 drivers/mtd/nand/Kconfig        |   6 +
 drivers/mtd/nand/Makefile       |   1 +
 drivers/mtd/nand/nand_base.c    |  28 +-
 drivers/mtd/nand/nand_timings.c |  26 +-
 drivers/mtd/nand/tango_nand.c   | 654 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h        |  18 ++
 6 files changed, 730 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mtd/nand/tango_nand.c

[PATCH v6 1/4] mtd: nand: Add a few more timings to nand_sdr_timings

[Marc Gonzalez]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

Add the tR_max, tBERS_max, tPROG_max and tCCS_min timings to the
nand_sdr_timings struct.
Assign default/safe values for the statically defined timings, and
extract them from the ONFI parameter table if the NAND is ONFI
compliant.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_timings.c | 26 +++++++++++++++++++++++++-
 include/linux/mtd/nand.h        |  8 ++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_timings.c b/drivers/mtd/nand/nand_timings.c
index 13a587407be3..f06312df3669 100644
--- a/drivers/mtd/nand/nand_timings.c
+++ b/drivers/mtd/nand/nand_timings.c
@@ -18,6 +18,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	{
 		.type = NAND_SDR_IFACE,
 		.timings.sdr = {
+			.tCCS_min = 500000,
+			.tR_max = 200000000,
 			.tADL_min = 400000,
 			.tALH_min = 20000,
 			.tALS_min = 50000,
@@ -58,6 +60,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	{
 		.type = NAND_SDR_IFACE,
 		.timings.sdr = {
+			.tCCS_min = 500000,
+			.tR_max = 200000000,
 			.tADL_min = 400000,
 			.tALH_min = 10000,
 			.tALS_min = 25000,
@@ -98,6 +102,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	{
 		.type = NAND_SDR_IFACE,
 		.timings.sdr = {
+			.tCCS_min = 500000,
+			.tR_max = 200000000,
 			.tADL_min = 400000,
 			.tALH_min = 10000,
 			.tALS_min = 15000,
@@ -138,6 +144,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	{
 		.type = NAND_SDR_IFACE,
 		.timings.sdr = {
+			.tCCS_min = 500000,
+			.tR_max = 200000000,
 			.tADL_min = 400000,
 			.tALH_min = 5000,
 			.tALS_min = 10000,
@@ -178,6 +186,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	{
 		.type = NAND_SDR_IFACE,
 		.timings.sdr = {
+			.tCCS_min = 500000,
+			.tR_max = 200000000,
 			.tADL_min = 400000,
 			.tALH_min = 5000,
 			.tALS_min = 10000,
@@ -218,6 +228,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
 	{
 		.type = NAND_SDR_IFACE,
 		.timings.sdr = {
+			.tCCS_min = 500000,
+			.tR_max = 200000000,
 			.tADL_min = 400000,
 			.tALH_min = 5000,
 			.tALS_min = 10000,
@@ -290,10 +302,22 @@ int onfi_init_data_interface(struct nand_chip *chip,
 	*iface = onfi_sdr_timings[timing_mode];
 
 	/*
-	 * TODO: initialize timings that cannot be deduced from timing mode:
+	 * Initialize timings that cannot be deduced from timing mode:
 	 * tR, tPROG, tCCS, ...
 	 * These information are part of the ONFI parameter page.
 	 */
+	if (chip->onfi_version) {
+		struct nand_onfi_params *params = &chip->onfi_params;
+		struct nand_sdr_timings *timings = &iface->timings.sdr;
+
+		/* microseconds -> picoseconds */
+		timings->tPROG_max = 1000000UL * le16_to_cpu(params->t_prog);
+		timings->tBERS_max = 1000000UL * le16_to_cpu(params->t_bers);
+		timings->tR_max = 1000000UL * le16_to_cpu(params->t_r);
+
+		/* nanoseconds -> picoseconds */
+		timings->tCCS_min = 1000UL * le16_to_cpu(params->t_ccs);
+	}
 
 	return 0;
 }
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index c5d3d5024fc8..6fe83bce83a6 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -584,6 +584,10 @@ struct nand_buffers {
  *
  * All these timings are expressed in picoseconds.
  *
+ * @tBERS_max: Block erase time
+ * @tCCS_min: Change column setup time
+ * @tPROG_max: Page program time
+ * @tR_max: Page read time
  * @tALH_min: ALE hold time
  * @tADL_min: ALE to data loading time
  * @tALS_min: ALE setup time
@@ -621,6 +625,10 @@ struct nand_buffers {
  * @tWW_min: WP# transition to WE# low
  */
 struct nand_sdr_timings {
+	u32 tBERS_max;
+	u32 tCCS_min;
+	u32 tPROG_max;
+	u32 tR_max;
 	u32 tALH_min;
 	u32 tADL_min;
 	u32 tALS_min;
-- 
2.9.0

[PATCH v6 2/4] mtd: nand: Wait tCCS after a column change

[Marc Gonzalez]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

Drivers implementing ->cmd_ctrl() and relying on the default ->cmdfunc()
implementation usually don't wait tCCS when a column change (RNDIN or
RNDOUT) is requested.
Add an option flag to ask the core to do so (note that we keep this as
an opt-in to avoid breaking existing implementations), and make use of
the ->data_interface information is available (otherwise, wait 500ns).

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

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e5718e5ecf92..0acb0070280a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -709,6 +709,25 @@ static void nand_command(struct mtd_info *mtd, unsigned int command,
 	nand_wait_ready(mtd);
 }
 
+static void nand_ccs_delay(struct nand_chip *chip)
+{
+	/*
+	 * The controller already takes care of waiting for tCCS when the RNDIN
+	 * or RNDOUT command is sent, return directly.
+	 */
+	if (!(chip->options & NAND_WAIT_TCCS))
+		return;
+
+	/*
+	 * Wait tCCS_min if it is correctly defined, otherwise wait 500ns
+	 * (which should be safe for all NANDs).
+	 */
+	if (chip->data_interface && chip->data_interface->timings.sdr.tCCS_min)
+		ndelay(chip->data_interface->timings.sdr.tCCS_min / 1000);
+	else
+		ndelay(500);
+}
+
 /**
  * nand_command_lp - [DEFAULT] Send command to NAND large page device
  * @mtd: MTD device structure
@@ -773,10 +792,13 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
 	case NAND_CMD_ERASE1:
 	case NAND_CMD_ERASE2:
 	case NAND_CMD_SEQIN:
-	case NAND_CMD_RNDIN:
 	case NAND_CMD_STATUS:
 		return;
 
+	case NAND_CMD_RNDIN:
+		nand_ccs_delay(chip);
+		return;
+
 	case NAND_CMD_RESET:
 		if (chip->dev_ready)
 			break;
@@ -795,6 +817,8 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
 			       NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
 		chip->cmd_ctrl(mtd, NAND_CMD_NONE,
 			       NAND_NCE | NAND_CTRL_CHANGE);
+
+		nand_ccs_delay(chip);
 		return;
 
 	case NAND_CMD_READ0:
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 6fe83bce83a6..970ceb948835 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -210,6 +210,16 @@ enum nand_ecc_algo {
  */
 #define NAND_USE_BOUNCE_BUFFER	0x00100000
 
+/*
+ * In case your controller is implementing ->cmd_ctrl() and is relying on the
+ * default ->cmdfunc() implementation, you may want to let the core handle the
+ * tCCS delay which is required when a column change (RNDIN or RNDOUT) is
+ * requested.
+ * If your controller already takes care of this delay, you don't need to set
+ * this flag.
+ */
+#define NAND_WAIT_TCCS		0x00200000
+
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
 #define NAND_CONTROLLER_ALLOC	0x80000000
-- 
2.9.0

[PATCH v6 3/4] mtd: nand: Actually apply the optimal timings

[Marc Gonzalez]

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/mtd/nand/nand_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0acb0070280a..b311ebc4bebc 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1108,7 +1108,7 @@ static int nand_init_data_interface(struct nand_chip *chip)
 		}
 	}
 
-	return 0;
+	return nand_setup_data_interface(chip);
 }
 
 static void nand_release_data_interface(struct nand_chip *chip)
-- 
2.9.0

[PATCH v6 4/4] mtd: nand: tango: import driver for tango chips

[Marc Gonzalez]

This driver supports the NAND Flash controller embedded in recent
Tango chips, such as SMP8758 and SMP8759.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/mtd/nand/Kconfig      |   6 +
 drivers/mtd/nand/Makefile     |   1 +
 drivers/mtd/nand/tango_nand.c | 654 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 661 insertions(+)
 create mode 100644 drivers/mtd/nand/tango_nand.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7b7a887b4709..844ab20a23a4 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -205,6 +205,12 @@ config MTD_NAND_S3C2410_CLKSTOP
 	  when the is NAND chip selected or released, but will save
 	  approximately 5mA of power when there is nothing happening.
 
+config MTD_NAND_TANGO
+	tristate "NAND Flash support for Tango chips"
+	depends on ARCH_TANGO
+	help
+	  Enables the NAND Flash controller on Tango chips.
+
 config MTD_NAND_DISKONCHIP
 	tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND reimplementation)"
 	depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index cafde6f3d957..4904ad3614fb 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_MTD_NAND_DENALI_DT)	+= denali_dt.o
 obj-$(CONFIG_MTD_NAND_AU1550)		+= au1550nd.o
 obj-$(CONFIG_MTD_NAND_BF5XX)		+= bf5xx_nand.o
 obj-$(CONFIG_MTD_NAND_S3C2410)		+= s3c2410.o
+obj-$(CONFIG_MTD_NAND_TANGO)		+= tango_nand.o
 obj-$(CONFIG_MTD_NAND_DAVINCI)		+= davinci_nand.o
 obj-$(CONFIG_MTD_NAND_DISKONCHIP)	+= diskonchip.o
 obj-$(CONFIG_MTD_NAND_DOCG4)		+= docg4.o
diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
new file mode 100644
index 000000000000..375ca8e4a3aa
--- /dev/null
+++ b/drivers/mtd/nand/tango_nand.c
@@ -0,0 +1,654 @@
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+
+/* Offsets relative to chip->base */
+#define PBUS_CMD	0
+#define PBUS_ADDR	4
+#define PBUS_DATA	8
+
+/* Offsets relative to reg_base */
+#define NFC_STATUS	0x00
+#define NFC_FLASH_CMD	0x04
+#define NFC_DEVICE_CFG	0x08
+#define NFC_TIMING1	0x0c
+#define NFC_TIMING2	0x10
+#define NFC_XFER_CFG	0x14
+#define NFC_PKT_0_CFG	0x18
+#define NFC_PKT_N_CFG	0x1c
+#define NFC_BB_CFG	0x20
+#define NFC_ADDR_PAGE	0x24
+#define NFC_ADDR_OFFSET	0x28
+#define NFC_XFER_STATUS	0x2c
+
+/* NFC_STATUS values */
+#define CMD_READY	BIT(31)
+
+/* NFC_FLASH_CMD values */
+#define NFC_READ	1
+#define NFC_WRITE	2
+
+/* NFC_XFER_STATUS values */
+#define PAGE_IS_EMPTY	BIT(16)
+
+/* Offsets relative to mem_base */
+#define METADATA	0x000
+#define ERROR_REPORT	0x1c0
+
+/*
+ * Error reports are split in two bytes:
+ * byte 0 for the first packet in the page (PKT_0)
+ * byte 1 for other packets in the page (PKT_N, for N > 0)
+ * ERR_COUNT_PKT_N is the max error count over all but the first packet.
+ */
+#define DECODE_OK_PKT_0(v)	(v & BIT(7))
+#define DECODE_OK_PKT_N(v)	(v & BIT(15))
+#define ERR_COUNT_PKT_0(v)	((v >> 0) & 0x3f)
+#define ERR_COUNT_PKT_N(v)	((v >> 8) & 0x3f)
+
+/* Offsets relative to pbus_base */
+#define PBUS_CS_CTRL	0x83c
+#define PBUS_PAD_MODE	0x8f0
+
+/* PBUS_CS_CTRL values */
+#define PBUS_IORDY	BIT(31)
+
+/*
+ * PBUS_PAD_MODE values
+ * In raw mode, the driver communicates directly with the NAND chips.
+ * In NFC mode, the NAND Flash controller manages the communication.
+ * We use NFC mode for read and write; raw mode for everything else.
+ */
+#define MODE_RAW	0
+#define MODE_NFC	BIT(31)
+
+#define METADATA_SIZE	4
+#define BBM_SIZE	6
+#define FIELD_ORDER	15
+
+#define MAX_CS		4
+
+struct tango_nfc {
+	struct nand_hw_control hw;
+	void __iomem *reg_base;
+	void __iomem *mem_base;
+	void __iomem *pbus_base;
+	struct tango_chip *chips[MAX_CS];
+	struct dma_chan *chan;
+	int freq_kHz;
+};
+
+#define to_tango_nfc(ptr) container_of(ptr, struct tango_nfc, hw)
+
+struct tango_chip {
+	struct nand_chip nand_chip;
+	void __iomem *base;
+	u32 timing1;
+	u32 timing2;
+	u32 xfer_cfg;
+	u32 pkt_0_cfg;
+	u32 pkt_n_cfg;
+	u32 bb_cfg;
+};
+
+#define to_tango_chip(ptr) container_of(ptr, struct tango_chip, nand_chip)
+
+#define XFER_CFG(cs, page_count, steps, metadata_size)	\
+	((cs) << 24 | (page_count) << 16 | (steps) << 8 | (metadata_size) << 0)
+
+#define PKT_CFG(size, strength) ((size) << 16 | (strength) << 0)
+
+#define BB_CFG(bb_offset, bb_size) ((bb_offset) << 16 | (bb_size) << 0)
+
+#define TIMING(t0, t1, t2, t3) (t0 << 24 | t1 << 16 | t2 << 8 | t3 << 0)
+
+static void tango_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
+{
+	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
+
+	if (ctrl & NAND_CLE)
+		writeb_relaxed(dat, tchip->base + PBUS_CMD);
+
+	if (ctrl & NAND_ALE)
+		writeb_relaxed(dat, tchip->base + PBUS_ADDR);
+}
+
+static int tango_dev_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
+
+	return readl_relaxed(nfc->pbus_base + PBUS_CS_CTRL) & PBUS_IORDY;
+}
+
+static uint8_t tango_read_byte(struct mtd_info *mtd)
+{
+	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
+
+	return readb_relaxed(tchip->base + PBUS_DATA);
+}
+
+static void tango_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
+
+	ioread8_rep(tchip->base + PBUS_DATA, buf, len);
+}
+
+static void tango_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
+
+	iowrite8_rep(tchip->base + PBUS_DATA, buf, len);
+}
+
+static void tango_select_chip(struct mtd_info *mtd, int idx)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
+	struct tango_chip *tchip = to_tango_chip(chip);
+
+	if (idx < 0)
+		return; /* No "chip unselect" function */
+
+	writel_relaxed(tchip->timing1, nfc->reg_base + NFC_TIMING1);
+	writel_relaxed(tchip->timing2, nfc->reg_base + NFC_TIMING2);
+	writel_relaxed(tchip->xfer_cfg, nfc->reg_base + NFC_XFER_CFG);
+	writel_relaxed(tchip->pkt_0_cfg, nfc->reg_base + NFC_PKT_0_CFG);
+	writel_relaxed(tchip->pkt_n_cfg, nfc->reg_base + NFC_PKT_N_CFG);
+	writel_relaxed(tchip->bb_cfg, nfc->reg_base + NFC_BB_CFG);
+}
+
+/*
+ * The controller does not check for bitflips in erased pages,
+ * therefore software must check instead.
+ */
+static int check_erased_page(struct nand_chip *chip, u8 *buf)
+{
+	u8 *meta = chip->oob_poi + BBM_SIZE;
+	u8 *ecc = chip->oob_poi + BBM_SIZE + METADATA_SIZE;
+	const int ecc_size = chip->ecc.bytes;
+	const int pkt_size = chip->ecc.size;
+	int i, res, meta_len, bitflips = 0;
+
+	for (i = 0; i < chip->ecc.steps; ++i)
+	{
+		meta_len = i ? 0 : METADATA_SIZE;
+		res = nand_check_erased_ecc_chunk(buf, pkt_size, ecc, ecc_size,
+				meta, meta_len, chip->ecc.strength);
+		if (res < 0)
+			return res;
+
+		bitflips = max(res, bitflips);
+		buf += pkt_size;
+		ecc += ecc_size;
+	}
+
+	return bitflips;
+}
+
+static int decode_error_report(struct tango_nfc *nfc)
+{
+	u32 status, res;
+
+	status = readl_relaxed(nfc->reg_base + NFC_XFER_STATUS);
+	if (status & PAGE_IS_EMPTY)
+		return 0;
+
+	res = readl_relaxed(nfc->mem_base + ERROR_REPORT);
+
+	if (DECODE_OK_PKT_0(res) && DECODE_OK_PKT_N(res))
+		return max(ERR_COUNT_PKT_0(res), ERR_COUNT_PKT_N(res));
+
+	return -EBADMSG;
+}
+
+static void tango_dma_callback(void *arg)
+{
+	complete(arg);
+}
+
+static int do_dma(struct tango_nfc *nfc, int dir, int cmd,
+		const void *buf, int len, int page)
+{
+	struct dma_chan *chan = nfc->chan;
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist sg;
+	struct completion tx_done;
+	int err = -EIO;
+	u32 res, val;
+
+	sg_init_one(&sg, buf, len);
+	if (dma_map_sg(chan->device->dev, &sg, 1, dir) != 1)
+		return -EIO;
+
+	desc = dmaengine_prep_slave_sg(chan, &sg, 1, dir, DMA_PREP_INTERRUPT);
+	if (!desc)
+		goto dma_unmap;
+
+	desc->callback = tango_dma_callback;
+	desc->callback_param = &tx_done;
+	init_completion(&tx_done);
+
+	writel_relaxed(MODE_NFC, nfc->pbus_base + PBUS_PAD_MODE);
+
+	writel_relaxed(page, nfc->reg_base + NFC_ADDR_PAGE);
+	writel_relaxed(   0, nfc->reg_base + NFC_ADDR_OFFSET);
+	writel_relaxed( cmd, nfc->reg_base + NFC_FLASH_CMD);
+
+	dmaengine_submit(desc);
+	dma_async_issue_pending(chan);
+
+	res = wait_for_completion_timeout(&tx_done, HZ);
+	if (res > 0) {
+		void __iomem *addr = nfc->reg_base + NFC_STATUS;
+		err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
+	}
+
+	writel_relaxed(MODE_RAW, nfc->pbus_base + PBUS_PAD_MODE);
+
+dma_unmap:
+	dma_unmap_sg(chan->device->dev, &sg, 1, dir);
+
+	return err;
+}
+
+static int tango_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+		uint8_t *buf, int oob_required, int page)
+{
+	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
+	int err, res, len = mtd->writesize;
+
+	if (oob_required)
+		chip->ecc.read_oob(mtd, chip, page);
+
+	err = do_dma(nfc, DMA_FROM_DEVICE, NFC_READ, buf, len, page);
+	if (err)
+		return err;
+
+	res = decode_error_report(nfc);
+	if (res < 0) {
+		chip->ecc.read_oob_raw(mtd, chip, page);
+		res = check_erased_page(chip, buf);
+	}
+
+	return res;
+}
+
+static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+		const uint8_t *buf, int oob_required, int page)
+{
+	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
+	int err, len = mtd->writesize;
+
+	writel_relaxed(0xffffffff, nfc->mem_base + METADATA);
+	err = do_dma(nfc, DMA_TO_DEVICE, NFC_WRITE, buf, len, page);
+	if (err)
+		return err;
+
+	if (oob_required)
+		return -ENOTSUPP; /* Sending PAGEPROG twice is forbidden */
+
+	return 0;
+}
+
+static void aux_read(struct nand_chip *chip, u8 **buf, int len, int *pos)
+{
+	*pos += len;
+
+	if (*buf == NULL) /* skip over len bytes */
+		chip->cmdfunc(&chip->mtd, NAND_CMD_RNDOUT, *pos, -1);
+	else {
+		tango_read_buf(&chip->mtd, *buf, len);
+		*buf += len;
+	}
+}
+
+static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
+{
+	*pos += len;
+
+	if (*buf == NULL) /* skip over len bytes */
+		chip->cmdfunc(&chip->mtd, NAND_CMD_SEQIN, *pos, -1);
+	else {
+		tango_write_buf(&chip->mtd, *buf, len);
+		*buf += len;
+	}
+}
+
+/*
+ * Physical page layout (not drawn to scale)
+ *
+ * NB: Bad Block Marker area splits PKT_N in two (N1, N2).
+ *
+ * +---+-----------------+-------+-----+-----------+-----+----+-------+
+ * | M |      PKT_0      | ECC_0 | ... |     N1    | BBM | N2 | ECC_N |
+ * +---+-----------------+-------+-----+-----------+-----+----+-------+
+ *
+ * Logical page layout:
+ *
+ *       +-----+---+-------+-----+-------+
+ * oob = | BBM | M | ECC_0 | ... | ECC_N |
+ *       +-----+---+-------+-----+-------+
+ *
+ *       +-----------------+-----+-----------------+
+ * buf = |      PKT_0      | ... |      PKT_N      |
+ *       +-----------------+-----+-----------------+
+ */
+static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+{
+	u8 *oob_orig = oob;
+	const int page_size = chip->mtd.writesize;
+	const int ecc_size = chip->ecc.bytes;
+	const int pkt_size = chip->ecc.size;
+	int pos = 0; /* position within physical page */
+	int rem = page_size; /* bytes remaining until BBM area */
+
+	if (oob != NULL)
+		oob += BBM_SIZE;
+
+	aux_read(chip, &oob, METADATA_SIZE, &pos);
+
+	while (rem > pkt_size) {
+		aux_read(chip, &buf, pkt_size, &pos);
+		aux_read(chip, &oob, ecc_size, &pos);
+		rem = page_size - pos;
+	}
+
+	aux_read(chip, &buf, rem, &pos);
+	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
+	aux_read(chip, &buf, pkt_size - rem, &pos);
+	aux_read(chip, &oob, ecc_size, &pos);
+
+	return 0;
+}
+
+static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+{
+	const u8 *oob_orig = oob;
+	const int page_size = chip->mtd.writesize;
+	const int ecc_size = chip->ecc.bytes;
+	const int pkt_size = chip->ecc.size;
+	int pos = 0; /* position within physical page */
+	int rem = page_size; /* bytes remaining until BBM area */
+
+	if (oob != NULL)
+		oob += BBM_SIZE;
+
+	aux_write(chip, &oob, METADATA_SIZE, &pos);
+
+	while (rem > pkt_size) {
+		aux_write(chip, &buf, pkt_size, &pos);
+		aux_write(chip, &oob, ecc_size, &pos);
+		rem = page_size - pos;
+	}
+
+	aux_write(chip, &buf, rem, &pos);
+	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
+	aux_write(chip, &buf, pkt_size - rem, &pos);
+	aux_write(chip, &oob, ecc_size, &pos);
+
+	return 0;
+}
+
+static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+		uint8_t *buf, int oob_required, int page)
+{
+	return raw_read(chip, buf, chip->oob_poi);
+}
+
+static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+		const uint8_t *buf, int oob_required, int page)
+{
+	return raw_write(chip, buf, chip->oob_poi);
+}
+
+static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
+{
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	return raw_read(chip, NULL, chip->oob_poi);
+}
+
+static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
+{
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+	raw_write(chip, NULL, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	chip->waitfunc(mtd, chip);
+	return 0;
+}
+
+static int oob_ecc(struct mtd_info *mtd, int idx, struct mtd_oob_region *res)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (idx >= ecc->steps)
+		return -ERANGE;
+
+	res->offset = BBM_SIZE + METADATA_SIZE + ecc->bytes * idx;
+	res->length = ecc->bytes;
+
+	return 0;
+}
+
+static int oob_free(struct mtd_info *mtd, int idx, struct mtd_oob_region *res)
+{
+	return -ERANGE; /* no free space in spare area */
+}
+
+static const struct mtd_ooblayout_ops tango_nand_ooblayout_ops = {
+	.ecc	= oob_ecc,
+	.free	= oob_free,
+};
+
+static u32 to_ticks(int kHz, int ps)
+{
+	return DIV_ROUND_UP_ULL((u64)kHz * ps, NSEC_PER_SEC);
+}
+
+static int tango_set_timings(struct mtd_info *mtd,
+		const struct nand_data_interface *conf, bool check_only)
+{
+	const struct nand_sdr_timings *sdr = nand_get_sdr_timings(conf);
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
+	struct tango_chip *tchip = to_tango_chip(chip);
+	u32 Trdy, Textw, Twc, Twpw, Tacc, Thold, Trpw, Textr;
+	int kHz = nfc->freq_kHz;
+
+	if (IS_ERR(sdr))
+		return PTR_ERR(sdr);
+
+	if (check_only)
+		return 0;
+
+	Trdy = to_ticks(kHz, sdr->tCEA_max - sdr->tREA_max);
+	Textw = to_ticks(kHz, sdr->tWB_max);
+	Twc = to_ticks(kHz, sdr->tWC_min);
+	Twpw = to_ticks(kHz, sdr->tWC_min - sdr->tWP_min);
+
+	Tacc = to_ticks(kHz, sdr->tREA_max);
+	Thold = to_ticks(kHz, sdr->tREH_min);
+	Trpw = to_ticks(kHz, sdr->tRC_min - sdr->tREH_min);
+	Textr = to_ticks(kHz, sdr->tRHZ_max);
+
+	tchip->timing1 = TIMING(Trdy, Textw, Twc, Twpw);
+	tchip->timing2 = TIMING(Tacc, Thold, Trpw, Textr);
+
+	return 0;
+}
+
+static int chip_init(struct device *dev, struct device_node *np)
+{
+	int err, res;
+	u32 cs, ecc_bits;
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+	struct tango_chip *tchip;
+	struct nand_ecc_ctrl *ecc;
+	struct tango_nfc *nfc = dev_get_drvdata(dev);
+
+	tchip = devm_kzalloc(dev, sizeof(*tchip), GFP_KERNEL);
+	if (!tchip)
+		return -ENOMEM;
+
+	res = of_property_count_u32_elems(np, "reg");
+	if (res < 0)
+		return res;
+
+	if (res != 1)
+		return -ENOTSUPP; /* Multi-CS chips are not supported */
+
+	err = of_property_read_u32_index(np, "reg", 0, &cs);
+	if (err)
+		return err;
+
+	if (cs >= MAX_CS)
+		return -EINVAL;
+
+	chip = &tchip->nand_chip;
+	ecc = &chip->ecc;
+	mtd = &chip->mtd;
+
+	chip->read_byte = tango_read_byte;
+	chip->write_buf = tango_write_buf;
+	chip->read_buf = tango_read_buf;
+	chip->select_chip = tango_select_chip;
+	chip->cmd_ctrl = tango_cmd_ctrl;
+	chip->dev_ready = tango_dev_ready;
+	chip->setup_data_interface = tango_set_timings;
+	chip->options = 0
+		| NAND_USE_BOUNCE_BUFFER
+		| NAND_NO_SUBPAGE_WRITE
+		| NAND_WAIT_TCCS;
+	chip->controller = &nfc->hw;
+	tchip->base = nfc->pbus_base + (cs * 256);
+
+	nand_set_flash_node(chip, np);
+	mtd_set_ooblayout(mtd, &tango_nand_ooblayout_ops);
+	mtd->dev.parent = dev;
+
+	ecc->mode = NAND_ECC_HW;
+	ecc->algo = NAND_ECC_BCH;
+
+	err = nand_scan_ident(mtd, 1, NULL);
+	if (err)
+		return err;
+
+	ecc->read_page_raw = tango_read_page_raw;
+	ecc->write_page_raw = tango_write_page_raw;
+	ecc->read_page = tango_read_page;
+	ecc->write_page = tango_write_page;
+	ecc->read_oob = tango_read_oob;
+	ecc->write_oob = tango_write_oob;
+
+	ecc_bits = ecc->strength * FIELD_ORDER;
+	ecc->bytes = DIV_ROUND_UP(ecc_bits, BITS_PER_BYTE);
+
+	err = nand_scan_tail(mtd);
+	if (err)
+		return err;
+
+	tchip->xfer_cfg = XFER_CFG(cs, 1, ecc->steps, METADATA_SIZE);
+	tchip->pkt_0_cfg = PKT_CFG(ecc->size + METADATA_SIZE, ecc->strength);
+	tchip->pkt_n_cfg = PKT_CFG(ecc->size, ecc->strength);
+	tchip->bb_cfg = BB_CFG(mtd->writesize, BBM_SIZE);
+
+	err = mtd_device_register(mtd, NULL, 0);
+	if (err)
+		return err;
+
+	nfc->chips[cs] = tchip;
+
+	return 0;
+}
+
+static int tango_nand_remove(struct platform_device *pdev)
+{
+	int cs;
+	struct tango_nfc *nfc = platform_get_drvdata(pdev);
+
+	dma_release_channel(nfc->chan);
+
+	for (cs = 0; cs < MAX_CS; ++cs)
+		if (nfc->chips[cs] != NULL)
+			nand_release(&nfc->chips[cs]->nand_chip.mtd);
+
+	return 0;
+}
+
+static int tango_nand_probe(struct platform_device *pdev)
+{
+	struct clk *clk;
+	struct resource *res;
+	struct tango_nfc *nfc;
+	struct device_node *np;
+
+	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
+	if (!nfc)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	nfc->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(nfc->reg_base))
+		return PTR_ERR(nfc->reg_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	nfc->mem_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(nfc->mem_base))
+		return PTR_ERR(nfc->mem_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	nfc->pbus_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(nfc->pbus_base))
+		return PTR_ERR(nfc->pbus_base);
+
+	clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	nfc->chan = dma_request_chan(&pdev->dev, "nfc_sbox");
+	if (IS_ERR(nfc->chan))
+		return PTR_ERR(nfc->chan);
+
+	platform_set_drvdata(pdev, nfc);
+	nand_hw_control_init(&nfc->hw);
+	nfc->freq_kHz = clk_get_rate(clk) / 1000;
+
+	for_each_child_of_node(pdev->dev.of_node, np) {
+		int err = chip_init(&pdev->dev, np);
+		if (err) {
+			tango_nand_remove(pdev);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id tango_nand_ids[] = {
+	{ .compatible = "sigma,smp8758-nand" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver tango_nand_driver = {
+	.probe	= tango_nand_probe,
+	.remove	= tango_nand_remove,
+	.driver	= {
+		.name		= "tango-nand",
+		.of_match_table	= tango_nand_ids,
+	},
+};
+
+module_platform_driver(tango_nand_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Sigma Designs");
+MODULE_DESCRIPTION("Tango4 NAND Flash controller driver");
-- 
2.9.0

Re: [PATCH v6 4/4] mtd: nand: tango: import driver for tango chips

[Boris Brezillon]

Hi Marc,

On Mon, 24 Oct 2016 14:55:16 +0200
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> wrote:

> This driver supports the NAND Flash controller embedded in recent
> Tango chips, such as SMP8758 and SMP8759.

A few comments below (most of them are about coding style).

> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
>  drivers/mtd/nand/Kconfig      |   6 +
>  drivers/mtd/nand/Makefile     |   1 +
>  drivers/mtd/nand/tango_nand.c | 654 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 661 insertions(+)
>  create mode 100644 drivers/mtd/nand/tango_nand.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 7b7a887b4709..844ab20a23a4 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -205,6 +205,12 @@ config MTD_NAND_S3C2410_CLKSTOP
>  	  when the is NAND chip selected or released, but will save
>  	  approximately 5mA of power when there is nothing happening.
>  
> +config MTD_NAND_TANGO
> +	tristate "NAND Flash support for Tango chips"
> +	depends on ARCH_TANGO
> +	help
> +	  Enables the NAND Flash controller on Tango chips.
> +
>  config MTD_NAND_DISKONCHIP
>  	tristate "DiskOnChip 2000, Millennium and Millennium Plus (NAND reimplementation)"
>  	depends on HAS_IOMEM
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index cafde6f3d957..4904ad3614fb 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_MTD_NAND_DENALI_DT)	+= denali_dt.o
>  obj-$(CONFIG_MTD_NAND_AU1550)		+= au1550nd.o
>  obj-$(CONFIG_MTD_NAND_BF5XX)		+= bf5xx_nand.o
>  obj-$(CONFIG_MTD_NAND_S3C2410)		+= s3c2410.o
> +obj-$(CONFIG_MTD_NAND_TANGO)		+= tango_nand.o
>  obj-$(CONFIG_MTD_NAND_DAVINCI)		+= davinci_nand.o
>  obj-$(CONFIG_MTD_NAND_DISKONCHIP)	+= diskonchip.o
>  obj-$(CONFIG_MTD_NAND_DOCG4)		+= docg4.o
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> new file mode 100644
> index 000000000000..375ca8e4a3aa
> --- /dev/null
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -0,0 +1,654 @@
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
> +
> +/* Offsets relative to chip->base */
> +#define PBUS_CMD	0
> +#define PBUS_ADDR	4
> +#define PBUS_DATA	8
> +
> +/* Offsets relative to reg_base */
> +#define NFC_STATUS	0x00
> +#define NFC_FLASH_CMD	0x04
> +#define NFC_DEVICE_CFG	0x08
> +#define NFC_TIMING1	0x0c
> +#define NFC_TIMING2	0x10
> +#define NFC_XFER_CFG	0x14
> +#define NFC_PKT_0_CFG	0x18
> +#define NFC_PKT_N_CFG	0x1c
> +#define NFC_BB_CFG	0x20
> +#define NFC_ADDR_PAGE	0x24
> +#define NFC_ADDR_OFFSET	0x28
> +#define NFC_XFER_STATUS	0x2c
> +
> +/* NFC_STATUS values */
> +#define CMD_READY	BIT(31)
> +
> +/* NFC_FLASH_CMD values */
> +#define NFC_READ	1
> +#define NFC_WRITE	2
> +
> +/* NFC_XFER_STATUS values */
> +#define PAGE_IS_EMPTY	BIT(16)
> +
> +/* Offsets relative to mem_base */
> +#define METADATA	0x000
> +#define ERROR_REPORT	0x1c0
> +
> +/*
> + * Error reports are split in two bytes:
> + * byte 0 for the first packet in the page (PKT_0)
> + * byte 1 for other packets in the page (PKT_N, for N > 0)
> + * ERR_COUNT_PKT_N is the max error count over all but the first packet.
> + */
> +#define DECODE_OK_PKT_0(v)	(v & BIT(7))
> +#define DECODE_OK_PKT_N(v)	(v & BIT(15))
> +#define ERR_COUNT_PKT_0(v)	((v >> 0) & 0x3f)
> +#define ERR_COUNT_PKT_N(v)	((v >> 8) & 0x3f)
> +
> +/* Offsets relative to pbus_base */
> +#define PBUS_CS_CTRL	0x83c
> +#define PBUS_PAD_MODE	0x8f0
> +
> +/* PBUS_CS_CTRL values */
> +#define PBUS_IORDY	BIT(31)
> +
> +/*
> + * PBUS_PAD_MODE values
> + * In raw mode, the driver communicates directly with the NAND chips.
> + * In NFC mode, the NAND Flash controller manages the communication.
> + * We use NFC mode for read and write; raw mode for everything else.
> + */
> +#define MODE_RAW	0
> +#define MODE_NFC	BIT(31)
> +
> +#define METADATA_SIZE	4
> +#define BBM_SIZE	6
> +#define FIELD_ORDER	15
> +
> +#define MAX_CS		4
> +
> +struct tango_nfc {
> +	struct nand_hw_control hw;
> +	void __iomem *reg_base;
> +	void __iomem *mem_base;
> +	void __iomem *pbus_base;
> +	struct tango_chip *chips[MAX_CS];
> +	struct dma_chan *chan;
> +	int freq_kHz;
> +};
> +
> +#define to_tango_nfc(ptr) container_of(ptr, struct tango_nfc, hw)
> +
> +struct tango_chip {
> +	struct nand_chip nand_chip;
> +	void __iomem *base;
> +	u32 timing1;
> +	u32 timing2;
> +	u32 xfer_cfg;
> +	u32 pkt_0_cfg;
> +	u32 pkt_n_cfg;
> +	u32 bb_cfg;
> +};
> +
> +#define to_tango_chip(ptr) container_of(ptr, struct tango_chip, nand_chip)
> +
> +#define XFER_CFG(cs, page_count, steps, metadata_size)	\
> +	((cs) << 24 | (page_count) << 16 | (steps) << 8 | (metadata_size) << 0)
> +
> +#define PKT_CFG(size, strength) ((size) << 16 | (strength) << 0)
> +
> +#define BB_CFG(bb_offset, bb_size) ((bb_offset) << 16 | (bb_size) << 0)
> +
> +#define TIMING(t0, t1, t2, t3) (t0 << 24 | t1 << 16 | t2 << 8 | t3 << 0)

Wrap tX params with parenthesis to make sure the shift operation is
applied to the correct value (for example, if t0 = a + b, then the
resulting operation will be a + (b << 24) instead of (a + b) << 24 if
you don't put those parenthesis).

> +
> +static void tango_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
> +{
> +	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	if (ctrl & NAND_CLE)
> +		writeb_relaxed(dat, tchip->base + PBUS_CMD);
> +
> +	if (ctrl & NAND_ALE)
> +		writeb_relaxed(dat, tchip->base + PBUS_ADDR);
> +}
> +
> +static int tango_dev_ready(struct mtd_info *mtd)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> +
> +	return readl_relaxed(nfc->pbus_base + PBUS_CS_CTRL) & PBUS_IORDY;
> +}
> +
> +static uint8_t tango_read_byte(struct mtd_info *mtd)
> +{
> +	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	return readb_relaxed(tchip->base + PBUS_DATA);
> +}
> +
> +static void tango_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	ioread8_rep(tchip->base + PBUS_DATA, buf, len);
> +}
> +
> +static void tango_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> +{
> +	struct tango_chip *tchip = to_tango_chip(mtd_to_nand(mtd));
> +
> +	iowrite8_rep(tchip->base + PBUS_DATA, buf, len);
> +}
> +
> +static void tango_select_chip(struct mtd_info *mtd, int idx)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> +	struct tango_chip *tchip = to_tango_chip(chip);
> +
> +	if (idx < 0)
> +		return; /* No "chip unselect" function */
> +
> +	writel_relaxed(tchip->timing1, nfc->reg_base + NFC_TIMING1);
> +	writel_relaxed(tchip->timing2, nfc->reg_base + NFC_TIMING2);
> +	writel_relaxed(tchip->xfer_cfg, nfc->reg_base + NFC_XFER_CFG);
> +	writel_relaxed(tchip->pkt_0_cfg, nfc->reg_base + NFC_PKT_0_CFG);
> +	writel_relaxed(tchip->pkt_n_cfg, nfc->reg_base + NFC_PKT_N_CFG);
> +	writel_relaxed(tchip->bb_cfg, nfc->reg_base + NFC_BB_CFG);
> +}
> +
> +/*
> + * The controller does not check for bitflips in erased pages,
> + * therefore software must check instead.
> + */
> +static int check_erased_page(struct nand_chip *chip, u8 *buf)
> +{
> +	u8 *meta = chip->oob_poi + BBM_SIZE;
> +	u8 *ecc = chip->oob_poi + BBM_SIZE + METADATA_SIZE;
> +	const int ecc_size = chip->ecc.bytes;
> +	const int pkt_size = chip->ecc.size;
> +	int i, res, meta_len, bitflips = 0;
> +
> +	for (i = 0; i < chip->ecc.steps; ++i)
> +	{
> +		meta_len = i ? 0 : METADATA_SIZE;
> +		res = nand_check_erased_ecc_chunk(buf, pkt_size, ecc, ecc_size,
> +				meta, meta_len, chip->ecc.strength);
> +		if (res < 0)
> +			return res;
> +
> +		bitflips = max(res, bitflips);
> +		buf += pkt_size;
> +		ecc += ecc_size;
> +	}
> +
> +	return bitflips;
> +}
> +
> +static int decode_error_report(struct tango_nfc *nfc)
> +{
> +	u32 status, res;
> +
> +	status = readl_relaxed(nfc->reg_base + NFC_XFER_STATUS);
> +	if (status & PAGE_IS_EMPTY)
> +		return 0;
> +
> +	res = readl_relaxed(nfc->mem_base + ERROR_REPORT);
> +
> +	if (DECODE_OK_PKT_0(res) && DECODE_OK_PKT_N(res))
> +		return max(ERR_COUNT_PKT_0(res), ERR_COUNT_PKT_N(res));
> +
> +	return -EBADMSG;
> +}
> +
> +static void tango_dma_callback(void *arg)
> +{
> +	complete(arg);
> +}
> +
> +static int do_dma(struct tango_nfc *nfc, int dir, int cmd,
> +		const void *buf, int len, int page)
> +{
> +	struct dma_chan *chan = nfc->chan;
> +	struct dma_async_tx_descriptor *desc;
> +	struct scatterlist sg;
> +	struct completion tx_done;
> +	int err = -EIO;
> +	u32 res, val;
> +
> +	sg_init_one(&sg, buf, len);
> +	if (dma_map_sg(chan->device->dev, &sg, 1, dir) != 1)
> +		return -EIO;
> +
> +	desc = dmaengine_prep_slave_sg(chan, &sg, 1, dir, DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		goto dma_unmap;
> +
> +	desc->callback = tango_dma_callback;
> +	desc->callback_param = &tx_done;
> +	init_completion(&tx_done);
> +
> +	writel_relaxed(MODE_NFC, nfc->pbus_base + PBUS_PAD_MODE);
> +
> +	writel_relaxed(page, nfc->reg_base + NFC_ADDR_PAGE);
> +	writel_relaxed(   0, nfc->reg_base + NFC_ADDR_OFFSET);
> +	writel_relaxed( cmd, nfc->reg_base + NFC_FLASH_CMD);

A vestige of you weird alignment policy ;). Please fix that.

> +
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(chan);
> +
> +	res = wait_for_completion_timeout(&tx_done, HZ);
> +	if (res > 0) {
> +		void __iomem *addr = nfc->reg_base + NFC_STATUS;
> +		err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
> +	}

I'd still prefer:

	if (res > 0)
		err = readl_poll_timeout(addr, val, val & CMD_READY, 0,
					 1000);

> +
> +	writel_relaxed(MODE_RAW, nfc->pbus_base + PBUS_PAD_MODE);
> +
> +dma_unmap:
> +	dma_unmap_sg(chan->device->dev, &sg, 1, dir);
> +
> +	return err;
> +}
> +
> +static int tango_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> +		uint8_t *buf, int oob_required, int page)
> +{
> +	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> +	int err, res, len = mtd->writesize;
> +
> +	if (oob_required)
> +		chip->ecc.read_oob(mtd, chip, page);
> +
> +	err = do_dma(nfc, DMA_FROM_DEVICE, NFC_READ, buf, len, page);
> +	if (err)
> +		return err;
> +
> +	res = decode_error_report(nfc);
> +	if (res < 0) {
> +		chip->ecc.read_oob_raw(mtd, chip, page);
> +		res = check_erased_page(chip, buf);
> +	}

You should not return an error, when the ECC engine detected
uncorrectable errors. You should just increment ecc->failed.

> +
> +	return res;
> +}
> +
> +static int tango_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +		const uint8_t *buf, int oob_required, int page)
> +{
> +	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> +	int err, len = mtd->writesize;
> +
> +	writel_relaxed(0xffffffff, nfc->mem_base + METADATA);
> +	err = do_dma(nfc, DMA_TO_DEVICE, NFC_WRITE, buf, len, page);
> +	if (err)
> +		return err;
> +
> +	if (oob_required)
> +		return -ENOTSUPP; /* Sending PAGEPROG twice is forbidden */

Please put the comment before the oob_required test.
You should probably test that before lauching the DMA transfer.

> +
> +	return 0;
> +}
> +
> +static void aux_read(struct nand_chip *chip, u8 **buf, int len, int *pos)
> +{
> +	*pos += len;
> +
> +	if (*buf == NULL) /* skip over len bytes */
> +		chip->cmdfunc(&chip->mtd, NAND_CMD_RNDOUT, *pos, -1);
> +	else {
> +		tango_read_buf(&chip->mtd, *buf, len);
> +		*buf += len;
> +	}

	if (*buf == NULL) {
		/* skip over len bytes */
		chip->cmdfunc(&chip->mtd, NAND_CMD_RNDOUT, *pos, -1);
	} else {
		tango_read_buf(&chip->mtd, *buf, len);
		*buf += len;
	}

> +}
> +
> +static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
> +{
> +	*pos += len;
> +
> +	if (*buf == NULL) /* skip over len bytes */
> +		chip->cmdfunc(&chip->mtd, NAND_CMD_SEQIN, *pos, -1);
> +	else {
> +		tango_write_buf(&chip->mtd, *buf, len);
> +		*buf += len;
> +	}

Ditto.

> +}
> +
> +/*
> + * Physical page layout (not drawn to scale)
> + *
> + * NB: Bad Block Marker area splits PKT_N in two (N1, N2).
> + *
> + * +---+-----------------+-------+-----+-----------+-----+----+-------+
> + * | M |      PKT_0      | ECC_0 | ... |     N1    | BBM | N2 | ECC_N |
> + * +---+-----------------+-------+-----+-----------+-----+----+-------+
> + *
> + * Logical page layout:
> + *
> + *       +-----+---+-------+-----+-------+
> + * oob = | BBM | M | ECC_0 | ... | ECC_N |
> + *       +-----+---+-------+-----+-------+
> + *
> + *       +-----------------+-----+-----------------+
> + * buf = |      PKT_0      | ... |      PKT_N      |
> + *       +-----------------+-----+-----------------+
> + */
> +static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
> +{
> +	u8 *oob_orig = oob;
> +	const int page_size = chip->mtd.writesize;
> +	const int ecc_size = chip->ecc.bytes;
> +	const int pkt_size = chip->ecc.size;
> +	int pos = 0; /* position within physical page */
> +	int rem = page_size; /* bytes remaining until BBM area */
> +
> +	if (oob != NULL)
> +		oob += BBM_SIZE;
> +
> +	aux_read(chip, &oob, METADATA_SIZE, &pos);
> +
> +	while (rem > pkt_size) {
> +		aux_read(chip, &buf, pkt_size, &pos);
> +		aux_read(chip, &oob, ecc_size, &pos);
> +		rem = page_size - pos;
> +	}
> +
> +	aux_read(chip, &buf, rem, &pos);
> +	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
> +	aux_read(chip, &buf, pkt_size - rem, &pos);
> +	aux_read(chip, &oob, ecc_size, &pos);
> +
> +	return 0;
> +}
> +
> +static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
> +{
> +	const u8 *oob_orig = oob;
> +	const int page_size = chip->mtd.writesize;
> +	const int ecc_size = chip->ecc.bytes;
> +	const int pkt_size = chip->ecc.size;
> +	int pos = 0; /* position within physical page */
> +	int rem = page_size; /* bytes remaining until BBM area */
> +
> +	if (oob != NULL)
> +		oob += BBM_SIZE;
> +
> +	aux_write(chip, &oob, METADATA_SIZE, &pos);
> +
> +	while (rem > pkt_size) {
> +		aux_write(chip, &buf, pkt_size, &pos);
> +		aux_write(chip, &oob, ecc_size, &pos);
> +		rem = page_size - pos;
> +	}
> +
> +	aux_write(chip, &buf, rem, &pos);
> +	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
> +	aux_write(chip, &buf, pkt_size - rem, &pos);
> +	aux_write(chip, &oob, ecc_size, &pos);
> +
> +	return 0;
> +}
> +
> +static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +		uint8_t *buf, int oob_required, int page)
> +{
> +	return raw_read(chip, buf, chip->oob_poi);
> +}
> +
> +static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +		const uint8_t *buf, int oob_required, int page)
> +{
> +	return raw_write(chip, buf, chip->oob_poi);
> +}
> +
> +static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
> +{
> +	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
> +	return raw_read(chip, NULL, chip->oob_poi);
> +}
> +
> +static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip, int page)
> +{
> +	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
> +	raw_write(chip, NULL, chip->oob_poi);
> +	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
> +	chip->waitfunc(mtd, chip);
> +	return 0;
> +}
> +
> +static int oob_ecc(struct mtd_info *mtd, int idx, struct mtd_oob_region *res)
> +{
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> +
> +	if (idx >= ecc->steps)
> +		return -ERANGE;
> +
> +	res->offset = BBM_SIZE + METADATA_SIZE + ecc->bytes * idx;
> +	res->length = ecc->bytes;
> +
> +	return 0;
> +}
> +
> +static int oob_free(struct mtd_info *mtd, int idx, struct mtd_oob_region *res)
> +{
> +	return -ERANGE; /* no free space in spare area */
> +}
> +
> +static const struct mtd_ooblayout_ops tango_nand_ooblayout_ops = {
> +	.ecc	= oob_ecc,
> +	.free	= oob_free,
> +};
> +
> +static u32 to_ticks(int kHz, int ps)
> +{
> +	return DIV_ROUND_UP_ULL((u64)kHz * ps, NSEC_PER_SEC);
> +}
> +
> +static int tango_set_timings(struct mtd_info *mtd,
> +		const struct nand_data_interface *conf, bool check_only)
> +{
> +	const struct nand_sdr_timings *sdr = nand_get_sdr_timings(conf);
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct tango_nfc *nfc = to_tango_nfc(chip->controller);
> +	struct tango_chip *tchip = to_tango_chip(chip);
> +	u32 Trdy, Textw, Twc, Twpw, Tacc, Thold, Trpw, Textr;
> +	int kHz = nfc->freq_kHz;
> +
> +	if (IS_ERR(sdr))
> +		return PTR_ERR(sdr);
> +
> +	if (check_only)
> +		return 0;
> +
> +	Trdy = to_ticks(kHz, sdr->tCEA_max - sdr->tREA_max);
> +	Textw = to_ticks(kHz, sdr->tWB_max);
> +	Twc = to_ticks(kHz, sdr->tWC_min);
> +	Twpw = to_ticks(kHz, sdr->tWC_min - sdr->tWP_min);
> +
> +	Tacc = to_ticks(kHz, sdr->tREA_max);
> +	Thold = to_ticks(kHz, sdr->tREH_min);
> +	Trpw = to_ticks(kHz, sdr->tRC_min - sdr->tREH_min);
> +	Textr = to_ticks(kHz, sdr->tRHZ_max);
> +
> +	tchip->timing1 = TIMING(Trdy, Textw, Twc, Twpw);
> +	tchip->timing2 = TIMING(Tacc, Thold, Trpw, Textr);
> +
> +	return 0;
> +}
> +
> +static int chip_init(struct device *dev, struct device_node *np)
> +{
> +	int err, res;
> +	u32 cs, ecc_bits;
> +	struct mtd_info *mtd;
> +	struct nand_chip *chip;
> +	struct tango_chip *tchip;
> +	struct nand_ecc_ctrl *ecc;
> +	struct tango_nfc *nfc = dev_get_drvdata(dev);
> +
> +	tchip = devm_kzalloc(dev, sizeof(*tchip), GFP_KERNEL);
> +	if (!tchip)
> +		return -ENOMEM;
> +
> +	res = of_property_count_u32_elems(np, "reg");
> +	if (res < 0)
> +		return res;
> +
> +	if (res != 1)
> +		return -ENOTSUPP; /* Multi-CS chips are not supported */
> +
> +	err = of_property_read_u32_index(np, "reg", 0, &cs);
> +	if (err)
> +		return err;
> +
> +	if (cs >= MAX_CS)
> +		return -EINVAL;
> +
> +	chip = &tchip->nand_chip;
> +	ecc = &chip->ecc;
> +	mtd = &chip->mtd;
> +
> +	chip->read_byte = tango_read_byte;
> +	chip->write_buf = tango_write_buf;
> +	chip->read_buf = tango_read_buf;
> +	chip->select_chip = tango_select_chip;
> +	chip->cmd_ctrl = tango_cmd_ctrl;
> +	chip->dev_ready = tango_dev_ready;
> +	chip->setup_data_interface = tango_set_timings;
> +	chip->options = 0
> +		| NAND_USE_BOUNCE_BUFFER
> +		| NAND_NO_SUBPAGE_WRITE
> +		| NAND_WAIT_TCCS;

	chip->options = NAND_USE_BOUNCE_BUFFER | NAND_NO_SUBPAGE_WRITE |
			NAND_WAIT_TCCS;

> +	chip->controller = &nfc->hw;
> +	tchip->base = nfc->pbus_base + (cs * 256);
> +
> +	nand_set_flash_node(chip, np);
> +	mtd_set_ooblayout(mtd, &tango_nand_ooblayout_ops);
> +	mtd->dev.parent = dev;
> +
> +	ecc->mode = NAND_ECC_HW;
> +	ecc->algo = NAND_ECC_BCH;

You only support HW ECC, so please put these two line after
nand_scan_ident(). Otherwise, the DT parsing done in nand_base.c might
override the ->mode and ->algo values.

> +
> +	err = nand_scan_ident(mtd, 1, NULL);
> +	if (err)
> +		return err;
> +
> +	ecc->read_page_raw = tango_read_page_raw;
> +	ecc->write_page_raw = tango_write_page_raw;
> +	ecc->read_page = tango_read_page;
> +	ecc->write_page = tango_write_page;
> +	ecc->read_oob = tango_read_oob;
> +	ecc->write_oob = tango_write_oob;
> +
> +	ecc_bits = ecc->strength * FIELD_ORDER;
> +	ecc->bytes = DIV_ROUND_UP(ecc_bits, BITS_PER_BYTE);
> +
> +	err = nand_scan_tail(mtd);
> +	if (err)
> +		return err;
> +
> +	tchip->xfer_cfg = XFER_CFG(cs, 1, ecc->steps, METADATA_SIZE);
> +	tchip->pkt_0_cfg = PKT_CFG(ecc->size + METADATA_SIZE, ecc->strength);
> +	tchip->pkt_n_cfg = PKT_CFG(ecc->size, ecc->strength);
> +	tchip->bb_cfg = BB_CFG(mtd->writesize, BBM_SIZE);
> +
> +	err = mtd_device_register(mtd, NULL, 0);
> +	if (err)
> +		return err;
> +
> +	nfc->chips[cs] = tchip;
> +
> +	return 0;
> +}
> +
> +static int tango_nand_remove(struct platform_device *pdev)
> +{
> +	int cs;
> +	struct tango_nfc *nfc = platform_get_drvdata(pdev);
> +
> +	dma_release_channel(nfc->chan);
> +
> +	for (cs = 0; cs < MAX_CS; ++cs)
> +		if (nfc->chips[cs] != NULL)
> +			nand_release(&nfc->chips[cs]->nand_chip.mtd);

Please add curly braces around the for loop.

> +
> +	return 0;
> +}
> +
> +static int tango_nand_probe(struct platform_device *pdev)
> +{
> +	struct clk *clk;
> +	struct resource *res;
> +	struct tango_nfc *nfc;
> +	struct device_node *np;
> +
> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> +	if (!nfc)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	nfc->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(nfc->reg_base))
> +		return PTR_ERR(nfc->reg_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	nfc->mem_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(nfc->mem_base))
> +		return PTR_ERR(nfc->mem_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	nfc->pbus_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(nfc->pbus_base))
> +		return PTR_ERR(nfc->pbus_base);
> +
> +	clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	nfc->chan = dma_request_chan(&pdev->dev, "nfc_sbox");
> +	if (IS_ERR(nfc->chan))
> +		return PTR_ERR(nfc->chan);
> +
> +	platform_set_drvdata(pdev, nfc);
> +	nand_hw_control_init(&nfc->hw);
> +	nfc->freq_kHz = clk_get_rate(clk) / 1000;
> +
> +	for_each_child_of_node(pdev->dev.of_node, np) {
> +		int err = chip_init(&pdev->dev, np);
> +		if (err) {
> +			tango_nand_remove(pdev);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tango_nand_ids[] = {
> +	{ .compatible = "sigma,smp8758-nand" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver tango_nand_driver = {
> +	.probe	= tango_nand_probe,
> +	.remove	= tango_nand_remove,
> +	.driver	= {
> +		.name		= "tango-nand",
> +		.of_match_table	= tango_nand_ids,
> +	},
> +};
> +
> +module_platform_driver(tango_nand_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Sigma Designs");
> +MODULE_DESCRIPTION("Tango4 NAND Flash controller driver");

Re: [PATCH v6 4/4] mtd: nand: tango: import driver for tango chips

[Marc Gonzalez]

Heya :-)

On 24/10/2016 17:10, Boris Brezillon wrote:

> Marc Gonzalez wrote:
> 
>> This driver supports the NAND Flash controller embedded in recent
>> Tango chips, such as SMP8758 and SMP8759.
> 
> A few comments below (most of them are about coding style).

20c25f430311 tweak chip->options
cd1f53e6d2b8 Move ECC init code around
b90efb09a921 Increment ecc_stats.failed on error
2236f67af8d6 move brace for checkpatch
83c1c0311c2a braces in for loop
13faec8fa36b proper braces for if-block
f879bf99d5c3 Move test before DMA write
a2d319b18702 no brackets for readl_poll_timeout
67acc45e26e1 Remove nice alignment
dd71ae037721 fix TIMING macro

Time to squash and send v7, along with the bindings doc.


>> +#define TIMING(t0, t1, t2, t3) (t0 << 24 | t1 << 16 | t2 << 8 | t3 << 0)
> 
> Wrap tX params with parenthesis to make sure the shift operation is
> applied to the correct value (for example, if t0 = a + b, then the
> resulting operation will be a + (b << 24) instead of (a + b) << 24 if
> you don't put those parenthesis).

Fixed.

For the record, addition has higher precedence than shift.
Thus a + b << n = (a + b) << n
But your point is valid for bit-wise operations, e.g.
a & b << n = a & (b << n)

>> +	writel_relaxed(page, nfc->reg_base + NFC_ADDR_PAGE);
>> +	writel_relaxed(   0, nfc->reg_base + NFC_ADDR_OFFSET);
>> +	writel_relaxed( cmd, nfc->reg_base + NFC_FLASH_CMD);
> 
> A vestige of you weird alignment policy ;). Please fix that.

Fixed. Even checkpatch complains...
(Sad puppy face)

Regards.