[PATCH 0/5] mtd: nand: vf610_nfc: Freescale NFC for VF610

[PATCH 0/5] mtd: nand: vf610_nfc: Freescale NFC for VF610

[Stefan Agner]

This adds support for Freescale NAND flash controller (NFC) found on
various devices such as Vybrid (VF610), MPC5125, MCF54418 (ColdFire)
and Kinetis K70.

The patchset is based on the patchset by Bill Pringlemeir, see:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/295419

A variant of this driver is in upstream U-Boot now. During this
process, various points have been discussed and changed. Those
changes are also incorporated in this patchset, including the
change to the name "vf610_nfc" (discussion in first patchset).
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/192465
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/193142
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/193351
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/195441
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/195513

MTD tests worked, except mtd_nandecctest.ko: The test stopped
already after changing the first bit. I guess this is due to not
properly working raw page read/write implementation. However,
implementing those functions is not feasible since ECC is always
done on-the-fly.

Performance keypoints (slightly less for page write/reads):
[ 1782.284398] mtd_speedtest: testing eraseblock write speed
[ 1963.472907] mtd_speedtest: eraseblock write speed is 2877 KiB/s
[ 1963.478913] mtd_speedtest: testing eraseblock read speed
[ 2004.906267] mtd_speedtest: eraseblock read speed is 12586 KiB/s

The driver has only been tested on Freescale Vybrid. Tests on other
platforms welcome, especially on big endian architectures...

Changes since RFC (Bill Pringlemeir):
- Renamed driver from fsl_nfc to vf610_nfc
- Use readl/writel for all register in accessor functions
- Optimized field accessor functions
- Implemented PM (suspend/resume) functions
- Implemented basic support for ECC strength/ECC step size from dt
- Improved performance of count_written_bits by using hweight32
- Support ECC with 60-bytes to correct up to 32 bit errors
- Changed to in-band BBT (NAND_BBT_NO_OOB) which also allows ECC modes
  which uses up to 60 bytes on 64 byte OOB
- Removed custom (downstream) BBT pattern since BBT table won't be
  compatible anyway (due to the change above)

Stefan Agner (5):
  mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
  mtd: nand: vf610_nfc: add hardware BCH-ECC support
  mtd: nand: vf610_nfc: add device tree bindings
  ARM: dts: vf610: add NAND flash controller peripherial
  ARM: dts: vf-colibri: enable NAND flash controller

 .../devicetree/bindings/mtd/vf610-nfc.txt          |  38 +
 arch/arm/boot/dts/vf-colibri.dtsi                  |  31 +
 arch/arm/boot/dts/vf500.dtsi                       |   4 +
 arch/arm/boot/dts/vf610-twr.dts                    |  43 +
 arch/arm/boot/dts/vfxxx.dtsi                       |   7 +
 arch/arm/mach-imx/Kconfig                          |   1 +
 drivers/mtd/nand/Kconfig                           |  14 +
 drivers/mtd/nand/Makefile                          |   1 +
 drivers/mtd/nand/vf610_nfc.c                       | 873 +++++++++++++++++++++
 9 files changed, 1012 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/vf610-nfc.txt
 create mode 100644 drivers/mtd/nand/vf610_nfc.c

-- 
2.3.0

[PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Stefan Agner]

This driver supports Freescale NFC (NAND flash controller) found on
Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.

Limitations:
- DMA and pipelining not used
- Pages larger than 2k are not supported
- No hardware ECC

The driver has only been tested on Vybrid (VF610).

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/mach-imx/Kconfig    |   1 +
 drivers/mtd/nand/Kconfig     |  12 +
 drivers/mtd/nand/Makefile    |   1 +
 drivers/mtd/nand/vf610_nfc.c | 730 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 744 insertions(+)
 create mode 100644 drivers/mtd/nand/vf610_nfc.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e8627e0..de4a51a 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -634,6 +634,7 @@ config SOC_VF610
 	select ARM_GIC
 	select PINCTRL_VF610
 	select PL310_ERRATA_769419 if CACHE_L2X0
+	select HAVE_NAND_VF610_NFC
 
 	help
 	  This enable support for Freescale Vybrid VF610 processor.
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b76a17..1be30a6 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
 	  This enables the driver for the NAND flash controller on the
 	  MPC5121 SoC.
 
+config HAVE_NAND_VF610_NFC
+	bool
+
+config MTD_NAND_VF610_NFC
+	tristate "Support for Freescale NFC for VF610/MPC5125"
+	depends on HAVE_NAND_VF610_NFC
+	help
+	  Enables support for NAND Flash Controller on some Freescale
+	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+	  The driver supports a maximum 2k page size. The driver
+	  currently does not support hardware ECC.
+
 config MTD_NAND_MXC
 	tristate "MXC NAND support"
 	depends on ARCH_MXC
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 582bbd05..e97ca7b 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES)		+= socrates_nand.o
 obj-$(CONFIG_MTD_NAND_TXX9NDFMC)	+= txx9ndfmc.o
 obj-$(CONFIG_MTD_NAND_NUC900)		+= nuc900_nand.o
 obj-$(CONFIG_MTD_NAND_MPC5121_NFC)	+= mpc5121_nfc.o
+obj-$(CONFIG_MTD_NAND_VF610_NFC)	+= vf610_nfc.o
 obj-$(CONFIG_MTD_NAND_RICOH)		+= r852.o
 obj-$(CONFIG_MTD_NAND_JZ4740)		+= jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
new file mode 100644
index 0000000..101fd20
--- /dev/null
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -0,0 +1,730 @@
+/*
+ * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
+ *
+ * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
+ * Jason ported to M54418TWR and MVFA5 (VF610).
+ * Authors: Stefan Agner <stefan.agner@toradex.com>
+ *          Bill Pringlemeir <bpringlemeir@nbsps.com>
+ *          Shaohui Xie <b21989@freescale.com>
+ *          Jason Jin <Jason.jin@freescale.com>
+ *
+ * Based on original driver mpc5121_nfc.c.
+ *
+ * This 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.
+ *
+ * Limitations:
+ * - Untested on MPC5125 and M54418.
+ * - DMA not used.
+ * - 2K pages or less.
+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of_mtd.h>
+
+#define	DRV_NAME		"vf610_nfc"
+
+/* Register Offsets */
+#define NFC_FLASH_CMD1			0x3F00
+#define NFC_FLASH_CMD2			0x3F04
+#define NFC_COL_ADDR			0x3F08
+#define NFC_ROW_ADDR			0x3F0c
+#define NFC_ROW_ADDR_INC		0x3F14
+#define NFC_FLASH_STATUS1		0x3F18
+#define NFC_FLASH_STATUS2		0x3F1c
+#define NFC_CACHE_SWAP			0x3F28
+#define NFC_SECTOR_SIZE			0x3F2c
+#define NFC_FLASH_CONFIG		0x3F30
+#define NFC_IRQ_STATUS			0x3F38
+
+/* Addresses for NFC MAIN RAM BUFFER areas */
+#define NFC_MAIN_AREA(n)		((n) *  0x1000)
+
+#define PAGE_2K				0x0800
+#define OOB_64				0x0040
+
+/*
+ * NFC_CMD2[CODE] values. See section:
+ *  - 31.4.7 Flash Command Code Description, Vybrid manual
+ *  - 23.8.6 Flash Command Sequencer, MPC5125 manual
+ *
+ * Briefly these are bitmasks of controller cycles.
+ */
+#define READ_PAGE_CMD_CODE		0x7EE0
+#define PROGRAM_PAGE_CMD_CODE		0x7FC0
+#define ERASE_CMD_CODE			0x4EC0
+#define READ_ID_CMD_CODE		0x4804
+#define RESET_CMD_CODE			0x4040
+#define STATUS_READ_CMD_CODE		0x4068
+
+/* NFC ECC mode define */
+#define ECC_BYPASS			0
+
+/*** Register Mask and bit definitions */
+
+/* NFC_FLASH_CMD1 Field */
+#define CMD_BYTE2_MASK				0xFF000000
+#define CMD_BYTE2_SHIFT				24
+
+/* NFC_FLASH_CM2 Field */
+#define CMD_BYTE1_MASK				0xFF000000
+#define CMD_BYTE1_SHIFT				24
+#define CMD_CODE_MASK				0x00FFFF00
+#define CMD_CODE_SHIFT				8
+#define BUFNO_MASK				0x00000006
+#define BUFNO_SHIFT				1
+#define START_BIT				(1<<0)
+
+/* NFC_COL_ADDR Field */
+#define COL_ADDR_MASK				0x0000FFFF
+#define COL_ADDR_SHIFT				0
+
+/* NFC_ROW_ADDR Field */
+#define ROW_ADDR_MASK				0x00FFFFFF
+#define ROW_ADDR_SHIFT				0
+#define ROW_ADDR_CHIP_SEL_RB_MASK		0xF0000000
+#define ROW_ADDR_CHIP_SEL_RB_SHIFT		28
+#define ROW_ADDR_CHIP_SEL_MASK			0x0F000000
+#define ROW_ADDR_CHIP_SEL_SHIFT			24
+
+/* NFC_FLASH_STATUS2 Field */
+#define STATUS_BYTE1_MASK			0x000000FF
+
+/* NFC_FLASH_CONFIG Field */
+#define CONFIG_ECC_SRAM_REQ_BIT			(1<<21)
+#define CONFIG_DMA_REQ_BIT			(1<<20)
+#define CONFIG_ECC_MODE_MASK			0x000E0000
+#define CONFIG_ECC_MODE_SHIFT			17
+#define CONFIG_FAST_FLASH_BIT			(1<<16)
+#define CONFIG_16BIT				(1<<7)
+#define CONFIG_BOOT_MODE_BIT			(1<<6)
+#define CONFIG_ADDR_AUTO_INCR_BIT		(1<<5)
+#define CONFIG_BUFNO_AUTO_INCR_BIT		(1<<4)
+#define CONFIG_PAGE_CNT_MASK			0xF
+#define CONFIG_PAGE_CNT_SHIFT			0
+
+/* NFC_IRQ_STATUS Field */
+#define IDLE_IRQ_BIT				(1<<29)
+#define IDLE_EN_BIT				(1<<20)
+#define CMD_DONE_CLEAR_BIT			(1<<18)
+#define IDLE_CLEAR_BIT				(1<<17)
+
+#define NFC_TIMEOUT		(HZ)
+
+struct vf610_nfc_config {
+	int width;
+	int flash_bbt;
+	u32 clkrate;
+};
+
+struct fsl_nfc {
+	struct mtd_info	   mtd;
+	struct nand_chip   chip;
+	struct device	  *dev;
+	void __iomem	  *regs;
+	wait_queue_head_t  irq_waitq;
+	uint               column;
+	int                spareonly;
+	int                page;
+	/* Status and ID are in alternate locations. */
+	int                alt_buf;
+#define ALT_BUF_ID   1
+#define ALT_BUF_STAT 2
+	struct clk        *clk;
+
+	struct vf610_nfc_config *cfg;
+};
+
+#define mtd_to_nfc(_mtd) container_of(_mtd, struct vf610_nfc, mtd)
+
+static u8 bbt_pattern[] = {'B', 'b', 't', '0' };
+static u8 mirror_pattern[] = {'1', 't', 'b', 'B' };
+
+static struct nand_bbt_descr bbt_main_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
+		   NAND_BBT_2BIT | NAND_BBT_VERSION,
+	.offs =	11,
+	.len = 4,
+	.veroffs = 15,
+	.maxblocks = 4,
+	.pattern = bbt_pattern,
+};
+
+static struct nand_bbt_descr bbt_mirror_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
+		   NAND_BBT_2BIT | NAND_BBT_VERSION,
+	.offs =	11,
+	.len = 4,
+	.veroffs = 15,
+	.maxblocks = 4,
+	.pattern = mirror_pattern,
+};
+
+static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
+{
+	return readl(nfc->regs + reg);
+}
+
+static inline void vf610_nfc_write(struct vf610_nfc *nfc, uint reg, u32 val)
+{
+	writel(val, nfc->regs + reg);
+}
+
+static inline void vf610_nfc_set(struct vf610_nfc *nfc, uint reg, u32 bits)
+{
+	vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) | bits);
+}
+
+static inline void vf610_nfc_clear(struct vf610_nfc *nfc, uint reg, u32 bits)
+{
+	vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) & ~bits);
+}
+
+static inline void vf610_nfc_set_field(struct vf610_nfc *nfc, u32 reg,
+				       u32 mask, u32 shift, u32 val)
+{
+	vf610_nfc_write(nfc, reg,
+			(vf610_nfc_read(nfc, reg) & (~mask)) | val << shift);
+}
+
+static inline void vf610_nfc_memcpy(void *dst, const void *src, size_t n)
+{
+	/*
+	 * Use this accessor for the interal SRAM buffers. On ARM we can
+	 * treat the SRAM buffer as if its memory, hence use memcpy
+	 */
+	memcpy(dst, src, n);
+}
+
+/* Clear flags for upcoming command */
+static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc)
+{
+	void __iomem *reg = nfc->regs + NFC_IRQ_STATUS;
+	u32 tmp = __raw_readl(reg);
+
+	tmp |= CMD_DONE_CLEAR_BIT | IDLE_CLEAR_BIT;
+	__raw_writel(tmp, reg);
+}
+
+static inline void vf610_nfc_done(struct vf610_nfc *nfc)
+{
+	int rv;
+
+	/*
+	 * Barrier is needed after this write. This write need
+	 * to be done before reading the next register the first
+	 * time.
+	 * vf610_nfc_set implicates such a barrier by using writel
+	 * to write to the register.
+	 */
+	vf610_nfc_set(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
+	vf610_nfc_set(nfc, NFC_FLASH_CMD2, START_BIT);
+
+	if (!(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT)) {
+		rv = wait_event_timeout(nfc->irq_waitq,
+			(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT),
+					NFC_TIMEOUT);
+		if (!rv)
+			dev_warn(nfc->dev, "Timeout while waiting for BUSY.\n");
+	}
+	vf610_nfc_clear_status(nfc);
+}
+
+static u8 vf610_nfc_get_id(struct vf610_nfc *nfc, int col)
+{
+	u32 flash_id;
+
+	if (col < 4) {
+		flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS1);
+		return (flash_id >> (3-col)*8) & 0xff;
+	} else {
+		flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS2);
+		return flash_id >> 24;
+	}
+}
+
+static u8 vf610_nfc_get_status(struct vf610_nfc *nfc)
+{
+	return vf610_nfc_read(nfc, NFC_FLASH_STATUS2) & STATUS_BYTE1_MASK;
+}
+
+static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
+				   u32 cmd_code)
+{
+	void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
+	u32 tmp;
+
+	vf610_nfc_clear_status(nfc);
+
+	tmp = __raw_readl(reg);
+	tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
+	tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
+	tmp |= cmd_code << CMD_CODE_SHIFT;
+	__raw_writel(tmp, reg);
+}
+
+static void vf610_nfc_send_commands(struct vf610_nfc *nfc, u32 cmd_byte1,
+				    u32 cmd_byte2, u32 cmd_code)
+{
+	void __iomem *reg = nfc->regs + NFC_FLASH_CMD1;
+	u32 tmp;
+
+	vf610_nfc_send_command(nfc, cmd_byte1, cmd_code);
+
+	tmp = __raw_readl(reg);
+	tmp &= ~CMD_BYTE2_MASK;
+	tmp |= cmd_byte2 << CMD_BYTE2_SHIFT;
+	__raw_writel(tmp, reg);
+}
+
+static irqreturn_t vf610_nfc_irq(int irq, void *data)
+{
+	struct mtd_info *mtd = data;
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+	vf610_nfc_clear(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
+	wake_up(&nfc->irq_waitq);
+
+	return IRQ_HANDLED;
+}
+
+static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
+{
+	if (column != -1) {
+		if (nfc->chip.options | NAND_BUSWIDTH_16)
+			column = column/2;
+		vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
+				    COL_ADDR_SHIFT, column);
+	}
+	if (page != -1)
+		vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
+				    ROW_ADDR_SHIFT, page);
+}
+
+static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
+			      int column, int page)
+{
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+	nfc->column     = max(column, 0);
+	nfc->spareonly	= 0;
+	nfc->alt_buf	= 0;
+
+	switch (command) {
+	case NAND_CMD_PAGEPROG:
+		nfc->page = -1;
+		vf610_nfc_send_commands(nfc, NAND_CMD_SEQIN,
+					command, PROGRAM_PAGE_CMD_CODE);
+		vf610_nfc_addr_cycle(nfc, column, page);
+		break;
+
+	case NAND_CMD_RESET:
+		vf610_nfc_send_command(nfc, command, RESET_CMD_CODE);
+		break;
+	/*
+	 * NFC does not support sub-page reads and writes,
+	 * so emulate them using full page transfers.
+	 */
+	case NAND_CMD_READOOB:
+		nfc->spareonly = 1;
+	case NAND_CMD_SEQIN: /* Pre-read for partial writes. */
+	case NAND_CMD_READ0:
+		column = 0;
+		/* Already read? */
+		if (nfc->page == page)
+			return;
+		nfc->page = page;
+		vf610_nfc_send_commands(nfc, NAND_CMD_READ0,
+					NAND_CMD_READSTART, READ_PAGE_CMD_CODE);
+		vf610_nfc_addr_cycle(nfc, column, page);
+		break;
+
+	case NAND_CMD_ERASE1:
+		if (nfc->page == page)
+			nfc->page = -1;
+		vf610_nfc_send_commands(nfc, command,
+				  NAND_CMD_ERASE2, ERASE_CMD_CODE);
+		vf610_nfc_addr_cycle(nfc, column, page);
+		break;
+
+	case NAND_CMD_READID:
+		nfc->alt_buf = ALT_BUF_ID;
+		vf610_nfc_send_command(nfc, command, READ_ID_CMD_CODE);
+		break;
+
+	case NAND_CMD_STATUS:
+		nfc->alt_buf = ALT_BUF_STAT;
+		vf610_nfc_send_command(nfc, command, STATUS_READ_CMD_CODE);
+		break;
+	default:
+		return;
+	}
+
+	vf610_nfc_done(nfc);
+}
+
+static inline void vf610_nfc_read_spare(struct mtd_info *mtd, void *buf,
+					int len)
+{
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+	len = min_t(uint, mtd->oobsize, len);
+	if (len > 0)
+		vf610_nfc_memcpy(buf, nfc->regs + mtd->writesize, len);
+}
+
+static void vf610_nfc_read_buf(struct mtd_info *mtd, u_char *buf, int len)
+{
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+	uint c = nfc->column;
+	uint l;
+
+	/* Handle main area */
+	if (!nfc->spareonly) {
+		l = min_t(uint, len, mtd->writesize - c);
+		nfc->column += l;
+
+		if (!nfc->alt_buf)
+			vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, l);
+		else
+			if (nfc->alt_buf & ALT_BUF_ID)
+				*buf = vf610_nfc_get_id(nfc, c);
+			else
+				*buf = vf610_nfc_get_status(nfc);
+
+		buf += l;
+		len -= l;
+	}
+
+	/* Handle spare area access */
+	if (len) {
+		nfc->column += len;
+		vf610_nfc_read_spare(mtd, buf, len);
+	}
+}
+
+static void vf610_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+	uint c = nfc->column;
+	uint l;
+
+	l = min_t(uint, len, mtd->writesize + mtd->oobsize - c);
+	nfc->column += l;
+	vf610_nfc_memcpy(nfc->regs + NFC_MAIN_AREA(0) + c, buf, l);
+}
+
+static uint8_t vf610_nfc_read_byte(struct mtd_info *mtd)
+{
+	u8 tmp;
+
+	vf610_nfc_read_buf(mtd, &tmp, sizeof(tmp));
+	return tmp;
+}
+
+static u16 vf610_nfc_read_word(struct mtd_info *mtd)
+{
+	u16 tmp;
+
+	vf610_nfc_read_buf(mtd, (u_char *)&tmp, sizeof(tmp));
+	return tmp;
+}
+
+/* If not provided, upper layers apply a fixed delay. */
+static int vf610_nfc_dev_ready(struct mtd_info *mtd)
+{
+	/* NFC handles R/B internally; always ready.  */
+	return 1;
+}
+
+/*
+ * This function supports Vybrid only (MPC5125 would have full RB and four CS)
+ */
+static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
+{
+#ifdef CONFIG_SOC_VF610
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+	u32 tmp = vf610_nfc_read(nfc, NFC_ROW_ADDR);
+
+	tmp &= ~(ROW_ADDR_CHIP_SEL_RB_MASK | ROW_ADDR_CHIP_SEL_MASK);
+	tmp |= 1 << ROW_ADDR_CHIP_SEL_RB_SHIFT;
+
+	if (chip == 0)
+		tmp |= 1 << ROW_ADDR_CHIP_SEL_SHIFT;
+	else if (chip == 1)
+		tmp |= 2 << ROW_ADDR_CHIP_SEL_SHIFT;
+
+	vf610_nfc_write(nfc, NFC_ROW_ADDR, tmp);
+#endif
+}
+
+#ifdef CONFIG_OF_MTD
+static const struct of_device_id vf610_nfc_dt_ids[] = {
+	{ .compatible = "fsl,vf610-nfc" },
+	{ .compatible = "fsl,mpc5125-nfc" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, vf610_nfc_dt_ids);
+
+static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
+{
+	struct device_node *np = dev->of_node;
+	int buswidth;
+	u32 clkrate;
+
+	if (!np)
+		return 1;
+
+	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
+
+	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
+		cfg->clkrate = clkrate;
+
+	buswidth = of_get_nand_bus_width(np);
+	if (buswidth < 0)
+		return buswidth;
+
+	cfg->width = buswidth;
+
+	return 0;
+}
+#else
+static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
+{
+	return 0;
+}
+#endif
+
+static int vf610_nfc_init_controller(struct vf610_nfc *nfc, int page_sz)
+{
+	struct vf610_nfc_config *cfg = nfc->cfg;
+
+	if (cfg->width == 16)
+		vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
+	else
+		vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
+
+	/* Set configuration register. */
+	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
+	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
+	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
+	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
+	vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
+
+	/* PAGE_CNT = 1 */
+	vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
+			    CONFIG_PAGE_CNT_SHIFT, 1);
+
+	/* Set ECC_STATUS offset */
+	vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
+			    CONFIG_ECC_SRAM_ADDR_MASK,
+			    CONFIG_ECC_SRAM_ADDR_SHIFT, ECC_SRAM_ADDR);
+
+	vf610_nfc_write(nfc, NFC_SECTOR_SIZE, page_sz);
+
+	return 0;
+}
+
+static int vf610_nfc_probe(struct platform_device *pdev)
+{
+	struct vf610_nfc *nfc;
+	struct resource *res;
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+	struct vf610_nfc_config *cfg;
+	int err = 0;
+	int page_sz;
+	int irq;
+
+	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
+	if (!nfc)
+		return -ENOMEM;
+
+	nfc->cfg = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
+	if (!nfc->cfg)
+		return -ENOMEM;
+	cfg = nfc->cfg;
+
+	nfc->dev = &pdev->dev;
+	nfc->page = -1;
+	mtd = &nfc->mtd;
+	chip = &nfc->chip;
+
+	mtd->priv = chip;
+	mtd->owner = THIS_MODULE;
+	mtd->dev.parent = nfc->dev;
+	mtd->name = DRV_NAME;
+
+	err = vf610_nfc_probe_dt(nfc->dev, cfg);
+	if (err)
+		return -ENODEV;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	nfc->regs = devm_ioremap_resource(nfc->dev, res);
+	if (IS_ERR(nfc->regs))
+		return PTR_ERR(nfc->regs);
+
+	nfc->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(nfc->clk))
+		return PTR_ERR(nfc->clk);
+
+	if (cfg->clkrate && clk_set_rate(nfc->clk, cfg->clkrate)) {
+		dev_err(nfc->dev, "Clock rate not set.");
+		return -EINVAL;
+	}
+
+	err = clk_prepare_enable(nfc->clk);
+	if (err) {
+		dev_err(nfc->dev, "Unable to enable clock!\n");
+		return err;
+	}
+
+	if (cfg->width == 16)
+		chip->options |= NAND_BUSWIDTH_16;
+	else
+		chip->options &= ~NAND_BUSWIDTH_16;
+
+	chip->dev_ready = vf610_nfc_dev_ready;
+	chip->cmdfunc = vf610_nfc_command;
+	chip->read_byte = vf610_nfc_read_byte;
+	chip->read_word = vf610_nfc_read_word;
+	chip->read_buf = vf610_nfc_read_buf;
+	chip->write_buf = vf610_nfc_write_buf;
+	chip->select_chip = vf610_nfc_select_chip;
+
+	/* Bad block options. */
+	if (cfg->flash_bbt)
+		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_CREATE;
+
+	chip->bbt_td = &bbt_main_descr;
+	chip->bbt_md = &bbt_mirror_descr;
+
+	init_waitqueue_head(&nfc->irq_waitq);
+
+	err = devm_request_irq(nfc->dev, irq, vf610_nfc_irq, 0, DRV_NAME, mtd);
+	if (err) {
+		dev_err(nfc->dev, "Error requesting IRQ!\n");
+		goto error;
+	}
+
+	page_sz = PAGE_2K + OOB_64;
+	page_sz += cfg->width == 16 ? 1 : 0;
+
+	vf610_nfc_init_controller(nfc, page_sz);
+
+	/* first scan to find the device and get the page size */
+	if (nand_scan_ident(mtd, 1, NULL)) {
+		err = -ENXIO;
+		goto error;
+	}
+
+	chip->ecc.mode = NAND_ECC_SOFT; /* default */
+
+	page_sz = mtd->writesize + mtd->oobsize;
+
+	/* Single buffer only, max 256 OOB minus ECC status */
+	if (page_sz > PAGE_2K + 256 - 8) {
+		dev_err(nfc->dev, "Unsupported flash size\n");
+		err = -ENXIO;
+		goto error;
+	}
+	page_sz += cfg->width == 16 ? 1 : 0;
+	vf610_nfc_write(nfc, NFC_SECTOR_SIZE, page_sz);
+
+	/* second phase scan */
+	if (nand_scan_tail(mtd)) {
+		err = -ENXIO;
+		goto error;
+	}
+
+	/* Register device in MTD */
+	mtd_device_parse_register(mtd, NULL,
+		&(struct mtd_part_parser_data){
+			.of_node = pdev->dev.of_node,
+		},
+		NULL, 0);
+
+	platform_set_drvdata(pdev, mtd);
+
+	return 0;
+
+error:
+	clk_disable_unprepare(nfc->clk);
+	return err;
+}
+
+static int vf610_nfc_remove(struct platform_device *pdev)
+{
+	struct mtd_info *mtd = platform_get_drvdata(pdev);
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+	nand_release(mtd);
+	clk_disable_unprepare(nfc->clk);
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int vf610_nfc_suspend(struct device *dev)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+	clk_disable_unprepare(nfc->clk);
+	return 0;
+}
+
+static int vf610_nfc_resume(struct device *dev)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+	int page_sz;
+
+	pinctrl_pm_select_default_state(dev);
+	if (nfc->cfg->clkrate && clk_set_rate(nfc->clk, nfc->cfg->clkrate)) {
+		dev_err(nfc->dev, "Clock rate not set.");
+		return -EINVAL;
+	}
+
+	clk_prepare_enable(nfc->clk);
+
+	page_sz = mtd->writesize + mtd->oobsize;
+	page_sz += nfc->cfg->width == 16 ? 1 : 0;
+
+	vf610_nfc_init_controller(nfc, page_sz);
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(vf610_nfc_pm_ops, vf610_nfc_suspend, vf610_nfc_resume);
+
+static struct platform_driver vf610_nfc_driver = {
+	.driver		= {
+		.name	= DRV_NAME,
+		.of_match_table = vf610_nfc_dt_ids,
+		.pm	= &vf610_nfc_pm_ops,
+	},
+	.probe		= vf610_nfc_probe,
+	.remove		= vf610_nfc_remove,
+};
+
+module_platform_driver(vf610_nfc_driver);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("Freescale VF610/MPC5125 NFC MTD NAND driver");
+MODULE_LICENSE("GPL v2");
-- 
2.3.0

[PATCH 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support

[Stefan Agner]

This adds hardware ECC support using the BCH encoder in the NFC IP.
The ECC encoder supports up to 32-bit correction by using 60 error
correction bytes. There is no sub-page ECC step, ECC is calculated
always accross the whole page (up to 2k pages).

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mtd/nand/Kconfig     |   6 +-
 drivers/mtd/nand/vf610_nfc.c | 193 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 172 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 1be30a6..0fb97b3 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -464,8 +464,10 @@ config MTD_NAND_VF610_NFC
 	help
 	  Enables support for NAND Flash Controller on some Freescale
 	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
-	  The driver supports a maximum 2k page size. The driver
-	  currently does not support hardware ECC.
+	  The driver supports a maximum 2k page size. With 2k pages and
+	  64 bytes or more of OOB, hardware ECC with up to 32-bit error
+	  correction is supported. Hardware ECC is only enabled through
+	  device tree.
 
 config MTD_NAND_MXC
 	tristate "MXC NAND support"
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
index 101fd20..c6fb379 100644
--- a/drivers/mtd/nand/vf610_nfc.c
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -19,6 +19,7 @@
  * - Untested on MPC5125 and M54418.
  * - DMA not used.
  * - 2K pages or less.
+ * - Only 2K page w. 64+OOB and hardware ECC.
  */
 
 #include <linux/module.h>
@@ -71,6 +72,8 @@
 
 /* NFC ECC mode define */
 #define ECC_BYPASS			0
+#define ECC_45_BYTE			6
+#define ECC_60_BYTE			7
 
 /*** Register Mask and bit definitions */
 
@@ -103,6 +106,8 @@
 #define STATUS_BYTE1_MASK			0x000000FF
 
 /* NFC_FLASH_CONFIG Field */
+#define CONFIG_ECC_SRAM_ADDR_MASK		0x7FC00000
+#define CONFIG_ECC_SRAM_ADDR_SHIFT		22
 #define CONFIG_ECC_SRAM_REQ_BIT			(1<<21)
 #define CONFIG_DMA_REQ_BIT			(1<<20)
 #define CONFIG_ECC_MODE_MASK			0x000E0000
@@ -123,13 +128,32 @@
 
 #define NFC_TIMEOUT		(HZ)
 
+/* ECC status placed at end of buffers. */
+#define ECC_SRAM_ADDR	((PAGE_2K+256-8) >> 3)
+#define ECC_STATUS_MASK	0x80
+#define ECC_ERR_COUNT	0x3F
+
+/*
+ * ECC status is stored at NFC_CFG[ECCADD] +4 for little-endian
+ * and +7 for big-endian SoCs.
+ */
+#ifdef CONFIG_SOC_VF610
+#define ECC_OFFSET	4
+#else
+#define ECC_OFFSET	7
+#endif
+
 struct vf610_nfc_config {
+	bool hardware_ecc;
+	int ecc_strength;
+	int ecc_step_size;
 	int width;
 	int flash_bbt;
 	u32 clkrate;
+	u32 ecc_mode;
 };
 
-struct fsl_nfc {
+struct vf610_nfc {
 	struct mtd_info	   mtd;
 	struct nand_chip   chip;
 	struct device	  *dev;
@@ -149,27 +173,32 @@ struct fsl_nfc {
 
 #define mtd_to_nfc(_mtd) container_of(_mtd, struct vf610_nfc, mtd)
 
-static u8 bbt_pattern[] = {'B', 'b', 't', '0' };
-static u8 mirror_pattern[] = {'1', 't', 'b', 'B' };
-
-static struct nand_bbt_descr bbt_main_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
-		   NAND_BBT_2BIT | NAND_BBT_VERSION,
-	.offs =	11,
-	.len = 4,
-	.veroffs = 15,
-	.maxblocks = 4,
-	.pattern = bbt_pattern,
+static struct nand_ecclayout vf610_nfc_ecc45 = {
+	.eccbytes = 45,
+	.eccpos = {19, 20, 21, 22, 23,
+		   24, 25, 26, 27, 28, 29, 30, 31,
+		   32, 33, 34, 35, 36, 37, 38, 39,
+		   40, 41, 42, 43, 44, 45, 46, 47,
+		   48, 49, 50, 51, 52, 53, 54, 55,
+		   56, 57, 58, 59, 60, 61, 62, 63},
+	.oobfree = {
+		{.offset = 2,
+		 .length = 17} }
 };
 
-static struct nand_bbt_descr bbt_mirror_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
-		   NAND_BBT_2BIT | NAND_BBT_VERSION,
-	.offs =	11,
-	.len = 4,
-	.veroffs = 15,
-	.maxblocks = 4,
-	.pattern = mirror_pattern,
+static struct nand_ecclayout vf610_nfc_ecc60 = {
+	.eccbytes = 60,
+	.eccpos = { 4,  5,  6,  7,  8,  9, 10, 11,
+		   12, 13, 14, 15, 16, 17, 18, 19,
+		   20, 21, 22, 23, 24, 25, 26, 27,
+		   28, 29, 30, 31, 32, 33, 34, 35,
+		   36, 37, 38, 39, 40, 41, 42, 43,
+		   44, 45, 46, 47, 48, 49, 50, 51,
+		   52, 53, 54, 55, 56, 57, 58, 59,
+		   60, 61, 62, 63 },
+	.oobfree = {
+		{.offset = 2,
+		 .length = 2} }
 };
 
 static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
@@ -470,6 +499,60 @@ static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
 #endif
 }
 
+/* Count the number of 0's in buff up to max_bits */
+static inline int count_written_bits(uint8_t *buff, int size, int max_bits)
+{
+	uint32_t *buff32 = (uint32_t *)buff;
+	int k, written_bits = 0;
+
+	for (k = 0; k < (size / 4); k++) {
+		written_bits += hweight32(~buff32[k]);
+		if (written_bits > max_bits)
+			break;
+	}
+
+	return written_bits;
+}
+
+static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat,
+					 uint8_t *read_ecc, uint8_t *calc_ecc)
+{
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+	u8 ecc_status;
+	u8 ecc_count;
+	int flip;
+
+	ecc_status = __raw_readb(nfc->regs + ECC_SRAM_ADDR * 8 + ECC_OFFSET);
+	ecc_count = ecc_status & ECC_ERR_COUNT;
+	if (!(ecc_status & ECC_STATUS_MASK))
+		return ecc_count;
+
+	/*
+	 * On an erased page, bit count should be zero or at least
+	 * less then half of the ECC strength
+	 */
+	flip = count_written_bits(dat, nfc->chip.ecc.size, ecc_count);
+
+	if (flip > ecc_count && flip > (nfc->chip.ecc.strength / 2)) {
+		nfc->page = -1;
+		return -1;
+	}
+
+	/* Erased page. */
+	memset(dat, 0xff, nfc->chip.ecc.size);
+	return 0;
+}
+
+static int vf610_nfc_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
+				   u_char *ecc_code)
+{
+	return 0;
+}
+
+static void vf610_nfc_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+}
+
 #ifdef CONFIG_OF_MTD
 static const struct of_device_id vf610_nfc_dt_ids[] = {
 	{ .compatible = "fsl,vf610-nfc" },
@@ -487,7 +570,17 @@ static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
 	if (!np)
 		return 1;
 
+	if (of_get_nand_ecc_mode(np) >= NAND_ECC_HW)
+		cfg->hardware_ecc = 1;
+
 	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
+	cfg->ecc_strength = of_get_nand_ecc_strength(np);
+	if (cfg->ecc_strength < 0)
+		cfg->ecc_strength = 0;
+
+	cfg->ecc_step_size = of_get_nand_ecc_step_size(np);
+	if (cfg->ecc_step_size < 0)
+		cfg->ecc_step_size = 0;
 
 	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
 		cfg->clkrate = clkrate;
@@ -534,6 +627,15 @@ static int vf610_nfc_init_controller(struct vf610_nfc *nfc, int page_sz)
 
 	vf610_nfc_write(nfc, NFC_SECTOR_SIZE, page_sz);
 
+	if (cfg->hardware_ecc) {
+		vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
+				    CONFIG_ECC_MODE_MASK,
+				    CONFIG_ECC_MODE_SHIFT, cfg->ecc_mode);
+
+		/* Enable ECC_STATUS */
+		vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_ECC_SRAM_REQ_BIT);
+	}
+
 	return 0;
 }
 
@@ -610,10 +712,8 @@ static int vf610_nfc_probe(struct platform_device *pdev)
 
 	/* Bad block options. */
 	if (cfg->flash_bbt)
-		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_CREATE;
-
-	chip->bbt_td = &bbt_main_descr;
-	chip->bbt_md = &bbt_mirror_descr;
+		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB |
+				    NAND_BBT_CREATE;
 
 	init_waitqueue_head(&nfc->irq_waitq);
 
@@ -640,13 +740,56 @@ static int vf610_nfc_probe(struct platform_device *pdev)
 
 	/* Single buffer only, max 256 OOB minus ECC status */
 	if (page_sz > PAGE_2K + 256 - 8) {
-		dev_err(nfc->dev, "Unsupported flash size\n");
+		dev_err(nfc->dev, "Unsupported flash page size\n");
 		err = -ENXIO;
 		goto error;
 	}
 	page_sz += cfg->width == 16 ? 1 : 0;
 	vf610_nfc_write(nfc, NFC_SECTOR_SIZE, page_sz);
 
+	if (cfg->hardware_ecc) {
+		if (mtd->writesize != PAGE_2K && mtd->oobsize < 64) {
+			dev_err(nfc->dev, "Unsupported flash with hwecc\n");
+			err = -ENXIO;
+			goto error;
+		}
+
+		if (cfg->ecc_step_size != mtd->writesize) {
+			dev_err(nfc->dev, "Step size needs to be page size\n");
+			err = -ENXIO;
+			goto error;
+		}
+
+		if (cfg->ecc_strength == 32) {
+			cfg->ecc_mode = ECC_60_BYTE;
+			chip->ecc.bytes = 60;
+			chip->ecc.layout = &vf610_nfc_ecc60;
+		} else if (cfg->ecc_strength == 24) {
+			cfg->ecc_mode = ECC_45_BYTE;
+			chip->ecc.bytes = 45;
+			chip->ecc.layout = &vf610_nfc_ecc45;
+		} else {
+			dev_err(nfc->dev, "Unsupported ECC strength\n");
+			err = -ENXIO;
+			goto error;
+		}
+
+		/* propagate ecc.layout to mtd_info */
+		mtd->ecclayout = chip->ecc.layout;
+		chip->ecc.calculate = vf610_nfc_calculate_ecc;
+		chip->ecc.hwctl = vf610_nfc_enable_hwecc;
+		chip->ecc.correct = vf610_nfc_correct_data;
+		chip->ecc.mode = NAND_ECC_HW;
+
+		chip->ecc.size = PAGE_2K;
+		chip->ecc.strength = cfg->ecc_strength;
+
+		/* set ECC mode according to required ECC strength */
+		vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
+				    CONFIG_ECC_MODE_MASK,
+				    CONFIG_ECC_MODE_SHIFT, cfg->ecc_mode);
+	}
+
 	/* second phase scan */
 	if (nand_scan_tail(mtd)) {
 		err = -ENXIO;
-- 
2.3.0

[PATCH 3/5] mtd: nand: vf610_nfc: add device tree bindings

[Stefan Agner]

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 .../devicetree/bindings/mtd/vf610-nfc.txt          | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/vf610-nfc.txt

diff --git a/Documentation/devicetree/bindings/mtd/vf610-nfc.txt b/Documentation/devicetree/bindings/mtd/vf610-nfc.txt
new file mode 100644
index 0000000..54207f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/vf610-nfc.txt
@@ -0,0 +1,38 @@
+Freescale's NAND flash controller (NFC)
+
+This variant of the Freescale NAND flash controller (NFC) is on Vybrid (vf610),
+MPC5125, MCF54418 and Kinetis K70.
+
+Required properties:
+- compatible: "fsl,vf610-nfc"
+- reg: Address range of the mtd chip
+- interrupts: Should contain the STMMAC interrupts
+- nand-bus-width: see nand.txt
+- nand-ecc-mode: see nand.txt
+- nand-on-flash-bbt: see nand.txt
+- clock-frequency : Optional clock rate to NFC in Hz
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+  representing partitions.
+
+Required properties for hardware ECC:
+- nand-ecc-strength: supported strengths are 24 and 32 (see nand.txt)
+- nand-ecc-step-size: step size equals page size, currently only 2k pages are
+  supported
+
+Example:
+
+	nfc: nand@400e0000 {
+		compatible = "fsl,vf610-nfc";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x400e0000 0x4000>;
+		interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clks VF610_CLK_NFC>;
+		clock-names = "nfc";
+		clock-frequency = <33000000>;
+		nand-bus-width = <8>;
+		nand-ecc-mode = "hw";
+		nand-on-flash-bbt;
+		nand-ecc-strength = <32>;
+		nand-ecc-step-size = <2048>;
+	};
-- 
2.3.0

[PATCH 4/5] ARM: dts: vf610: add NAND flash controller peripherial

[Stefan Agner]

This adds the NAND flash controller (NFC) peripherial. The driver
supports the SLC NAND chips found on Freescale's Vybrid Tower System
Module. The Micron NAND chip on the module needs 4-bit ECC per 512
byte page. Use 24-bit ECC per 2k page, which is supported by the
driver.

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/vf500.dtsi    |  4 ++++
 arch/arm/boot/dts/vf610-twr.dts | 43 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/vfxxx.dtsi    |  7 +++++++
 3 files changed, 54 insertions(+)

diff --git a/arch/arm/boot/dts/vf500.dtsi b/arch/arm/boot/dts/vf500.dtsi
index 1dbf8d2..1aa1e47 100644
--- a/arch/arm/boot/dts/vf500.dtsi
+++ b/arch/arm/boot/dts/vf500.dtsi
@@ -118,6 +118,10 @@
 	interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
 };
 
+&nfc {
+	interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+};
+
 &pit {
 	interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
 };
diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index f64fddc..704564e 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -287,6 +287,49 @@
 	status = "okay";
 };
 
+&iomuxc {
+	vf610-twr {
+		pinctrl_nfc_1: nfcgrp_1 {
+			fsl,pins = <
+			VF610_PAD_PTD31__NF_IO15	0x28df
+			VF610_PAD_PTD30__NF_IO14	0x28df
+			VF610_PAD_PTD29__NF_IO13	0x28df
+			VF610_PAD_PTD28__NF_IO12	0x28df
+			VF610_PAD_PTD27__NF_IO11	0x28df
+			VF610_PAD_PTD26__NF_IO10	0x28df
+			VF610_PAD_PTD25__NF_IO9		0x28df
+			VF610_PAD_PTD24__NF_IO8		0x28df
+			VF610_PAD_PTD23__NF_IO7		0x28df
+			VF610_PAD_PTD22__NF_IO6		0x28df
+			VF610_PAD_PTD21__NF_IO5		0x28df
+			VF610_PAD_PTD20__NF_IO4		0x28df
+			VF610_PAD_PTD19__NF_IO3		0x28df
+			VF610_PAD_PTD18__NF_IO2		0x28df
+			VF610_PAD_PTD17__NF_IO1		0x28df
+			VF610_PAD_PTD16__NF_IO0		0x28df
+			VF610_PAD_PTB24__NF_WE_B	0x28c2
+			VF610_PAD_PTB25__NF_CE0_B	0x28c2
+			VF610_PAD_PTB27__NF_RE_B	0x28c2
+			VF610_PAD_PTC26__NF_RB_B	0x283d
+			VF610_PAD_PTC27__NF_ALE		0x28c2
+			VF610_PAD_PTC28__NF_CLE		0x28c2
+			>;
+		};
+	};
+};
+
+&nfc {
+	clock-frequency = <33000000>;
+	nand-bus-width = <16>;
+	nand-ecc-mode = "hw";
+	nand-on-flash-bbt;
+	nand-ecc-strength = <24>;
+	nand-ecc-step-size = <2048>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_nfc_1>;
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index a29c7ce..03b4b55 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -458,6 +458,13 @@
 				status = "disabled";
 			};
 
+			nfc: nand@400e0000 {
+				compatible = "fsl,vf610-nfc";
+				reg = <0x400e0000 0x4000>;
+				clocks = <&clks VF610_CLK_NFC>;
+				clock-names = "nfc";
+				status = "disabled";
+			};
 		};
 	};
 };
-- 
2.3.0

[PATCH 5/5] ARM: dts: vf-colibri: enable NAND flash controller

[Stefan Agner]

Enable NAND access by adding pinmux and NAND flash controller  node
to device tree. The NAND chips currently used on the Colibri VF61
requires 8-bit ECC per 512 byte page, hence specify 32-bit ECC
strength per 2k page size.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/vf-colibri.dtsi | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index 5c2b732..651d13f 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -46,6 +46,18 @@
 	pinctrl-0 = <&pinctrl_i2c0>;
 };
 
+&nfc {
+	clock-frequency = <33000000>;
+	nand-bus-width = <8>;
+	nand-ecc-mode = "hw";
+	nand-on-flash-bbt;
+	nand-ecc-strength = <32>;
+	nand-ecc-step-size = <2048>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_nfc_1>;
+	status = "okay";
+};
+
 &pwm0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm0>;
@@ -141,6 +153,25 @@
 			>;
 		};
 
+		pinctrl_nfc_1: nfcgrp_1 {
+			fsl,pins = <
+				VF610_PAD_PTD23__NF_IO7		0x28df
+				VF610_PAD_PTD22__NF_IO6		0x28df
+				VF610_PAD_PTD21__NF_IO5		0x28df
+				VF610_PAD_PTD20__NF_IO4		0x28df
+				VF610_PAD_PTD19__NF_IO3		0x28df
+				VF610_PAD_PTD18__NF_IO2		0x28df
+				VF610_PAD_PTD17__NF_IO1		0x28df
+				VF610_PAD_PTD16__NF_IO0		0x28df
+				VF610_PAD_PTB24__NF_WE_B	0x28c2
+				VF610_PAD_PTB25__NF_CE0_B	0x28c2
+				VF610_PAD_PTB27__NF_RE_B	0x28c2
+				VF610_PAD_PTC26__NF_RB_B	0x283d
+				VF610_PAD_PTC27__NF_ALE		0x28c2
+				VF610_PAD_PTC28__NF_CLE		0x28c2
+			>;
+		};
+
 		pinctrl_pwm0: pwm0grp {
 			fsl,pins = <
 				VF610_PAD_PTB0__FTM0_CH0		0x1182
-- 
2.3.0

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Paul Bolle]

On Thu, 2015-03-05 at 00:10 +0100, Stefan Agner wrote:
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,730 @@

> + * This 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 states the license is GPL v2 or later.

> +MODULE_LICENSE("GPL v2");

So you probably want
    MODULE_LICENSE("GPL");

here.


Paul Bolle

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Shawn Guo]

On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
> This driver supports Freescale NFC (NAND flash controller) found on
> Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.
> 
> Limitations:
> - DMA and pipelining not used
> - Pages larger than 2k are not supported
> - No hardware ECC
> 
> The driver has only been tested on Vybrid (VF610).
> 
> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  arch/arm/mach-imx/Kconfig    |   1 +

This change shouldn't be part of driver patch.

Shawn

>  drivers/mtd/nand/Kconfig     |  12 +
>  drivers/mtd/nand/Makefile    |   1 +
>  drivers/mtd/nand/vf610_nfc.c | 730 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 744 insertions(+)
>  create mode 100644 drivers/mtd/nand/vf610_nfc.c
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index e8627e0..de4a51a 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -634,6 +634,7 @@ config SOC_VF610
>  	select ARM_GIC
>  	select PINCTRL_VF610
>  	select PL310_ERRATA_769419 if CACHE_L2X0
> +	select HAVE_NAND_VF610_NFC
>  
>  	help
>  	  This enable support for Freescale Vybrid VF610 processor.
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b76a17..1be30a6 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
>  	  This enables the driver for the NAND flash controller on the
>  	  MPC5121 SoC.
>  
> +config HAVE_NAND_VF610_NFC
> +	bool
> +
> +config MTD_NAND_VF610_NFC
> +	tristate "Support for Freescale NFC for VF610/MPC5125"
> +	depends on HAVE_NAND_VF610_NFC
> +	help
> +	  Enables support for NAND Flash Controller on some Freescale
> +	  processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
> +	  The driver supports a maximum 2k page size. The driver
> +	  currently does not support hardware ECC.
> +
>  config MTD_NAND_MXC
>  	tristate "MXC NAND support"
>  	depends on ARCH_MXC
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 582bbd05..e97ca7b 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES)		+= socrates_nand.o
>  obj-$(CONFIG_MTD_NAND_TXX9NDFMC)	+= txx9ndfmc.o
>  obj-$(CONFIG_MTD_NAND_NUC900)		+= nuc900_nand.o
>  obj-$(CONFIG_MTD_NAND_MPC5121_NFC)	+= mpc5121_nfc.o
> +obj-$(CONFIG_MTD_NAND_VF610_NFC)	+= vf610_nfc.o
>  obj-$(CONFIG_MTD_NAND_RICOH)		+= r852.o
>  obj-$(CONFIG_MTD_NAND_JZ4740)		+= jz4740_nand.o
>  obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 0000000..101fd20
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,730 @@
> +/*
> + * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
> + *
> + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
> + * Jason ported to M54418TWR and MVFA5 (VF610).
> + * Authors: Stefan Agner <stefan.agner@toradex.com>
> + *          Bill Pringlemeir <bpringlemeir@nbsps.com>
> + *          Shaohui Xie <b21989@freescale.com>
> + *          Jason Jin <Jason.jin@freescale.com>
> + *
> + * Based on original driver mpc5121_nfc.c.
> + *
> + * This 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.
> + *
> + * Limitations:
> + * - Untested on MPC5125 and M54418.
> + * - DMA not used.
> + * - 2K pages or less.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of_mtd.h>
> +
> +#define	DRV_NAME		"vf610_nfc"
> +
> +/* Register Offsets */
> +#define NFC_FLASH_CMD1			0x3F00
> +#define NFC_FLASH_CMD2			0x3F04
> +#define NFC_COL_ADDR			0x3F08
> +#define NFC_ROW_ADDR			0x3F0c
> +#define NFC_ROW_ADDR_INC		0x3F14
> +#define NFC_FLASH_STATUS1		0x3F18
> +#define NFC_FLASH_STATUS2		0x3F1c
> +#define NFC_CACHE_SWAP			0x3F28
> +#define NFC_SECTOR_SIZE			0x3F2c
> +#define NFC_FLASH_CONFIG		0x3F30
> +#define NFC_IRQ_STATUS			0x3F38
> +
> +/* Addresses for NFC MAIN RAM BUFFER areas */
> +#define NFC_MAIN_AREA(n)		((n) *  0x1000)
> +
> +#define PAGE_2K				0x0800
> +#define OOB_64				0x0040
> +
> +/*
> + * NFC_CMD2[CODE] values. See section:
> + *  - 31.4.7 Flash Command Code Description, Vybrid manual
> + *  - 23.8.6 Flash Command Sequencer, MPC5125 manual
> + *
> + * Briefly these are bitmasks of controller cycles.
> + */
> +#define READ_PAGE_CMD_CODE		0x7EE0
> +#define PROGRAM_PAGE_CMD_CODE		0x7FC0
> +#define ERASE_CMD_CODE			0x4EC0
> +#define READ_ID_CMD_CODE		0x4804
> +#define RESET_CMD_CODE			0x4040
> +#define STATUS_READ_CMD_CODE		0x4068
> +
> +/* NFC ECC mode define */
> +#define ECC_BYPASS			0
> +
> +/*** Register Mask and bit definitions */
> +
> +/* NFC_FLASH_CMD1 Field */
> +#define CMD_BYTE2_MASK				0xFF000000
> +#define CMD_BYTE2_SHIFT				24
> +
> +/* NFC_FLASH_CM2 Field */
> +#define CMD_BYTE1_MASK				0xFF000000
> +#define CMD_BYTE1_SHIFT				24
> +#define CMD_CODE_MASK				0x00FFFF00
> +#define CMD_CODE_SHIFT				8
> +#define BUFNO_MASK				0x00000006
> +#define BUFNO_SHIFT				1
> +#define START_BIT				(1<<0)
> +
> +/* NFC_COL_ADDR Field */
> +#define COL_ADDR_MASK				0x0000FFFF
> +#define COL_ADDR_SHIFT				0
> +
> +/* NFC_ROW_ADDR Field */
> +#define ROW_ADDR_MASK				0x00FFFFFF
> +#define ROW_ADDR_SHIFT				0
> +#define ROW_ADDR_CHIP_SEL_RB_MASK		0xF0000000
> +#define ROW_ADDR_CHIP_SEL_RB_SHIFT		28
> +#define ROW_ADDR_CHIP_SEL_MASK			0x0F000000
> +#define ROW_ADDR_CHIP_SEL_SHIFT			24
> +
> +/* NFC_FLASH_STATUS2 Field */
> +#define STATUS_BYTE1_MASK			0x000000FF
> +
> +/* NFC_FLASH_CONFIG Field */
> +#define CONFIG_ECC_SRAM_REQ_BIT			(1<<21)
> +#define CONFIG_DMA_REQ_BIT			(1<<20)
> +#define CONFIG_ECC_MODE_MASK			0x000E0000
> +#define CONFIG_ECC_MODE_SHIFT			17
> +#define CONFIG_FAST_FLASH_BIT			(1<<16)
> +#define CONFIG_16BIT				(1<<7)
> +#define CONFIG_BOOT_MODE_BIT			(1<<6)
> +#define CONFIG_ADDR_AUTO_INCR_BIT		(1<<5)
> +#define CONFIG_BUFNO_AUTO_INCR_BIT		(1<<4)
> +#define CONFIG_PAGE_CNT_MASK			0xF
> +#define CONFIG_PAGE_CNT_SHIFT			0
> +
> +/* NFC_IRQ_STATUS Field */
> +#define IDLE_IRQ_BIT				(1<<29)
> +#define IDLE_EN_BIT				(1<<20)
> +#define CMD_DONE_CLEAR_BIT			(1<<18)
> +#define IDLE_CLEAR_BIT				(1<<17)
> +
> +#define NFC_TIMEOUT		(HZ)
> +
> +struct vf610_nfc_config {
> +	int width;
> +	int flash_bbt;
> +	u32 clkrate;
> +};
> +
> +struct fsl_nfc {
> +	struct mtd_info	   mtd;
> +	struct nand_chip   chip;
> +	struct device	  *dev;
> +	void __iomem	  *regs;
> +	wait_queue_head_t  irq_waitq;
> +	uint               column;
> +	int                spareonly;
> +	int                page;
> +	/* Status and ID are in alternate locations. */
> +	int                alt_buf;
> +#define ALT_BUF_ID   1
> +#define ALT_BUF_STAT 2
> +	struct clk        *clk;
> +
> +	struct vf610_nfc_config *cfg;
> +};
> +
> +#define mtd_to_nfc(_mtd) container_of(_mtd, struct vf610_nfc, mtd)
> +
> +static u8 bbt_pattern[] = {'B', 'b', 't', '0' };
> +static u8 mirror_pattern[] = {'1', 't', 'b', 'B' };
> +
> +static struct nand_bbt_descr bbt_main_descr = {
> +	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
> +		   NAND_BBT_2BIT | NAND_BBT_VERSION,
> +	.offs =	11,
> +	.len = 4,
> +	.veroffs = 15,
> +	.maxblocks = 4,
> +	.pattern = bbt_pattern,
> +};
> +
> +static struct nand_bbt_descr bbt_mirror_descr = {
> +	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
> +		   NAND_BBT_2BIT | NAND_BBT_VERSION,
> +	.offs =	11,
> +	.len = 4,
> +	.veroffs = 15,
> +	.maxblocks = 4,
> +	.pattern = mirror_pattern,
> +};
> +
> +static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
> +{
> +	return readl(nfc->regs + reg);
> +}
> +
> +static inline void vf610_nfc_write(struct vf610_nfc *nfc, uint reg, u32 val)
> +{
> +	writel(val, nfc->regs + reg);
> +}
> +
> +static inline void vf610_nfc_set(struct vf610_nfc *nfc, uint reg, u32 bits)
> +{
> +	vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) | bits);
> +}
> +
> +static inline void vf610_nfc_clear(struct vf610_nfc *nfc, uint reg, u32 bits)
> +{
> +	vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) & ~bits);
> +}
> +
> +static inline void vf610_nfc_set_field(struct vf610_nfc *nfc, u32 reg,
> +				       u32 mask, u32 shift, u32 val)
> +{
> +	vf610_nfc_write(nfc, reg,
> +			(vf610_nfc_read(nfc, reg) & (~mask)) | val << shift);
> +}
> +
> +static inline void vf610_nfc_memcpy(void *dst, const void *src, size_t n)
> +{
> +	/*
> +	 * Use this accessor for the interal SRAM buffers. On ARM we can
> +	 * treat the SRAM buffer as if its memory, hence use memcpy
> +	 */
> +	memcpy(dst, src, n);
> +}
> +
> +/* Clear flags for upcoming command */
> +static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc)
> +{
> +	void __iomem *reg = nfc->regs + NFC_IRQ_STATUS;
> +	u32 tmp = __raw_readl(reg);
> +
> +	tmp |= CMD_DONE_CLEAR_BIT | IDLE_CLEAR_BIT;
> +	__raw_writel(tmp, reg);
> +}
> +
> +static inline void vf610_nfc_done(struct vf610_nfc *nfc)
> +{
> +	int rv;
> +
> +	/*
> +	 * Barrier is needed after this write. This write need
> +	 * to be done before reading the next register the first
> +	 * time.
> +	 * vf610_nfc_set implicates such a barrier by using writel
> +	 * to write to the register.
> +	 */
> +	vf610_nfc_set(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
> +	vf610_nfc_set(nfc, NFC_FLASH_CMD2, START_BIT);
> +
> +	if (!(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT)) {
> +		rv = wait_event_timeout(nfc->irq_waitq,
> +			(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT),
> +					NFC_TIMEOUT);
> +		if (!rv)
> +			dev_warn(nfc->dev, "Timeout while waiting for BUSY.\n");
> +	}
> +	vf610_nfc_clear_status(nfc);
> +}
> +
> +static u8 vf610_nfc_get_id(struct vf610_nfc *nfc, int col)
> +{
> +	u32 flash_id;
> +
> +	if (col < 4) {
> +		flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS1);
> +		return (flash_id >> (3-col)*8) & 0xff;
> +	} else {
> +		flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS2);
> +		return flash_id >> 24;
> +	}
> +}
> +
> +static u8 vf610_nfc_get_status(struct vf610_nfc *nfc)
> +{
> +	return vf610_nfc_read(nfc, NFC_FLASH_STATUS2) & STATUS_BYTE1_MASK;
> +}
> +
> +static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
> +				   u32 cmd_code)
> +{
> +	void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
> +	u32 tmp;
> +
> +	vf610_nfc_clear_status(nfc);
> +
> +	tmp = __raw_readl(reg);
> +	tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
> +	tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
> +	tmp |= cmd_code << CMD_CODE_SHIFT;
> +	__raw_writel(tmp, reg);
> +}
> +
> +static void vf610_nfc_send_commands(struct vf610_nfc *nfc, u32 cmd_byte1,
> +				    u32 cmd_byte2, u32 cmd_code)
> +{
> +	void __iomem *reg = nfc->regs + NFC_FLASH_CMD1;
> +	u32 tmp;
> +
> +	vf610_nfc_send_command(nfc, cmd_byte1, cmd_code);
> +
> +	tmp = __raw_readl(reg);
> +	tmp &= ~CMD_BYTE2_MASK;
> +	tmp |= cmd_byte2 << CMD_BYTE2_SHIFT;
> +	__raw_writel(tmp, reg);
> +}
> +
> +static irqreturn_t vf610_nfc_irq(int irq, void *data)
> +{
> +	struct mtd_info *mtd = data;
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> +	vf610_nfc_clear(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
> +	wake_up(&nfc->irq_waitq);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
> +{
> +	if (column != -1) {
> +		if (nfc->chip.options | NAND_BUSWIDTH_16)
> +			column = column/2;
> +		vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
> +				    COL_ADDR_SHIFT, column);
> +	}
> +	if (page != -1)
> +		vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
> +				    ROW_ADDR_SHIFT, page);
> +}
> +
> +static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
> +			      int column, int page)
> +{
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> +	nfc->column     = max(column, 0);
> +	nfc->spareonly	= 0;
> +	nfc->alt_buf	= 0;
> +
> +	switch (command) {
> +	case NAND_CMD_PAGEPROG:
> +		nfc->page = -1;
> +		vf610_nfc_send_commands(nfc, NAND_CMD_SEQIN,
> +					command, PROGRAM_PAGE_CMD_CODE);
> +		vf610_nfc_addr_cycle(nfc, column, page);
> +		break;
> +
> +	case NAND_CMD_RESET:
> +		vf610_nfc_send_command(nfc, command, RESET_CMD_CODE);
> +		break;
> +	/*
> +	 * NFC does not support sub-page reads and writes,
> +	 * so emulate them using full page transfers.
> +	 */
> +	case NAND_CMD_READOOB:
> +		nfc->spareonly = 1;
> +	case NAND_CMD_SEQIN: /* Pre-read for partial writes. */
> +	case NAND_CMD_READ0:
> +		column = 0;
> +		/* Already read? */
> +		if (nfc->page == page)
> +			return;
> +		nfc->page = page;
> +		vf610_nfc_send_commands(nfc, NAND_CMD_READ0,
> +					NAND_CMD_READSTART, READ_PAGE_CMD_CODE);
> +		vf610_nfc_addr_cycle(nfc, column, page);
> +		break;
> +
> +	case NAND_CMD_ERASE1:
> +		if (nfc->page == page)
> +			nfc->page = -1;
> +		vf610_nfc_send_commands(nfc, command,
> +				  NAND_CMD_ERASE2, ERASE_CMD_CODE);
> +		vf610_nfc_addr_cycle(nfc, column, page);
> +		break;
> +
> +	case NAND_CMD_READID:
> +		nfc->alt_buf = ALT_BUF_ID;
> +		vf610_nfc_send_command(nfc, command, READ_ID_CMD_CODE);
> +		break;
> +
> +	case NAND_CMD_STATUS:
> +		nfc->alt_buf = ALT_BUF_STAT;
> +		vf610_nfc_send_command(nfc, command, STATUS_READ_CMD_CODE);
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	vf610_nfc_done(nfc);
> +}
> +
> +static inline void vf610_nfc_read_spare(struct mtd_info *mtd, void *buf,
> +					int len)
> +{
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> +	len = min_t(uint, mtd->oobsize, len);
> +	if (len > 0)
> +		vf610_nfc_memcpy(buf, nfc->regs + mtd->writesize, len);
> +}
> +
> +static void vf610_nfc_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> +{
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +	uint c = nfc->column;
> +	uint l;
> +
> +	/* Handle main area */
> +	if (!nfc->spareonly) {
> +		l = min_t(uint, len, mtd->writesize - c);
> +		nfc->column += l;
> +
> +		if (!nfc->alt_buf)
> +			vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, l);
> +		else
> +			if (nfc->alt_buf & ALT_BUF_ID)
> +				*buf = vf610_nfc_get_id(nfc, c);
> +			else
> +				*buf = vf610_nfc_get_status(nfc);
> +
> +		buf += l;
> +		len -= l;
> +	}
> +
> +	/* Handle spare area access */
> +	if (len) {
> +		nfc->column += len;
> +		vf610_nfc_read_spare(mtd, buf, len);
> +	}
> +}
> +
> +static void vf610_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> +{
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +	uint c = nfc->column;
> +	uint l;
> +
> +	l = min_t(uint, len, mtd->writesize + mtd->oobsize - c);
> +	nfc->column += l;
> +	vf610_nfc_memcpy(nfc->regs + NFC_MAIN_AREA(0) + c, buf, l);
> +}
> +
> +static uint8_t vf610_nfc_read_byte(struct mtd_info *mtd)
> +{
> +	u8 tmp;
> +
> +	vf610_nfc_read_buf(mtd, &tmp, sizeof(tmp));
> +	return tmp;
> +}
> +
> +static u16 vf610_nfc_read_word(struct mtd_info *mtd)
> +{
> +	u16 tmp;
> +
> +	vf610_nfc_read_buf(mtd, (u_char *)&tmp, sizeof(tmp));
> +	return tmp;
> +}
> +
> +/* If not provided, upper layers apply a fixed delay. */
> +static int vf610_nfc_dev_ready(struct mtd_info *mtd)
> +{
> +	/* NFC handles R/B internally; always ready.  */
> +	return 1;
> +}
> +
> +/*
> + * This function supports Vybrid only (MPC5125 would have full RB and four CS)
> + */
> +static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
> +{
> +#ifdef CONFIG_SOC_VF610
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +	u32 tmp = vf610_nfc_read(nfc, NFC_ROW_ADDR);
> +
> +	tmp &= ~(ROW_ADDR_CHIP_SEL_RB_MASK | ROW_ADDR_CHIP_SEL_MASK);
> +	tmp |= 1 << ROW_ADDR_CHIP_SEL_RB_SHIFT;
> +
> +	if (chip == 0)
> +		tmp |= 1 << ROW_ADDR_CHIP_SEL_SHIFT;
> +	else if (chip == 1)
> +		tmp |= 2 << ROW_ADDR_CHIP_SEL_SHIFT;
> +
> +	vf610_nfc_write(nfc, NFC_ROW_ADDR, tmp);
> +#endif
> +}
> +
> +#ifdef CONFIG_OF_MTD
> +static const struct of_device_id vf610_nfc_dt_ids[] = {
> +	{ .compatible = "fsl,vf610-nfc" },
> +	{ .compatible = "fsl,mpc5125-nfc" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, vf610_nfc_dt_ids);
> +
> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
> +{
> +	struct device_node *np = dev->of_node;
> +	int buswidth;
> +	u32 clkrate;
> +
> +	if (!np)
> +		return 1;
> +
> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
> +
> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
> +		cfg->clkrate = clkrate;
> +
> +	buswidth = of_get_nand_bus_width(np);
> +	if (buswidth < 0)
> +		return buswidth;
> +
> +	cfg->width = buswidth;
> +
> +	return 0;
> +}
> +#else
> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static int vf610_nfc_init_controller(struct vf610_nfc *nfc, int page_sz)
> +{
> +	struct vf610_nfc_config *cfg = nfc->cfg;
> +
> +	if (cfg->width == 16)
> +		vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
> +	else
> +		vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
> +
> +	/* Set configuration register. */
> +	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
> +	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
> +	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
> +	vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
> +	vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
> +
> +	/* PAGE_CNT = 1 */
> +	vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
> +			    CONFIG_PAGE_CNT_SHIFT, 1);
> +
> +	/* Set ECC_STATUS offset */
> +	vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
> +			    CONFIG_ECC_SRAM_ADDR_MASK,
> +			    CONFIG_ECC_SRAM_ADDR_SHIFT, ECC_SRAM_ADDR);
> +
> +	vf610_nfc_write(nfc, NFC_SECTOR_SIZE, page_sz);
> +
> +	return 0;
> +}
> +
> +static int vf610_nfc_probe(struct platform_device *pdev)
> +{
> +	struct vf610_nfc *nfc;
> +	struct resource *res;
> +	struct mtd_info *mtd;
> +	struct nand_chip *chip;
> +	struct vf610_nfc_config *cfg;
> +	int err = 0;
> +	int page_sz;
> +	int irq;
> +
> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> +	if (!nfc)
> +		return -ENOMEM;
> +
> +	nfc->cfg = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> +	if (!nfc->cfg)
> +		return -ENOMEM;
> +	cfg = nfc->cfg;
> +
> +	nfc->dev = &pdev->dev;
> +	nfc->page = -1;
> +	mtd = &nfc->mtd;
> +	chip = &nfc->chip;
> +
> +	mtd->priv = chip;
> +	mtd->owner = THIS_MODULE;
> +	mtd->dev.parent = nfc->dev;
> +	mtd->name = DRV_NAME;
> +
> +	err = vf610_nfc_probe_dt(nfc->dev, cfg);
> +	if (err)
> +		return -ENODEV;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq <= 0)
> +		return -EINVAL;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	nfc->regs = devm_ioremap_resource(nfc->dev, res);
> +	if (IS_ERR(nfc->regs))
> +		return PTR_ERR(nfc->regs);
> +
> +	nfc->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(nfc->clk))
> +		return PTR_ERR(nfc->clk);
> +
> +	if (cfg->clkrate && clk_set_rate(nfc->clk, cfg->clkrate)) {
> +		dev_err(nfc->dev, "Clock rate not set.");
> +		return -EINVAL;
> +	}
> +
> +	err = clk_prepare_enable(nfc->clk);
> +	if (err) {
> +		dev_err(nfc->dev, "Unable to enable clock!\n");
> +		return err;
> +	}
> +
> +	if (cfg->width == 16)
> +		chip->options |= NAND_BUSWIDTH_16;
> +	else
> +		chip->options &= ~NAND_BUSWIDTH_16;
> +
> +	chip->dev_ready = vf610_nfc_dev_ready;
> +	chip->cmdfunc = vf610_nfc_command;
> +	chip->read_byte = vf610_nfc_read_byte;
> +	chip->read_word = vf610_nfc_read_word;
> +	chip->read_buf = vf610_nfc_read_buf;
> +	chip->write_buf = vf610_nfc_write_buf;
> +	chip->select_chip = vf610_nfc_select_chip;
> +
> +	/* Bad block options. */
> +	if (cfg->flash_bbt)
> +		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_CREATE;
> +
> +	chip->bbt_td = &bbt_main_descr;
> +	chip->bbt_md = &bbt_mirror_descr;
> +
> +	init_waitqueue_head(&nfc->irq_waitq);
> +
> +	err = devm_request_irq(nfc->dev, irq, vf610_nfc_irq, 0, DRV_NAME, mtd);
> +	if (err) {
> +		dev_err(nfc->dev, "Error requesting IRQ!\n");
> +		goto error;
> +	}
> +
> +	page_sz = PAGE_2K + OOB_64;
> +	page_sz += cfg->width == 16 ? 1 : 0;
> +
> +	vf610_nfc_init_controller(nfc, page_sz);
> +
> +	/* first scan to find the device and get the page size */
> +	if (nand_scan_ident(mtd, 1, NULL)) {
> +		err = -ENXIO;
> +		goto error;
> +	}
> +
> +	chip->ecc.mode = NAND_ECC_SOFT; /* default */
> +
> +	page_sz = mtd->writesize + mtd->oobsize;
> +
> +	/* Single buffer only, max 256 OOB minus ECC status */
> +	if (page_sz > PAGE_2K + 256 - 8) {
> +		dev_err(nfc->dev, "Unsupported flash size\n");
> +		err = -ENXIO;
> +		goto error;
> +	}
> +	page_sz += cfg->width == 16 ? 1 : 0;
> +	vf610_nfc_write(nfc, NFC_SECTOR_SIZE, page_sz);
> +
> +	/* second phase scan */
> +	if (nand_scan_tail(mtd)) {
> +		err = -ENXIO;
> +		goto error;
> +	}
> +
> +	/* Register device in MTD */
> +	mtd_device_parse_register(mtd, NULL,
> +		&(struct mtd_part_parser_data){
> +			.of_node = pdev->dev.of_node,
> +		},
> +		NULL, 0);
> +
> +	platform_set_drvdata(pdev, mtd);
> +
> +	return 0;
> +
> +error:
> +	clk_disable_unprepare(nfc->clk);
> +	return err;
> +}
> +
> +static int vf610_nfc_remove(struct platform_device *pdev)
> +{
> +	struct mtd_info *mtd = platform_get_drvdata(pdev);
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> +	nand_release(mtd);
> +	clk_disable_unprepare(nfc->clk);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int vf610_nfc_suspend(struct device *dev)
> +{
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> +	clk_disable_unprepare(nfc->clk);
> +	return 0;
> +}
> +
> +static int vf610_nfc_resume(struct device *dev)
> +{
> +	struct mtd_info *mtd = dev_get_drvdata(dev);
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +	int page_sz;
> +
> +	pinctrl_pm_select_default_state(dev);
> +	if (nfc->cfg->clkrate && clk_set_rate(nfc->clk, nfc->cfg->clkrate)) {
> +		dev_err(nfc->dev, "Clock rate not set.");
> +		return -EINVAL;
> +	}
> +
> +	clk_prepare_enable(nfc->clk);
> +
> +	page_sz = mtd->writesize + mtd->oobsize;
> +	page_sz += nfc->cfg->width == 16 ? 1 : 0;
> +
> +	vf610_nfc_init_controller(nfc, page_sz);
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(vf610_nfc_pm_ops, vf610_nfc_suspend, vf610_nfc_resume);
> +
> +static struct platform_driver vf610_nfc_driver = {
> +	.driver		= {
> +		.name	= DRV_NAME,
> +		.of_match_table = vf610_nfc_dt_ids,
> +		.pm	= &vf610_nfc_pm_ops,
> +	},
> +	.probe		= vf610_nfc_probe,
> +	.remove		= vf610_nfc_remove,
> +};
> +
> +module_platform_driver(vf610_nfc_driver);
> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
> +MODULE_DESCRIPTION("Freescale VF610/MPC5125 NFC MTD NAND driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.3.0
> 

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Sascha Hauer]

Hi Stefan,

On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
> +
> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
> +{
> +	struct device_node *np = dev->of_node;
> +	int buswidth;
> +	u32 clkrate;
> +
> +	if (!np)
> +		return 1;
> +
> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
> +
> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
> +		cfg->clkrate = clkrate;

Normally the clock-frequency property tells the driver at which
frequency the device actually is running, not to tell the driver at
which frequency the device *should* run. It's strange to use the value
of the clock-frequency property as input to clk_set_rate(). Maybe the
assigned clock binding is more appropriate here, see
Documentation/devicetree/bindings/clock/clock-bindings.txt.

BTW the above can easier be written as:

	of_property_read_u32(np, "clock-frequency", &cfg->clkrate);

No return value checking necessary.

> +static int vf610_nfc_probe(struct platform_device *pdev)
> +{
> +	struct vf610_nfc *nfc;
> +	struct resource *res;
> +	struct mtd_info *mtd;
> +	struct nand_chip *chip;
> +	struct vf610_nfc_config *cfg;
> +	int err = 0;
> +	int page_sz;
> +	int irq;
> +
> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> +	if (!nfc)
> +		return -ENOMEM;
> +
> +	nfc->cfg = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> +	if (!nfc->cfg)
> +		return -ENOMEM;
> +	cfg = nfc->cfg;

Why is nfc->cfg allocated separately instead of embedding it into struct
vf610_nfc? Is this some platform_data leftover you can remove now?

> +
> +	nfc->dev = &pdev->dev;
> +	nfc->page = -1;
> +	mtd = &nfc->mtd;
> +	chip = &nfc->chip;
> +
> +	mtd->priv = chip;
> +	mtd->owner = THIS_MODULE;
> +	mtd->dev.parent = nfc->dev;
> +	mtd->name = DRV_NAME;
> +
> +	err = vf610_nfc_probe_dt(nfc->dev, cfg);
> +	if (err)
> +		return -ENODEV;

Does this driver work without device tree or not? Currently the driver
bails out when device tree support is enabled but no device node is
given. When device tree support is disabled in the kernel though the
driver happily continues here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Stefan Agner]

On 2015-03-06 07:15, Sascha Hauer wrote:
> Hi Stefan,
> 
> On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>> +
>> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
>> +{
>> +	struct device_node *np = dev->of_node;
>> +	int buswidth;
>> +	u32 clkrate;
>> +
>> +	if (!np)
>> +		return 1;
>> +
>> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>> +
>> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
>> +		cfg->clkrate = clkrate;
> 
> Normally the clock-frequency property tells the driver at which
> frequency the device actually is running, not to tell the driver at
> which frequency the device *should* run. It's strange to use the value
> of the clock-frequency property as input to clk_set_rate(). Maybe the
> assigned clock binding is more appropriate here, see
> Documentation/devicetree/bindings/clock/clock-bindings.txt.

What we try to do here is to specify the hardware limitations. There
seem to be some hardware restrictions when it comes to clock
frequencies. There has been a rather long discussion over at Freescales
community about it:
https://community.freescale.com/thread/317074

Not sure if this is the right way to specify the supported frequencies,
or should we create a custom property for this, something like
fsl,max-nfc-frequency = <33000000>?


> 
> BTW the above can easier be written as:
> 
> 	of_property_read_u32(np, "clock-frequency", &cfg->clkrate);
> 
> No return value checking necessary.
> 
>> +static int vf610_nfc_probe(struct platform_device *pdev)
>> +{
>> +	struct vf610_nfc *nfc;
>> +	struct resource *res;
>> +	struct mtd_info *mtd;
>> +	struct nand_chip *chip;
>> +	struct vf610_nfc_config *cfg;
>> +	int err = 0;
>> +	int page_sz;
>> +	int irq;
>> +
>> +	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
>> +	if (!nfc)
>> +		return -ENOMEM;
>> +
>> +	nfc->cfg = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
>> +	if (!nfc->cfg)
>> +		return -ENOMEM;
>> +	cfg = nfc->cfg;
> 
> Why is nfc->cfg allocated separately instead of embedding it into struct
> vf610_nfc? Is this some platform_data leftover you can remove now?
> 

Yeah, looks like a platform_data leftover, will change that.

>> +
>> +	nfc->dev = &pdev->dev;
>> +	nfc->page = -1;
>> +	mtd = &nfc->mtd;
>> +	chip = &nfc->chip;
>> +
>> +	mtd->priv = chip;
>> +	mtd->owner = THIS_MODULE;
>> +	mtd->dev.parent = nfc->dev;
>> +	mtd->name = DRV_NAME;
>> +
>> +	err = vf610_nfc_probe_dt(nfc->dev, cfg);
>> +	if (err)
>> +		return -ENODEV;
> 
> Does this driver work without device tree or not? Currently the driver
> bails out when device tree support is enabled but no device node is
> given. When device tree support is disabled in the kernel though the
> driver happily continues here.
> 

Hm, I never tried using this Driver without DT. I guess in practice, we
won't get here in a DT enabled kernel anyway: If there is no device tree
node with one of the drivers compatibility ids, the probe function won't
be called.

Theoretically, the IP supported by this driver is part of two other
SoC's which are supported by Linux: PowerPC MPC5125 (DT) and a ColdFire
MCF54418 (non-DT). So this driver might be used on a ColdFire SoC using
the platform support. Currently, one would just not have the ability to
set the hardware ECC options... I'm a bit reluctant to implement full
platform support without being able to properly test it. If anybody is
interested in it, one could easily extend the driver...

Put Geert in CC...

--
Stefan

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Bill Pringlemeir]

On  6 Mar 2015, stefan@agner.ch wrote:

> On 2015-03-06 07:15, Sascha Hauer wrote:
>> Hi Stefan,

>> On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>>> +
>>> +static int vf610_nfc_probe_dt(struct device *dev, struct
>>> vf610_nfc_config *cfg)
>>> +{
>>> +	struct device_node *np = dev->of_node;
>>> +	int buswidth;
>>> +	u32 clkrate;
>>> +
>>> +	if (!np)
>>> +		return 1;
>>> +
>>> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>>> +
>>> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
>>> +		cfg->clkrate = clkrate;

>> Normally the clock-frequency property tells the driver at which
>> frequency the device actually is running, not to tell the driver at
>> which frequency the device *should* run. It's strange to use the
>> value of the clock-frequency property as input to
>> clk_set_rate(). Maybe the assigned clock binding is more appropriate
>> here, see Documentation/devicetree/bindings/clock/clock-bindings.txt.

> What we try to do here is to specify the hardware limitations. There
> seem to be some hardware restrictions when it comes to clock
> frequencies. There has been a rather long discussion over at
> Freescales community about it:
> https://community.freescale.com/thread/317074

> Not sure if this is the right way to specify the supported
> frequencies, or should we create a custom property for this, something
> like fsl,max-nfc-frequency = <33000000>?

On most SOC's with this controller, the input clock to the controller
affects the NAND flash timing and other factors; so you will want to set
it based on the board/NAND stuffed.  The clock is for synchronous logic
in the controller and affects many properties.  

I guess Sascha's point is, the board's DT should just have some
'&nfc_clk' node and not have this part of the driver?  Either way works.
However, this clock is very important to get the driver to function.  It
seem better for a user/porter to have this info in the node.  I guess
you need to be trained to look at every node in the sub-tree for a
device.  I think the other way might be better or a sub-system
maintainer.  I looked at 'i2c' and other node which have a
'clock-frequency' parameter. 

In the Documentation/devicetree/bindings/clock/clock-bindings.txt,

    uart@a000 {
        compatible = "fsl,imx-uart";
        reg = <0xa000 0x1000>;
        interrupts = <33>;
        clocks = <&osc 0>, <&pll 1>;
        clock-names = "baud", "register";
    };

Here, this uart clock may affect the maximum baud rate supported by the
device.  For this controller (vf610_nfc), the clock is like setting the
'baud rate'; it affect the NAND memory cycles.  There is not really any
'wait state' type logic in the controller register set that would allow
the driver to work with a 'given clock' rate.  For certain a board
should set this clock for the NAND chips they wish to support.

What would the board file look like to use clock node?

[generic]
        nfc: nand@400e0000 {
                compatible = "fsl,vf610-nfc";
                reg = <0x400e0000 0x4000>;
                clocks = <&clks VF610_CLK_NFC>;
                clock-names = "nfc_clk";
                status = "disabled";
        };

[board]

&nfc {
        nand-bus-width = <16>;
        nand-ecc-mode = "hw";
        nand-on-flash-bbt;
        nand-ecc-strength = <24>;
        nand-ecc-step-size = <2048>;
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_nfc_1>;
        status = "okay";
        &nfc_clk { frequency = <33000000>}  /* Like this? */
};

I don't know how to do the 'Like this?' part.  I don't think the
'clock-bindings.txt' explains it.  I see this is better as the the
driver needs no 'clock handling' code.  It is definitely a little more
obtuse for the users.

[snip]

>> Does this driver work without device tree or not? Currently the
>> driver bails out when device tree support is enabled but no device
>> node is given. When device tree support is disabled in the kernel
>> though the driver happily continues here.

> Hm, I never tried using this Driver without DT. 

[snip]

I also didn't test this.  The driver was ported from Linux versions
where DT did not exist.  It is used in some OpenWRT/68k/coldfire
(patched) kernels and I wanted it to be useful for them.  However, we
could probably remove the 'platform support'.  Other people are using
this on PPC platforms and they will also have dt/of.

Currently the platform control has no way to 'pass data', so the driver
only works with whatever defaults it has (or that is my belief).  For
instance, those OpenWRT kernels have a 'machine file' which will set the
'clock-frequency' and other parameters.  We could remove the platform
support completely if it is misleading.  I guess the KConfig would need
a 'depends on CONFIG_OF'.

Thanks for the review,
Bill Pringlemeir.

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Stefan Agner]

On 2015-03-06 16:32, Bill Pringlemeir wrote:
> On  6 Mar 2015, stefan@agner.ch wrote:
> 
>> On 2015-03-06 07:15, Sascha Hauer wrote:
>>> Hi Stefan,
> 
>>> On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>>>> +
>>>> +static int vf610_nfc_probe_dt(struct device *dev, struct
>>>> vf610_nfc_config *cfg)
>>>> +{
>>>> +	struct device_node *np = dev->of_node;
>>>> +	int buswidth;
>>>> +	u32 clkrate;
>>>> +
>>>> +	if (!np)
>>>> +		return 1;
>>>> +
>>>> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>>>> +
>>>> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
>>>> +		cfg->clkrate = clkrate;
> 
>>> Normally the clock-frequency property tells the driver at which
>>> frequency the device actually is running, not to tell the driver at
>>> which frequency the device *should* run. It's strange to use the
>>> value of the clock-frequency property as input to
>>> clk_set_rate(). Maybe the assigned clock binding is more appropriate
>>> here, see Documentation/devicetree/bindings/clock/clock-bindings.txt.
> 
>> What we try to do here is to specify the hardware limitations. There
>> seem to be some hardware restrictions when it comes to clock
>> frequencies. There has been a rather long discussion over at
>> Freescales community about it:
>> https://community.freescale.com/thread/317074
> 
>> Not sure if this is the right way to specify the supported
>> frequencies, or should we create a custom property for this, something
>> like fsl,max-nfc-frequency = <33000000>?
> 
> On most SOC's with this controller, the input clock to the controller
> affects the NAND flash timing and other factors; so you will want to set
> it based on the board/NAND stuffed.  The clock is for synchronous logic
> in the controller and affects many properties.  
> 
> I guess Sascha's point is, the board's DT should just have some
> '&nfc_clk' node and not have this part of the driver?  Either way works.
> However, this clock is very important to get the driver to function.  It
> seem better for a user/porter to have this info in the node.  I guess
> you need to be trained to look at every node in the sub-tree for a
> device.  I think the other way might be better or a sub-system
> maintainer.  I looked at 'i2c' and other node which have a
> 'clock-frequency' parameter. 
> 
> In the Documentation/devicetree/bindings/clock/clock-bindings.txt,
> 
>     uart@a000 {
>         compatible = "fsl,imx-uart";
>         reg = <0xa000 0x1000>;
>         interrupts = <33>;
>         clocks = <&osc 0>, <&pll 1>;
>         clock-names = "baud", "register";
>     };
> 
> Here, this uart clock may affect the maximum baud rate supported by the
> device.  For this controller (vf610_nfc), the clock is like setting the
> 'baud rate'; it affect the NAND memory cycles.  There is not really any
> 'wait state' type logic in the controller register set that would allow
> the driver to work with a 'given clock' rate.  For certain a board
> should set this clock for the NAND chips they wish to support.
> 
> What would the board file look like to use clock node?
> 
> [generic]
>         nfc: nand@400e0000 {
>                 compatible = "fsl,vf610-nfc";
>                 reg = <0x400e0000 0x4000>;
>                 clocks = <&clks VF610_CLK_NFC>;
>                 clock-names = "nfc_clk";
>                 status = "disabled";
>         };
> 
> [board]
> 
> &nfc {
>         nand-bus-width = <16>;
>         nand-ecc-mode = "hw";
>         nand-on-flash-bbt;
>         nand-ecc-strength = <24>;
>         nand-ecc-step-size = <2048>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_nfc_1>;
>         status = "okay";
>         &nfc_clk { frequency = <33000000>}  /* Like this? */

There is a property called "clock-frequency", but this is really for
static clocks (e.g. external oscillators). I don't think this is the
right approach.

> };
> 
> I don't know how to do the 'Like this?' part.  I don't think the
> 'clock-bindings.txt' explains it.  I see this is better as the the
> driver needs no 'clock handling' code.  It is definitely a little more
> obtuse for the users.

There are drivers which use the clock-frequency property to actually
specify maximum operating frequencies:

Documentation/devicetree/bindings/i2c/i2c-efm32.txt: - clock-frequency :
maximal I2C bus clock frequency in Hz.
Documentation/devicetree/bindings/media/samsung-fimc.txt:-
clock-frequency: maximum FIMC local clock (LCLK) frequency;

In the MMC subsystem there is a property called max-frequency, the SPI
subsystem uses the property spi-max-frequency to specify maximum
operating frequencies of SPI slaves.

How about just max-frequency? Or with vendor prefix, e.g.
fsl,max-frequency...?

> 
> [snip]
> 
>>> Does this driver work without device tree or not? Currently the
>>> driver bails out when device tree support is enabled but no device
>>> node is given. When device tree support is disabled in the kernel
>>> though the driver happily continues here.
> 
>> Hm, I never tried using this Driver without DT.
> 
> [snip]
> 
> I also didn't test this.  The driver was ported from Linux versions
> where DT did not exist.  It is used in some OpenWRT/68k/coldfire
> (patched) kernels and I wanted it to be useful for them.  However, we
> could probably remove the 'platform support'.  Other people are using
> this on PPC platforms and they will also have dt/of.
> 
> Currently the platform control has no way to 'pass data', so the driver
> only works with whatever defaults it has (or that is my belief).  For
> instance, those OpenWRT kernels have a 'machine file' which will set the
> 'clock-frequency' and other parameters.  We could remove the platform
> support completely if it is misleading.  I guess the KConfig would need
> a 'depends on CONFIG_OF'.
> 
> Thanks for the review,
> Bill Pringlemeir.

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Sascha Hauer]

On Fri, Mar 06, 2015 at 02:31:51PM +0100, Stefan Agner wrote:
> On 2015-03-06 07:15, Sascha Hauer wrote:
> > Hi Stefan,
> > 
> > On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
> >> +
> >> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
> >> +{
> >> +	struct device_node *np = dev->of_node;
> >> +	int buswidth;
> >> +	u32 clkrate;
> >> +
> >> +	if (!np)
> >> +		return 1;
> >> +
> >> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
> >> +
> >> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
> >> +		cfg->clkrate = clkrate;
> > 
> > Normally the clock-frequency property tells the driver at which
> > frequency the device actually is running, not to tell the driver at
> > which frequency the device *should* run. It's strange to use the value
> > of the clock-frequency property as input to clk_set_rate(). Maybe the
> > assigned clock binding is more appropriate here, see
> > Documentation/devicetree/bindings/clock/clock-bindings.txt.
> 
> What we try to do here is to specify the hardware limitations. There
> seem to be some hardware restrictions when it comes to clock
> frequencies. There has been a rather long discussion over at Freescales
> community about it:
> https://community.freescale.com/thread/317074
> 
> Not sure if this is the right way to specify the supported frequencies,
> or should we create a custom property for this, something like
> fsl,max-nfc-frequency = <33000000>?

What's wrong with the assigned clock binding? All you have to do is to
add it to the device node. The rest will be done from the generic clock
framework with no additional driver code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Stefan Agner]

On 2015-03-09 10:58, Sascha Hauer wrote:
> On Fri, Mar 06, 2015 at 02:31:51PM +0100, Stefan Agner wrote:
>> On 2015-03-06 07:15, Sascha Hauer wrote:
>> > Hi Stefan,
>> >
>> > On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>> >> +
>> >> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
>> >> +{
>> >> +	struct device_node *np = dev->of_node;
>> >> +	int buswidth;
>> >> +	u32 clkrate;
>> >> +
>> >> +	if (!np)
>> >> +		return 1;
>> >> +
>> >> +	cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>> >> +
>> >> +	if (!of_property_read_u32(np, "clock-frequency", &clkrate))
>> >> +		cfg->clkrate = clkrate;
>> >
>> > Normally the clock-frequency property tells the driver at which
>> > frequency the device actually is running, not to tell the driver at
>> > which frequency the device *should* run. It's strange to use the value
>> > of the clock-frequency property as input to clk_set_rate(). Maybe the
>> > assigned clock binding is more appropriate here, see
>> > Documentation/devicetree/bindings/clock/clock-bindings.txt.
>>
>> What we try to do here is to specify the hardware limitations. There
>> seem to be some hardware restrictions when it comes to clock
>> frequencies. There has been a rather long discussion over at Freescales
>> community about it:
>> https://community.freescale.com/thread/317074
>>
>> Not sure if this is the right way to specify the supported frequencies,
>> or should we create a custom property for this, something like
>> fsl,max-nfc-frequency = <33000000>?
> 
> What's wrong with the assigned clock binding? All you have to do is to
> add it to the device node. The rest will be done from the generic clock
> framework with no additional driver code.

Ah yes, assigned clock bindings seems like a match. So, the NFC node
would look something like

...
        assigned-clocks = <&clks VF610_CLK_NFC>;
        assigned-clock-rates = <33000000>;
...

Will try to use that, thx.

--
Stefan