[PATCH v6 0/2] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc

[PATCH v6 0/2] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc

[Zhou Wang]

This is a repeated version patchset with E-mail address changed in signed-off-by.

This patchset adds the support for NAND controller of hisilicon hip04 Soc.
The NAND controller IP was developed by hisilicon and needs a new driver to
support it. This patchset is based on l2-mtd/master.

I have tested this NAND flash controller driver in Hip04 D01 board using MTD
test modules. All testes passed except mtd_nandbiterrs. The reason is that this
NAND controller doesn't support to write raw page in ECC mode. In the test of
mtd_nandbiterrs, when rewriting raw page(with some bits changed) back, this NAND
controller will also produce ECC codes and write into NAND flash OOB area. As
ECC codes in OOB area have been broken, read error comes out as follow.

Changes in v6:
- Change E-mail address in signed-off-by to "wangzhou1@hisilicon.com"

Changes in v5:
- Make modifications according to the comments from Brian, thanks a lot.
- Add hisi_nand_read_oob callback function.

Changes in v4:
- Add mtd->dev.parent = &pdev->dev, thanks Frans Klaver.  

Changes in v3:
- Modify code to eliminate some code style warnings.
- Add ecc-bits input check.
- Avoid using waterfall style in hisi_nfc_cmdfunc().

Changes in v2:
- Remove the patch for device tree, now patchset only has the driver and its

device tree binding documentation.
- Change the file name: hisi_nand.c to hisi504_nand.c.

Changes in v1:
- Remove callback functions out of struct hinfc_host, and call them directly
  in relative functions.
- Change hinfc_read and hinfc_write from macros to inline functions.
- Instead of putting pointers, embed struct nand_chip and struct mtd_info
  in struct hinfc_host directly.
- rewrite some unclear lines in device tree binding document, correct
  some code style error.

Link on v4:
- https://lkml.org/lkml/2014/11/4/377
Link on v3:
- https://lkml.org/lkml/2014/10/28/386
Link on v2:
- https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg750071.html
Link on v1:
- https://lkml.org/lkml/2014/7/15/198

Zhou Wang (2):
  mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc
  mtd: hisilicon: add device tree binding documentation

 .../devicetree/bindings/mtd/hisi504-nand.txt       |  48 ++
 drivers/mtd/nand/Kconfig                           |   5 +
 drivers/mtd/nand/Makefile                          |   1 +
 drivers/mtd/nand/hisi504_nand.c                    | 907 +++++++++++++++++++++
 4 files changed, 961 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/hisi504-nand.txt
 create mode 100644 drivers/mtd/nand/hisi504_nand.c

-- 
1.9.1

[PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Zhou Wang]

Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 drivers/mtd/nand/Kconfig        |   5 +
 drivers/mtd/nand/Makefile       |   1 +
 drivers/mtd/nand/hisi504_nand.c | 907 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 913 insertions(+)
 create mode 100644 drivers/mtd/nand/hisi504_nand.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7d0150d..e1220fc 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -524,4 +524,9 @@ config MTD_NAND_SUNXI
 	help
 	  Enables support for NAND Flash chips on Allwinner SoCs.
 
+config MTD_NAND_HISI504
+	tristate "Support for NAND controller on Hisilicon SoC Hip04"
+	help
+	  Enables support for NAND controller on Hisilicon SoC Hip04.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index bd38f21..582bbd05 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -51,5 +51,6 @@ obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
 obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
 obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
+obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/hisi504_nand.c b/drivers/mtd/nand/hisi504_nand.c
new file mode 100644
index 0000000..2000f21
--- /dev/null
+++ b/drivers/mtd/nand/hisi504_nand.c
@@ -0,0 +1,907 @@
+/*
+ * Hisilicon NAND Flash controller driver
+ *
+ * Copyright © 2012-2014 HiSilicon Technologies Co., Ltd.
+ *              http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ * The initial developer of the original code is Zhiyong Cai
+ * <caizhiyong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/of.h>
+#include <linux/of_mtd.h>
+#include <linux/mtd/mtd.h>
+#include <linux/sizes.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/mtd/nand.h>
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+#include <linux/mtd/partitions.h>
+
+#define HINFC504_MAX_CHIP                               (4)
+#define HINFC504_W_LATCH                                (5)
+#define HINFC504_R_LATCH                                (7)
+#define HINFC504_RW_LATCH                               (3)
+
+#define HINFC504_NFC_TIMEOUT				(2 * HZ)
+#define HINFC504_NFC_PM_TIMEOUT				(1 * HZ)
+#define HINFC504_NFC_DMA_TIMEOUT			(5 * HZ)
+#define HINFC504_CHIP_DELAY				(25)
+
+#define HINFC504_REG_BASE_ADDRESS_LEN			(0x100)
+#define HINFC504_BUFFER_BASE_ADDRESS_LEN		(2048 + 128)
+
+#define HINFC504_ADDR_CYCLE_MASK			0x4
+
+#define HINFC504_CON					0x00
+#define HINFC504_CON_OP_MODE_NORMAL			BIT(0)
+#define HINFC504_CON_PAGEISZE_SHIFT			(1)
+#define HINFC504_CON_PAGESIZE_MASK			(0x07)
+#define HINFC504_CON_BUS_WIDTH				BIT(4)
+#define HINFC504_CON_READY_BUSY_SEL			BIT(8)
+#define HINFC504_CON_ECCTYPE_SHIFT			(9)
+#define HINFC504_CON_ECCTYPE_MASK			(0x07)
+
+#define HINFC504_PWIDTH					0x04
+#define SET_HINFC504_PWIDTH(_w_lcnt, _r_lcnt, _rw_hcnt) \
+	((_w_lcnt) | (((_r_lcnt) & 0x0F) << 4) | (((_rw_hcnt) & 0x0F) << 8))
+
+#define HINFC504_CMD					0x0C
+#define HINFC504_ADDRL					0x10
+#define HINFC504_ADDRH					0x14
+#define HINFC504_DATA_NUM				0x18
+
+#define HINFC504_OP					0x1C
+#define HINFC504_OP_READ_DATA_EN			BIT(1)
+#define HINFC504_OP_WAIT_READY_EN			BIT(2)
+#define HINFC504_OP_CMD2_EN				BIT(3)
+#define HINFC504_OP_WRITE_DATA_EN			BIT(4)
+#define HINFC504_OP_ADDR_EN				BIT(5)
+#define HINFC504_OP_CMD1_EN				BIT(6)
+#define HINFC504_OP_NF_CS_SHIFT                         (7)
+#define HINFC504_OP_NF_CS_MASK				(3)
+#define HINFC504_OP_ADDR_CYCLE_SHIFT			(9)
+#define HINFC504_OP_ADDR_CYCLE_MASK			(7)
+
+#define HINFC504_STATUS                                 0x20
+#define HINFC504_READY					BIT(0)
+
+#define HINFC504_INTEN					0x24
+#define HINFC504_INTEN_DMA				BIT(9)
+#define HINFC504_INTEN_UE				BIT(6)
+#define HINFC504_INTEN_CE				BIT(5)
+
+#define HINFC504_INTS					0x28
+#define HINFC504_INTS_DMA				BIT(9)
+#define HINFC504_INTS_UE				BIT(6)
+#define HINFC504_INTS_CE				BIT(5)
+
+#define HINFC504_INTCLR                                 0x2C
+#define HINFC504_INTCLR_DMA				BIT(9)
+#define HINFC504_INTCLR_UE				BIT(6)
+#define HINFC504_INTCLR_CE				BIT(5)
+
+#define HINFC504_ECC_STATUS                             0x5C
+#define HINFC504_ECC_1_BIT_SHIFT                        16
+#define HINFC504_ECC_16_BIT_SHIFT                       12
+
+#define HINFC504_DMA_CTRL				0x60
+#define HINFC504_DMA_CTRL_DMA_START			BIT(0)
+#define HINFC504_DMA_CTRL_WE				BIT(1)
+#define HINFC504_DMA_CTRL_DATA_AREA_EN			BIT(2)
+#define HINFC504_DMA_CTRL_OOB_AREA_EN			BIT(3)
+#define HINFC504_DMA_CTRL_BURST4_EN			BIT(4)
+#define HINFC504_DMA_CTRL_BURST8_EN			BIT(5)
+#define HINFC504_DMA_CTRL_BURST16_EN			BIT(6)
+#define HINFC504_DMA_CTRL_ADDR_NUM_SHIFT		(7)
+#define HINFC504_DMA_CTRL_ADDR_NUM_MASK                 (1)
+#define HINFC504_DMA_CTRL_CS_SHIFT			(8)
+#define HINFC504_DMA_CTRL_CS_MASK			(0x03)
+
+#define HINFC504_DMA_ADDR_DATA				0x64
+#define HINFC504_DMA_ADDR_OOB				0x68
+
+#define HINFC504_DMA_LEN				0x6C
+#define HINFC504_DMA_LEN_OOB_SHIFT			(16)
+#define HINFC504_DMA_LEN_OOB_MASK			(0xFFF)
+
+#define HINFC504_DMA_PARA				0x70
+#define HINFC504_DMA_PARA_DATA_RW_EN			BIT(0)
+#define HINFC504_DMA_PARA_OOB_RW_EN			BIT(1)
+#define HINFC504_DMA_PARA_DATA_EDC_EN			BIT(2)
+#define HINFC504_DMA_PARA_OOB_EDC_EN			BIT(3)
+#define HINFC504_DMA_PARA_DATA_ECC_EN			BIT(4)
+#define HINFC504_DMA_PARA_OOB_ECC_EN			BIT(5)
+
+#define HINFC_VERSION                                   0x74
+#define HINFC504_LOG_READ_ADDR				0x7C
+#define HINFC504_LOG_READ_LEN				0x80
+
+#define HINFC504_NANDINFO_LEN				0x10
+
+struct hinfc_host {
+	struct nand_chip	chip;
+	struct mtd_info		mtd;
+	struct device		*dev;
+	void __iomem		*iobase;
+	struct completion       cmd_complete;
+	unsigned int		offset;
+	unsigned int		command;
+	int			chipselect;
+	unsigned int		addr_cycle;
+	u32                     addr_value[2];
+	u32                     cache_addr_value[2];
+	char			*buffer;
+	dma_addr_t		dma_buffer;
+	dma_addr_t		dma_oob;
+	int			version;
+	unsigned int            irq_status; /* interrupt status */
+};
+
+static inline unsigned int hinfc_read(struct hinfc_host *host, unsigned int reg)
+{
+	return readl(host->iobase + reg);
+}
+
+static inline void hinfc_write(struct hinfc_host *host, unsigned int value,
+			       unsigned int reg)
+{
+	writel(value, host->iobase + reg);
+}
+
+static void wait_controller_finished(struct hinfc_host *host)
+{
+	unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT;
+	int val;
+
+	while (time_before(jiffies, timeout)) {
+		val = hinfc_read(host, HINFC504_STATUS);
+		if (host->command == NAND_CMD_ERASE2) {
+			/* nfc is ready */
+			while (!(val & HINFC504_READY))	{
+				usleep_range(500, 1000);
+				val = hinfc_read(host, HINFC504_STATUS);
+			}
+			return;
+		}
+
+		if (val & HINFC504_READY)
+			return;
+	}
+
+	/* wait cmd timeout */
+	dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n");
+}
+
+static void hisi_nfc_dma_transfer(struct hinfc_host *host, int todev)
+{
+	struct mtd_info	*mtd = &host->mtd;
+	struct nand_chip *chip = mtd->priv;
+	unsigned long val;
+	int ret;
+
+	hinfc_write(host, host->dma_buffer, HINFC504_DMA_ADDR_DATA);
+	hinfc_write(host, host->dma_oob, HINFC504_DMA_ADDR_OOB);
+
+	if (chip->ecc.mode == NAND_ECC_NONE) {
+		hinfc_write(host, ((mtd->oobsize & HINFC504_DMA_LEN_OOB_MASK)
+			<< HINFC504_DMA_LEN_OOB_SHIFT), HINFC504_DMA_LEN);
+
+		hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN
+			| HINFC504_DMA_PARA_OOB_RW_EN, HINFC504_DMA_PARA);
+	} else {
+		if (host->command == NAND_CMD_READOOB)
+			hinfc_write(host, HINFC504_DMA_PARA_OOB_RW_EN
+			| HINFC504_DMA_PARA_OOB_EDC_EN
+			| HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA);
+		else
+			hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN
+			| HINFC504_DMA_PARA_OOB_RW_EN
+			| HINFC504_DMA_PARA_DATA_EDC_EN
+			| HINFC504_DMA_PARA_OOB_EDC_EN
+			| HINFC504_DMA_PARA_DATA_ECC_EN
+			| HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA);
+
+	}
+
+	val = (HINFC504_DMA_CTRL_DMA_START | HINFC504_DMA_CTRL_BURST4_EN
+		| HINFC504_DMA_CTRL_BURST8_EN | HINFC504_DMA_CTRL_BURST16_EN
+		| HINFC504_DMA_CTRL_DATA_AREA_EN | HINFC504_DMA_CTRL_OOB_AREA_EN
+		| ((host->addr_cycle == 4 ? 1 : 0)
+			<< HINFC504_DMA_CTRL_ADDR_NUM_SHIFT)
+		| ((host->chipselect & HINFC504_DMA_CTRL_CS_MASK)
+			<< HINFC504_DMA_CTRL_CS_SHIFT));
+
+	if (todev)
+		val |= HINFC504_DMA_CTRL_WE;
+
+	init_completion(&host->cmd_complete);
+
+	hinfc_write(host, val, HINFC504_DMA_CTRL);
+	ret = wait_for_completion_timeout(&host->cmd_complete,
+			HINFC504_NFC_DMA_TIMEOUT);
+
+	if (!ret) {
+		dev_err(host->dev, "DMA operation(irq) timeout!\n");
+		/* sanity check */
+		val = hinfc_read(host, HINFC504_DMA_CTRL);
+		if (!(val & HINFC504_DMA_CTRL_DMA_START))
+			dev_err(host->dev, "DMA is already done but without irq ACK!\n");
+		else
+			dev_err(host->dev, "DMA is really timeout!\n");
+	}
+}
+
+static int hisi_nfc_send_cmd_pageprog(struct hinfc_host *host)
+{
+	host->addr_value[0] &= 0xffff0000;
+
+	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
+	hinfc_write(host, host->addr_value[1], HINFC504_ADDRH);
+	hinfc_write(host, NAND_CMD_PAGEPROG << 8 | NAND_CMD_SEQIN,
+		    HINFC504_CMD);
+
+	hisi_nfc_dma_transfer(host, 1);
+
+	return 0;
+}
+
+static int hisi_nfc_send_cmd_readstart(struct hinfc_host *host)
+{
+	struct mtd_info	*mtd = &host->mtd;
+
+	if ((host->addr_value[0] == host->cache_addr_value[0]) &&
+	    (host->addr_value[1] == host->cache_addr_value[1]))
+		return 0;
+
+	host->addr_value[0] &= 0xffff0000;
+
+	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
+	hinfc_write(host, host->addr_value[1], HINFC504_ADDRH);
+	hinfc_write(host, NAND_CMD_READSTART << 8 | NAND_CMD_READ0,
+		    HINFC504_CMD);
+
+	hinfc_write(host, 0, HINFC504_LOG_READ_ADDR);
+	hinfc_write(host, mtd->writesize + mtd->oobsize,
+		    HINFC504_LOG_READ_LEN);
+
+	hisi_nfc_dma_transfer(host, 0);
+
+	host->cache_addr_value[0] = host->addr_value[0];
+	host->cache_addr_value[1] = host->addr_value[1];
+
+	return 0;
+}
+
+static int hisi_nfc_send_cmd_erase(struct hinfc_host *host)
+{
+	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
+	hinfc_write(host, (NAND_CMD_ERASE2 << 8) | NAND_CMD_ERASE1,
+		    HINFC504_CMD);
+
+	hinfc_write(host, HINFC504_OP_WAIT_READY_EN
+		| HINFC504_OP_CMD2_EN
+		| HINFC504_OP_CMD1_EN
+		| HINFC504_OP_ADDR_EN
+		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
+			<< HINFC504_OP_NF_CS_SHIFT)
+		| ((host->addr_cycle & HINFC504_OP_ADDR_CYCLE_MASK)
+			<< HINFC504_OP_ADDR_CYCLE_SHIFT),
+		HINFC504_OP);
+
+	wait_controller_finished(host);
+
+	return 0;
+}
+
+static int hisi_nfc_send_cmd_readid(struct hinfc_host *host)
+{
+	hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM);
+	hinfc_write(host, NAND_CMD_READID, HINFC504_CMD);
+	hinfc_write(host, 0, HINFC504_ADDRL);
+
+	hinfc_write(host, HINFC504_OP_CMD1_EN | HINFC504_OP_ADDR_EN
+		| HINFC504_OP_READ_DATA_EN
+		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
+			<< HINFC504_OP_NF_CS_SHIFT)
+		| 1 << HINFC504_OP_ADDR_CYCLE_SHIFT, HINFC504_OP);
+
+	wait_controller_finished(host);
+
+	return 0;
+}
+
+static int hisi_nfc_send_cmd_status(struct hinfc_host *host)
+{
+	hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM);
+	hinfc_write(host, NAND_CMD_STATUS, HINFC504_CMD);
+	hinfc_write(host, HINFC504_OP_CMD1_EN
+		| HINFC504_OP_READ_DATA_EN
+		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
+			<< HINFC504_OP_NF_CS_SHIFT),
+		HINFC504_OP);
+
+	wait_controller_finished(host);
+
+	return 0;
+}
+
+static int hisi_nfc_send_cmd_reset(struct hinfc_host *host, int chipselect)
+{
+	hinfc_write(host, NAND_CMD_RESET, HINFC504_CMD);
+
+	hinfc_write(host, HINFC504_OP_CMD1_EN
+		| ((chipselect & HINFC504_OP_NF_CS_MASK)
+			<< HINFC504_OP_NF_CS_SHIFT)
+		| HINFC504_OP_WAIT_READY_EN,
+		HINFC504_OP);
+
+	wait_controller_finished(host);
+
+	return 0;
+}
+
+static void hisi_nfc_select_chip(struct mtd_info *mtd, int chipselect)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+
+	if (chipselect < 0)
+		return;
+
+	host->chipselect = chipselect;
+}
+
+static uint8_t hisi_nfc_read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+
+	if (host->command == NAND_CMD_STATUS)
+		return readb(chip->IO_ADDR_R);
+
+	host->offset++;
+
+	if (host->command == NAND_CMD_READID)
+		return readb(chip->IO_ADDR_R + host->offset - 1);
+
+	return readb(host->buffer + host->offset - 1);
+}
+
+static u16 hisi_nfc_read_word(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+
+	host->offset += 2;
+	return readw(host->buffer + host->offset - 2);
+}
+
+static void
+hisi_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+
+	memcpy(host->buffer + host->offset, buf, len);
+	host->offset += len;
+}
+
+static void hisi_nfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+
+	memcpy(buf, host->buffer + host->offset, len);
+	host->offset += len;
+}
+
+static void set_addr(struct mtd_info *mtd, int column, int page_addr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+	unsigned int command = host->command;
+
+	host->addr_cycle    = 0;
+	host->addr_value[0] = 0;
+	host->addr_value[1] = 0;
+
+	/* Serially input address */
+	if (column != -1) {
+		/* Adjust columns for 16 bit buswidth */
+		if (chip->options & NAND_BUSWIDTH_16 &&
+				!nand_opcode_8bits(command))
+			column >>= 1;
+
+		host->addr_value[0] = column & 0xffff;
+		host->addr_cycle    = 2;
+	}
+	if (page_addr != -1) {
+		host->addr_value[0] |= (page_addr & 0xffff)
+			<< (host->addr_cycle * 8);
+		host->addr_cycle    += 2;
+		/* One more address cycle for devices > 128MiB */
+		if (chip->chipsize > (128 << 20)) {
+			host->addr_cycle += 1;
+			if (host->command == NAND_CMD_ERASE1)
+				host->addr_value[0] |= ((page_addr >> 16) & 0xff) << 16;
+			else
+				host->addr_value[1] |= ((page_addr >> 16) & 0xff);
+		}
+	}
+}
+
+static void hisi_nfc_cmdfunc(struct mtd_info *mtd, unsigned command, int column,
+		int page_addr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct hinfc_host *host = chip->priv;
+	int is_cache_invalid = 1;
+	unsigned int flag = 0;
+
+	host->command =  command;
+
+	switch (command) {
+	case NAND_CMD_READ0:
+	case NAND_CMD_READOOB:
+		if (command == NAND_CMD_READ0)
+			host->offset = column;
+		else
+			host->offset = column + mtd->writesize;
+
+		is_cache_invalid = 0;
+		set_addr(mtd, column, page_addr);
+		hisi_nfc_send_cmd_readstart(host);
+		break;
+
+	case NAND_CMD_SEQIN:
+		host->offset = column;
+		set_addr(mtd, column, page_addr);
+		break;
+
+	case NAND_CMD_ERASE1:
+		set_addr(mtd, column, page_addr);
+		break;
+
+	case NAND_CMD_PAGEPROG:
+		hisi_nfc_send_cmd_pageprog(host);
+		break;
+
+	case NAND_CMD_ERASE2:
+		hisi_nfc_send_cmd_erase(host);
+		break;
+
+	case NAND_CMD_READID:
+		host->offset = column;
+		memset(chip->IO_ADDR_R, 0, 0x10);
+		hisi_nfc_send_cmd_readid(host);
+		break;
+
+	case NAND_CMD_STATUS:
+		flag = hinfc_read(host, HINFC504_CON);
+		if (chip->ecc.mode == NAND_ECC_HW)
+			hinfc_write(host,
+				    flag && ~(HINFC504_CON_ECCTYPE_MASK <<
+				    HINFC504_CON_ECCTYPE_SHIFT), HINFC504_CON);
+
+		host->offset = 0;
+		memset(chip->IO_ADDR_R, 0, 0x10);
+		hisi_nfc_send_cmd_status(host);
+		hinfc_write(host, flag, HINFC504_CON);
+		break;
+
+	case NAND_CMD_RESET:
+		hisi_nfc_send_cmd_reset(host, host->chipselect);
+		break;
+
+	default:
+		dev_err(host->dev, "Error: unsupported cmd(cmd=%x, col=%x, page=%x)\n",
+			command, column, page_addr);
+	}
+
+	if (is_cache_invalid) {
+		host->cache_addr_value[0] = ~0;
+		host->cache_addr_value[1] = ~0;
+	}
+}
+
+static irqreturn_t hinfc_irq_handle(int irq, void *devid)
+{
+	struct hinfc_host *host = devid;
+	unsigned int flag;
+
+	flag = hinfc_read(host, HINFC504_INTS);
+	/* store interrupts state */
+	host->irq_status |= flag;
+
+	if (flag & HINFC504_INTS_DMA) {
+		hinfc_write(host, HINFC504_INTCLR_DMA, HINFC504_INTCLR);
+		complete(&host->cmd_complete);
+	} else if (flag & HINFC504_INTS_CE) {
+		hinfc_write(host, HINFC504_INTCLR_CE, HINFC504_INTCLR);
+	} else if (flag & HINFC504_INTS_UE) {
+		hinfc_write(host, HINFC504_INTCLR_UE, HINFC504_INTCLR);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int hisi_nand_read_page_hwecc(struct mtd_info *mtd,
+	struct nand_chip *chip, uint8_t *buf, int oob_required, int page)
+{
+	struct hinfc_host *host = chip->priv;
+	int max_bitflips = 0, stat = 0, stat_max, status_ecc;
+	int stat_1, stat_2;
+
+	chip->read_buf(mtd, buf, mtd->writesize);
+	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	/* errors which can not be corrected by ECC */
+	if (host->irq_status & HINFC504_INTS_UE) {
+		mtd->ecc_stats.failed++;
+	} else if (host->irq_status & HINFC504_INTS_CE) {
+		/* TODO: need add other ECC modes! */
+		switch (chip->ecc.strength) {
+		case 1:
+			stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>>
+						HINFC504_ECC_1_BIT_SHIFT);
+			stat_max = 1;
+			break;
+		case 16:
+			status_ecc = hinfc_read(host, HINFC504_ECC_STATUS) >>
+					HINFC504_ECC_16_BIT_SHIFT & 0x0fff;
+			stat_2 = status_ecc & 0x3f;
+			stat_1 = status_ecc >> 6 & 0x3f;
+			stat = stat_1 + stat_2;
+			stat_max = max_t(int, stat_1, stat_2);
+		}
+		mtd->ecc_stats.corrected += stat;
+		max_bitflips = max_t(int, max_bitflips, stat_max);
+	}
+	host->irq_status = 0;
+
+	return max_bitflips;
+}
+
+static int hisi_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
+				int page)
+{
+	struct hinfc_host *host = chip->priv;
+
+	chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
+	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	if (host->irq_status & HINFC504_INTS_UE) {
+		host->irq_status = 0;
+		return -EBADMSG;
+	}
+
+	host->irq_status = 0;
+	return 0;
+}
+
+static int hisi_nand_write_page_hwecc(struct mtd_info *mtd,
+		struct nand_chip *chip, const uint8_t *buf, int oob_required)
+{
+	chip->write_buf(mtd, buf, mtd->writesize);
+	if (oob_required)
+		chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	return 0;
+}
+
+static void hisi_nfc_host_init(struct hinfc_host *host)
+{
+	struct nand_chip *chip = &host->chip;
+	unsigned int flag = 0;
+
+	host->version = hinfc_read(host, HINFC_VERSION);
+	host->addr_cycle		= 0;
+	host->addr_value[0]		= 0;
+	host->addr_value[1]		= 0;
+	host->cache_addr_value[0]	= ~0;
+	host->cache_addr_value[1]	= ~0;
+	host->chipselect		= 0;
+
+	/* default page size: 2K, ecc_none. need modify */
+	flag = HINFC504_CON_OP_MODE_NORMAL | HINFC504_CON_READY_BUSY_SEL
+		| ((0x001 & HINFC504_CON_PAGESIZE_MASK)
+			<< HINFC504_CON_PAGEISZE_SHIFT)
+		| ((0x0 & HINFC504_CON_ECCTYPE_MASK)
+			<< HINFC504_CON_ECCTYPE_SHIFT)
+		| ((chip->options & NAND_BUSWIDTH_16) ?
+			HINFC504_CON_BUS_WIDTH : 0);
+	hinfc_write(host, flag, HINFC504_CON);
+
+	memset(chip->IO_ADDR_R, 0xff, HINFC504_BUFFER_BASE_ADDRESS_LEN);
+
+	hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH,
+		    HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH);
+
+	/* enable DMA irq */
+	hinfc_write(host, HINFC504_INTEN_DMA, HINFC504_INTEN);
+}
+
+static struct nand_ecclayout nand_ecc_2K_1bit = {
+	.oobfree = { {24, 40} }
+};
+
+static struct nand_ecclayout nand_ecc_2K_16bits = {
+	.oobavail = 6,
+	.oobfree = { {2, 6} },
+};
+
+static int hisi_nfc_ecc_probe(struct hinfc_host *host)
+{
+	unsigned int flag;
+	int size, strength, ecc_bits;
+	struct device *dev = host->dev;
+	struct nand_chip *chip = &host->chip;
+	struct mtd_info *mtd = &host->mtd;
+	struct device_node *np = host->dev->of_node;
+
+	size = of_get_nand_ecc_step_size(np);
+	strength = of_get_nand_ecc_strength(np);
+	if ((size != 512) && (size != 1024)) {
+		dev_err(dev, "error ecc size: %d\n", size);
+		return -EINVAL;
+	}
+
+	if ((size == 512) && (strength != 1)) {
+		dev_err(dev, "ecc size and strength do not match\n");
+		return -EINVAL;
+	}
+	if ((size == 1024) && ((strength != 8) && (strength != 16) &&
+				(strength != 24) && (strength != 40))) {
+		dev_err(dev, "ecc size and strength do not match\n");
+		return -EINVAL;
+	}
+
+	chip->ecc.size = size;
+	chip->ecc.strength = strength;
+
+	chip->ecc.read_page = hisi_nand_read_page_hwecc;
+	chip->ecc.read_oob = hisi_nand_read_oob;
+	chip->ecc.write_page = hisi_nand_write_page_hwecc;
+
+	switch (chip->ecc.strength) {
+	case 1:
+		ecc_bits = 1;
+		if (mtd->writesize == 2048)
+			chip->ecc.layout = &nand_ecc_2K_1bit;
+
+		/* TODO: add more page size support */
+		break;
+	case 16:
+		ecc_bits = 6;
+		if (mtd->writesize == 2048)
+			chip->ecc.layout = &nand_ecc_2K_16bits;
+
+		/* TODO: add more page size support */
+		break;
+
+	/* TODO: add more ecc strength support */
+	default:
+		dev_err(dev, "not support strength: %d\n", chip->ecc.strength);
+		return -EINVAL;
+	}
+
+	flag = hinfc_read(host, HINFC504_CON);
+	/* add ecc type configure */
+	flag |= ((ecc_bits & HINFC504_CON_ECCTYPE_MASK)
+						<< HINFC504_CON_ECCTYPE_SHIFT);
+	hinfc_write(host, flag, HINFC504_CON);
+
+	/* enable ecc irq */
+	flag = hinfc_read(host, HINFC504_INTEN) & 0xfff;
+	hinfc_write(host, flag | HINFC504_INTEN_UE | HINFC504_INTEN_CE,
+		    HINFC504_INTEN);
+
+	return 0;
+}
+
+static int hisi_nfc_probe(struct platform_device *pdev)
+{
+	int ret = 0, irq, buswidth, flag, max_chips = HINFC504_MAX_CHIP;
+	struct device *dev = &pdev->dev;
+	struct hinfc_host *host;
+	struct nand_chip  *chip;
+	struct mtd_info   *mtd;
+	struct resource	  *res;
+	struct device_node *np = dev->of_node;
+	struct mtd_part_parser_data ppdata;
+
+	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
+	if (!host)
+		return -ENOMEM;
+	host->dev = dev;
+
+	platform_set_drvdata(pdev, host);
+	chip = &host->chip;
+	mtd  = &host->mtd;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "no IRQ resource defined\n");
+		ret = -ENXIO;
+		goto err_res;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	host->iobase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(host->iobase)) {
+		ret = PTR_ERR(host->iobase);
+		dev_err(dev, "devm_ioremap_resource[0] fail\n");
+		goto err_res;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res);
+	if (IS_ERR(chip->IO_ADDR_R)) {
+		ret = PTR_ERR(chip->IO_ADDR_R);
+		dev_err(dev, "devm_ioremap_resource[1] fail\n");
+		goto err_res;
+	}
+
+	mtd->priv		= chip;
+	mtd->owner		= THIS_MODULE;
+	mtd->name		= "hisi_nand";
+	mtd->dev.parent         = &pdev->dev;
+
+	chip->priv		= host;
+	chip->cmdfunc		= hisi_nfc_cmdfunc;
+	chip->select_chip	= hisi_nfc_select_chip;
+	chip->read_byte		= hisi_nfc_read_byte;
+	chip->read_word		= hisi_nfc_read_word;
+	chip->write_buf		= hisi_nfc_write_buf;
+	chip->read_buf		= hisi_nfc_read_buf;
+	chip->chip_delay	= HINFC504_CHIP_DELAY;
+
+	chip->ecc.mode = of_get_nand_ecc_mode(np);
+
+	buswidth = of_get_nand_bus_width(np);
+	if (buswidth == 16)
+		chip->options |= NAND_BUSWIDTH_16;
+
+	hisi_nfc_host_init(host);
+
+	ret = devm_request_irq(dev, irq, hinfc_irq_handle, IRQF_DISABLED,
+				"nandc", host);
+	if (ret) {
+		dev_err(dev, "failed to request IRQ\n");
+		goto err_res;
+	}
+
+	ret = nand_scan_ident(mtd, max_chips, NULL);
+	if (ret) {
+		ret = -ENODEV;
+		goto err_res;
+	}
+
+	host->buffer = dmam_alloc_coherent(dev, mtd->writesize + mtd->oobsize,
+		&host->dma_buffer, GFP_KERNEL);
+	host->dma_oob = host->dma_buffer + mtd->writesize;
+	memset(host->buffer, 0xff, mtd->writesize + mtd->oobsize);
+
+	flag = hinfc_read(host, HINFC504_CON);
+	flag &= ~(HINFC504_CON_PAGESIZE_MASK << HINFC504_CON_PAGEISZE_SHIFT);
+	switch (mtd->writesize) {
+	case 2048:
+		flag |= (0x001 << HINFC504_CON_PAGEISZE_SHIFT); break;
+	/*
+	 * TODO: add more pagesize support,
+	 * default pagesize has been set in hisi_nfc_host_init
+	 */
+	default:
+		dev_err(dev, "NON-2KB page size nand flash\n");
+		ret = -EINVAL;
+		goto err_res;
+	}
+	hinfc_write(host, flag, HINFC504_CON);
+
+	if (chip->ecc.mode == NAND_ECC_HW)
+		hisi_nfc_ecc_probe(host);
+
+	ret = nand_scan_tail(mtd);
+	if (ret) {
+		dev_err(dev, "nand_scan_tail failed: %d\n", ret);
+		goto err_res;
+	}
+
+	ppdata.of_node = np;
+	ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+	if (ret) {
+		dev_err(dev, "Err MTD partition=%d\n", ret);
+		goto err_mtd;
+	}
+
+	return 0;
+
+err_mtd:
+	nand_release(mtd);
+err_res:
+	return ret;
+}
+
+static int hisi_nfc_remove(struct platform_device *pdev)
+{
+	struct hinfc_host *host = platform_get_drvdata(pdev);
+	struct mtd_info *mtd = &host->mtd;
+
+	nand_release(mtd);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int hisi_nfc_suspend(struct device *dev)
+{
+	struct hinfc_host *host = dev_get_drvdata(dev);
+	unsigned long timeout = jiffies + HINFC504_NFC_PM_TIMEOUT;
+
+	while (time_before(jiffies, timeout)) {
+		if (((hinfc_read(host, HINFC504_STATUS) & 0x1) == 0x0) &&
+		    (hinfc_read(host, HINFC504_DMA_CTRL) &
+		     HINFC504_DMA_CTRL_DMA_START)) {
+			_cond_resched();
+			return 0;
+		}
+	}
+
+	dev_err(host->dev, "nand controller suspend timeout.\n");
+
+	return -EAGAIN;
+}
+
+static int hisi_nfc_resume(struct device *dev)
+{
+	int cs;
+	struct hinfc_host *host = dev_get_drvdata(dev);
+	struct nand_chip *chip = &host->chip;
+
+	for (cs = 0; cs < chip->numchips; cs++)
+		hisi_nfc_send_cmd_reset(host, cs);
+	hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH,
+		    HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH);
+
+	return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(hisi_nfc_pm_ops, hisi_nfc_suspend, hisi_nfc_resume);
+
+static const struct of_device_id nfc_id_table[] = {
+	{ .compatible = "hisilicon,504-nfc" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, nfc_id_table);
+
+static struct platform_driver hisi_nfc_driver = {
+	.driver = {
+		.name  = "hisi_nand",
+		.of_match_table = of_match_ptr(nfc_id_table),
+		.pm = &hisi_nfc_pm_ops,
+	},
+	.probe		= hisi_nfc_probe,
+	.remove		= hisi_nfc_remove,
+};
+
+module_platform_driver(hisi_nfc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Zhiyong Cai");
+MODULE_AUTHOR("Zhou Wang");
+MODULE_DESCRIPTION("Hisilicon Nand Flash Controller Driver");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Arnd Bergmann]

On Monday 12 January 2015 15:28:53 Zhou Wang wrote:
> Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---

I haven't looked at earlier versions, but it looks very good to
me overall. Just add a patch description above please.

One tiny detail:

> +static const struct of_device_id nfc_id_table[] = {
> +	{ .compatible = "hisilicon,504-nfc" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, nfc_id_table);
> +
> +static struct platform_driver hisi_nfc_driver = {
> +	.driver = {
> +		.name  = "hisi_nand",
> +		.of_match_table = of_match_ptr(nfc_id_table),
> +		.pm = &hisi_nfc_pm_ops,
> +	},
> +	.probe		= hisi_nfc_probe,
> +	.remove		= hisi_nfc_remove,
> +};

Remove the of_match_ptr() macro here to avoid a warning when
CONFIG_OF is not set.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Zhou Wang]

On 2015/1/12 16:21, Arnd Bergmann wrote:
> On Monday 12 January 2015 15:28:53 Zhou Wang wrote:
>> Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>> ---
> 
> I haven't looked at earlier versions, but it looks very good to
> me overall. Just add a patch description above please.

Hi Arnd,

The earlier version is same with this one, just change E-mail
address here. Will add a patch description in next version. Thanks!

> 
> One tiny detail:
> 
>> +static const struct of_device_id nfc_id_table[] = {
>> +	{ .compatible = "hisilicon,504-nfc" },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, nfc_id_table);
>> +
>> +static struct platform_driver hisi_nfc_driver = {
>> +	.driver = {
>> +		.name  = "hisi_nand",
>> +		.of_match_table = of_match_ptr(nfc_id_table),
>> +		.pm = &hisi_nfc_pm_ops,
>> +	},
>> +	.probe		= hisi_nfc_probe,
>> +	.remove		= hisi_nfc_remove,
>> +};
> 
> Remove the of_match_ptr() macro here to avoid a warning when
> CONFIG_OF is not set.

Will remove the macro here, thanks!

Best regards,
Zhou Wang

> 
> 	Arnd
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Brian Norris]

On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
> Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

Mostly good. A few small comments.

> ---
>  drivers/mtd/nand/Kconfig        |   5 +
>  drivers/mtd/nand/Makefile       |   1 +
>  drivers/mtd/nand/hisi504_nand.c | 907 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 913 insertions(+)
>  create mode 100644 drivers/mtd/nand/hisi504_nand.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 7d0150d..e1220fc 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -524,4 +524,9 @@ config MTD_NAND_SUNXI
>  	help
>  	  Enables support for NAND Flash chips on Allwinner SoCs.
>  
> +config MTD_NAND_HISI504
> +	tristate "Support for NAND controller on Hisilicon SoC Hip04"
> +	help
> +	  Enables support for NAND controller on Hisilicon SoC Hip04.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index bd38f21..582bbd05 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -51,5 +51,6 @@ obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
>  obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
>  obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
>  obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
> +obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
>  
>  nand-objs := nand_base.o nand_bbt.o nand_timings.o
> diff --git a/drivers/mtd/nand/hisi504_nand.c b/drivers/mtd/nand/hisi504_nand.c
> new file mode 100644
> index 0000000..2000f21
> --- /dev/null
> +++ b/drivers/mtd/nand/hisi504_nand.c
> @@ -0,0 +1,907 @@
> +/*
> + * Hisilicon NAND Flash controller driver
> + *
> + * Copyright © 2012-2014 HiSilicon Technologies Co., Ltd.
> + *              http://www.hisilicon.com
> + *
> + * Author: Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + * The initial developer of the original code is Zhiyong Cai
> + * <caizhiyong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/of.h>
> +#include <linux/of_mtd.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/sizes.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
> +#include <linux/mtd/partitions.h>
> +
> +#define HINFC504_MAX_CHIP                               (4)
> +#define HINFC504_W_LATCH                                (5)
> +#define HINFC504_R_LATCH                                (7)
> +#define HINFC504_RW_LATCH                               (3)
> +
> +#define HINFC504_NFC_TIMEOUT				(2 * HZ)
> +#define HINFC504_NFC_PM_TIMEOUT				(1 * HZ)
> +#define HINFC504_NFC_DMA_TIMEOUT			(5 * HZ)
> +#define HINFC504_CHIP_DELAY				(25)
> +
> +#define HINFC504_REG_BASE_ADDRESS_LEN			(0x100)
> +#define HINFC504_BUFFER_BASE_ADDRESS_LEN		(2048 + 128)
> +
> +#define HINFC504_ADDR_CYCLE_MASK			0x4
> +
> +#define HINFC504_CON					0x00
> +#define HINFC504_CON_OP_MODE_NORMAL			BIT(0)
> +#define HINFC504_CON_PAGEISZE_SHIFT			(1)
> +#define HINFC504_CON_PAGESIZE_MASK			(0x07)
> +#define HINFC504_CON_BUS_WIDTH				BIT(4)
> +#define HINFC504_CON_READY_BUSY_SEL			BIT(8)
> +#define HINFC504_CON_ECCTYPE_SHIFT			(9)
> +#define HINFC504_CON_ECCTYPE_MASK			(0x07)
> +
> +#define HINFC504_PWIDTH					0x04
> +#define SET_HINFC504_PWIDTH(_w_lcnt, _r_lcnt, _rw_hcnt) \
> +	((_w_lcnt) | (((_r_lcnt) & 0x0F) << 4) | (((_rw_hcnt) & 0x0F) << 8))
> +
> +#define HINFC504_CMD					0x0C
> +#define HINFC504_ADDRL					0x10
> +#define HINFC504_ADDRH					0x14
> +#define HINFC504_DATA_NUM				0x18
> +
> +#define HINFC504_OP					0x1C
> +#define HINFC504_OP_READ_DATA_EN			BIT(1)
> +#define HINFC504_OP_WAIT_READY_EN			BIT(2)
> +#define HINFC504_OP_CMD2_EN				BIT(3)
> +#define HINFC504_OP_WRITE_DATA_EN			BIT(4)
> +#define HINFC504_OP_ADDR_EN				BIT(5)
> +#define HINFC504_OP_CMD1_EN				BIT(6)
> +#define HINFC504_OP_NF_CS_SHIFT                         (7)
> +#define HINFC504_OP_NF_CS_MASK				(3)
> +#define HINFC504_OP_ADDR_CYCLE_SHIFT			(9)
> +#define HINFC504_OP_ADDR_CYCLE_MASK			(7)
> +
> +#define HINFC504_STATUS                                 0x20
> +#define HINFC504_READY					BIT(0)
> +
> +#define HINFC504_INTEN					0x24
> +#define HINFC504_INTEN_DMA				BIT(9)
> +#define HINFC504_INTEN_UE				BIT(6)
> +#define HINFC504_INTEN_CE				BIT(5)
> +
> +#define HINFC504_INTS					0x28
> +#define HINFC504_INTS_DMA				BIT(9)
> +#define HINFC504_INTS_UE				BIT(6)
> +#define HINFC504_INTS_CE				BIT(5)
> +
> +#define HINFC504_INTCLR                                 0x2C
> +#define HINFC504_INTCLR_DMA				BIT(9)
> +#define HINFC504_INTCLR_UE				BIT(6)
> +#define HINFC504_INTCLR_CE				BIT(5)
> +
> +#define HINFC504_ECC_STATUS                             0x5C
> +#define HINFC504_ECC_1_BIT_SHIFT                        16
> +#define HINFC504_ECC_16_BIT_SHIFT                       12
> +
> +#define HINFC504_DMA_CTRL				0x60
> +#define HINFC504_DMA_CTRL_DMA_START			BIT(0)
> +#define HINFC504_DMA_CTRL_WE				BIT(1)
> +#define HINFC504_DMA_CTRL_DATA_AREA_EN			BIT(2)
> +#define HINFC504_DMA_CTRL_OOB_AREA_EN			BIT(3)
> +#define HINFC504_DMA_CTRL_BURST4_EN			BIT(4)
> +#define HINFC504_DMA_CTRL_BURST8_EN			BIT(5)
> +#define HINFC504_DMA_CTRL_BURST16_EN			BIT(6)
> +#define HINFC504_DMA_CTRL_ADDR_NUM_SHIFT		(7)
> +#define HINFC504_DMA_CTRL_ADDR_NUM_MASK                 (1)
> +#define HINFC504_DMA_CTRL_CS_SHIFT			(8)
> +#define HINFC504_DMA_CTRL_CS_MASK			(0x03)
> +
> +#define HINFC504_DMA_ADDR_DATA				0x64
> +#define HINFC504_DMA_ADDR_OOB				0x68
> +
> +#define HINFC504_DMA_LEN				0x6C
> +#define HINFC504_DMA_LEN_OOB_SHIFT			(16)
> +#define HINFC504_DMA_LEN_OOB_MASK			(0xFFF)
> +
> +#define HINFC504_DMA_PARA				0x70
> +#define HINFC504_DMA_PARA_DATA_RW_EN			BIT(0)
> +#define HINFC504_DMA_PARA_OOB_RW_EN			BIT(1)
> +#define HINFC504_DMA_PARA_DATA_EDC_EN			BIT(2)
> +#define HINFC504_DMA_PARA_OOB_EDC_EN			BIT(3)
> +#define HINFC504_DMA_PARA_DATA_ECC_EN			BIT(4)
> +#define HINFC504_DMA_PARA_OOB_ECC_EN			BIT(5)
> +
> +#define HINFC_VERSION                                   0x74
> +#define HINFC504_LOG_READ_ADDR				0x7C
> +#define HINFC504_LOG_READ_LEN				0x80
> +
> +#define HINFC504_NANDINFO_LEN				0x10
> +
> +struct hinfc_host {
> +	struct nand_chip	chip;
> +	struct mtd_info		mtd;
> +	struct device		*dev;
> +	void __iomem		*iobase;
> +	struct completion       cmd_complete;
> +	unsigned int		offset;
> +	unsigned int		command;
> +	int			chipselect;
> +	unsigned int		addr_cycle;
> +	u32                     addr_value[2];
> +	u32                     cache_addr_value[2];
> +	char			*buffer;
> +	dma_addr_t		dma_buffer;
> +	dma_addr_t		dma_oob;
> +	int			version;
> +	unsigned int            irq_status; /* interrupt status */
> +};
> +
> +static inline unsigned int hinfc_read(struct hinfc_host *host, unsigned int reg)
> +{
> +	return readl(host->iobase + reg);
> +}
> +
> +static inline void hinfc_write(struct hinfc_host *host, unsigned int value,
> +			       unsigned int reg)
> +{
> +	writel(value, host->iobase + reg);
> +}
> +
> +static void wait_controller_finished(struct hinfc_host *host)
> +{
> +	unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT;
> +	int val;
> +
> +	while (time_before(jiffies, timeout)) {
> +		val = hinfc_read(host, HINFC504_STATUS);
> +		if (host->command == NAND_CMD_ERASE2) {
> +			/* nfc is ready */
> +			while (!(val & HINFC504_READY))	{
> +				usleep_range(500, 1000);
> +				val = hinfc_read(host, HINFC504_STATUS);
> +			}
> +			return;
> +		}
> +
> +		if (val & HINFC504_READY)
> +			return;
> +	}
> +
> +	/* wait cmd timeout */
> +	dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n");
> +}
> +
> +static void hisi_nfc_dma_transfer(struct hinfc_host *host, int todev)
> +{
> +	struct mtd_info	*mtd = &host->mtd;
> +	struct nand_chip *chip = mtd->priv;
> +	unsigned long val;
> +	int ret;
> +
> +	hinfc_write(host, host->dma_buffer, HINFC504_DMA_ADDR_DATA);
> +	hinfc_write(host, host->dma_oob, HINFC504_DMA_ADDR_OOB);
> +
> +	if (chip->ecc.mode == NAND_ECC_NONE) {
> +		hinfc_write(host, ((mtd->oobsize & HINFC504_DMA_LEN_OOB_MASK)
> +			<< HINFC504_DMA_LEN_OOB_SHIFT), HINFC504_DMA_LEN);
> +
> +		hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN
> +			| HINFC504_DMA_PARA_OOB_RW_EN, HINFC504_DMA_PARA);
> +	} else {
> +		if (host->command == NAND_CMD_READOOB)
> +			hinfc_write(host, HINFC504_DMA_PARA_OOB_RW_EN
> +			| HINFC504_DMA_PARA_OOB_EDC_EN
> +			| HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA);
> +		else
> +			hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN
> +			| HINFC504_DMA_PARA_OOB_RW_EN
> +			| HINFC504_DMA_PARA_DATA_EDC_EN
> +			| HINFC504_DMA_PARA_OOB_EDC_EN
> +			| HINFC504_DMA_PARA_DATA_ECC_EN
> +			| HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA);
> +
> +	}
> +
> +	val = (HINFC504_DMA_CTRL_DMA_START | HINFC504_DMA_CTRL_BURST4_EN
> +		| HINFC504_DMA_CTRL_BURST8_EN | HINFC504_DMA_CTRL_BURST16_EN
> +		| HINFC504_DMA_CTRL_DATA_AREA_EN | HINFC504_DMA_CTRL_OOB_AREA_EN
> +		| ((host->addr_cycle == 4 ? 1 : 0)
> +			<< HINFC504_DMA_CTRL_ADDR_NUM_SHIFT)
> +		| ((host->chipselect & HINFC504_DMA_CTRL_CS_MASK)
> +			<< HINFC504_DMA_CTRL_CS_SHIFT));
> +
> +	if (todev)
> +		val |= HINFC504_DMA_CTRL_WE;
> +
> +	init_completion(&host->cmd_complete);
> +
> +	hinfc_write(host, val, HINFC504_DMA_CTRL);
> +	ret = wait_for_completion_timeout(&host->cmd_complete,
> +			HINFC504_NFC_DMA_TIMEOUT);
> +
> +	if (!ret) {
> +		dev_err(host->dev, "DMA operation(irq) timeout!\n");
> +		/* sanity check */
> +		val = hinfc_read(host, HINFC504_DMA_CTRL);
> +		if (!(val & HINFC504_DMA_CTRL_DMA_START))
> +			dev_err(host->dev, "DMA is already done but without irq ACK!\n");
> +		else
> +			dev_err(host->dev, "DMA is really timeout!\n");
> +	}
> +}
> +
> +static int hisi_nfc_send_cmd_pageprog(struct hinfc_host *host)
> +{
> +	host->addr_value[0] &= 0xffff0000;
> +
> +	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
> +	hinfc_write(host, host->addr_value[1], HINFC504_ADDRH);
> +	hinfc_write(host, NAND_CMD_PAGEPROG << 8 | NAND_CMD_SEQIN,
> +		    HINFC504_CMD);
> +
> +	hisi_nfc_dma_transfer(host, 1);
> +
> +	return 0;
> +}
> +
> +static int hisi_nfc_send_cmd_readstart(struct hinfc_host *host)
> +{
> +	struct mtd_info	*mtd = &host->mtd;
> +
> +	if ((host->addr_value[0] == host->cache_addr_value[0]) &&
> +	    (host->addr_value[1] == host->cache_addr_value[1]))
> +		return 0;
> +
> +	host->addr_value[0] &= 0xffff0000;
> +
> +	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
> +	hinfc_write(host, host->addr_value[1], HINFC504_ADDRH);
> +	hinfc_write(host, NAND_CMD_READSTART << 8 | NAND_CMD_READ0,
> +		    HINFC504_CMD);
> +
> +	hinfc_write(host, 0, HINFC504_LOG_READ_ADDR);
> +	hinfc_write(host, mtd->writesize + mtd->oobsize,
> +		    HINFC504_LOG_READ_LEN);
> +
> +	hisi_nfc_dma_transfer(host, 0);
> +
> +	host->cache_addr_value[0] = host->addr_value[0];
> +	host->cache_addr_value[1] = host->addr_value[1];
> +
> +	return 0;
> +}
> +
> +static int hisi_nfc_send_cmd_erase(struct hinfc_host *host)
> +{
> +	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
> +	hinfc_write(host, (NAND_CMD_ERASE2 << 8) | NAND_CMD_ERASE1,
> +		    HINFC504_CMD);
> +
> +	hinfc_write(host, HINFC504_OP_WAIT_READY_EN
> +		| HINFC504_OP_CMD2_EN
> +		| HINFC504_OP_CMD1_EN
> +		| HINFC504_OP_ADDR_EN
> +		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
> +			<< HINFC504_OP_NF_CS_SHIFT)
> +		| ((host->addr_cycle & HINFC504_OP_ADDR_CYCLE_MASK)
> +			<< HINFC504_OP_ADDR_CYCLE_SHIFT),
> +		HINFC504_OP);
> +
> +	wait_controller_finished(host);
> +
> +	return 0;
> +}
> +
> +static int hisi_nfc_send_cmd_readid(struct hinfc_host *host)
> +{
> +	hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM);
> +	hinfc_write(host, NAND_CMD_READID, HINFC504_CMD);
> +	hinfc_write(host, 0, HINFC504_ADDRL);
> +
> +	hinfc_write(host, HINFC504_OP_CMD1_EN | HINFC504_OP_ADDR_EN
> +		| HINFC504_OP_READ_DATA_EN
> +		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
> +			<< HINFC504_OP_NF_CS_SHIFT)
> +		| 1 << HINFC504_OP_ADDR_CYCLE_SHIFT, HINFC504_OP);
> +
> +	wait_controller_finished(host);
> +
> +	return 0;
> +}
> +
> +static int hisi_nfc_send_cmd_status(struct hinfc_host *host)
> +{
> +	hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM);
> +	hinfc_write(host, NAND_CMD_STATUS, HINFC504_CMD);
> +	hinfc_write(host, HINFC504_OP_CMD1_EN
> +		| HINFC504_OP_READ_DATA_EN
> +		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
> +			<< HINFC504_OP_NF_CS_SHIFT),
> +		HINFC504_OP);
> +
> +	wait_controller_finished(host);
> +
> +	return 0;
> +}
> +
> +static int hisi_nfc_send_cmd_reset(struct hinfc_host *host, int chipselect)
> +{
> +	hinfc_write(host, NAND_CMD_RESET, HINFC504_CMD);
> +
> +	hinfc_write(host, HINFC504_OP_CMD1_EN
> +		| ((chipselect & HINFC504_OP_NF_CS_MASK)
> +			<< HINFC504_OP_NF_CS_SHIFT)
> +		| HINFC504_OP_WAIT_READY_EN,
> +		HINFC504_OP);
> +
> +	wait_controller_finished(host);
> +
> +	return 0;
> +}
> +
> +static void hisi_nfc_select_chip(struct mtd_info *mtd, int chipselect)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +
> +	if (chipselect < 0)
> +		return;
> +
> +	host->chipselect = chipselect;
> +}
> +
> +static uint8_t hisi_nfc_read_byte(struct mtd_info *mtd)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +
> +	if (host->command == NAND_CMD_STATUS)
> +		return readb(chip->IO_ADDR_R);
> +
> +	host->offset++;
> +
> +	if (host->command == NAND_CMD_READID)
> +		return readb(chip->IO_ADDR_R + host->offset - 1);
> +
> +	return readb(host->buffer + host->offset - 1);

You're reading from memory, not IO. You don't need readb().

> +}
> +
> +static u16 hisi_nfc_read_word(struct mtd_info *mtd)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +
> +	host->offset += 2;
> +	return readw(host->buffer + host->offset - 2);

Same here, for readw().

> +}
> +
> +static void
> +hisi_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +
> +	memcpy(host->buffer + host->offset, buf, len);
> +	host->offset += len;
> +}
> +
> +static void hisi_nfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +
> +	memcpy(buf, host->buffer + host->offset, len);
> +	host->offset += len;
> +}
> +
> +static void set_addr(struct mtd_info *mtd, int column, int page_addr)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +	unsigned int command = host->command;
> +
> +	host->addr_cycle    = 0;
> +	host->addr_value[0] = 0;
> +	host->addr_value[1] = 0;
> +
> +	/* Serially input address */
> +	if (column != -1) {
> +		/* Adjust columns for 16 bit buswidth */
> +		if (chip->options & NAND_BUSWIDTH_16 &&
> +				!nand_opcode_8bits(command))
> +			column >>= 1;
> +
> +		host->addr_value[0] = column & 0xffff;
> +		host->addr_cycle    = 2;
> +	}
> +	if (page_addr != -1) {
> +		host->addr_value[0] |= (page_addr & 0xffff)
> +			<< (host->addr_cycle * 8);
> +		host->addr_cycle    += 2;
> +		/* One more address cycle for devices > 128MiB */
> +		if (chip->chipsize > (128 << 20)) {
> +			host->addr_cycle += 1;
> +			if (host->command == NAND_CMD_ERASE1)
> +				host->addr_value[0] |= ((page_addr >> 16) & 0xff) << 16;
> +			else
> +				host->addr_value[1] |= ((page_addr >> 16) & 0xff);
> +		}
> +	}
> +}
> +
> +static void hisi_nfc_cmdfunc(struct mtd_info *mtd, unsigned command, int column,
> +		int page_addr)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct hinfc_host *host = chip->priv;
> +	int is_cache_invalid = 1;
> +	unsigned int flag = 0;
> +
> +	host->command =  command;
> +
> +	switch (command) {
> +	case NAND_CMD_READ0:
> +	case NAND_CMD_READOOB:
> +		if (command == NAND_CMD_READ0)
> +			host->offset = column;
> +		else
> +			host->offset = column + mtd->writesize;
> +
> +		is_cache_invalid = 0;
> +		set_addr(mtd, column, page_addr);
> +		hisi_nfc_send_cmd_readstart(host);
> +		break;
> +
> +	case NAND_CMD_SEQIN:
> +		host->offset = column;
> +		set_addr(mtd, column, page_addr);
> +		break;
> +
> +	case NAND_CMD_ERASE1:
> +		set_addr(mtd, column, page_addr);
> +		break;
> +
> +	case NAND_CMD_PAGEPROG:
> +		hisi_nfc_send_cmd_pageprog(host);
> +		break;
> +
> +	case NAND_CMD_ERASE2:
> +		hisi_nfc_send_cmd_erase(host);
> +		break;
> +
> +	case NAND_CMD_READID:
> +		host->offset = column;
> +		memset(chip->IO_ADDR_R, 0, 0x10);
> +		hisi_nfc_send_cmd_readid(host);
> +		break;
> +
> +	case NAND_CMD_STATUS:
> +		flag = hinfc_read(host, HINFC504_CON);
> +		if (chip->ecc.mode == NAND_ECC_HW)
> +			hinfc_write(host,
> +				    flag && ~(HINFC504_CON_ECCTYPE_MASK <<
> +				    HINFC504_CON_ECCTYPE_SHIFT), HINFC504_CON);
> +
> +		host->offset = 0;
> +		memset(chip->IO_ADDR_R, 0, 0x10);
> +		hisi_nfc_send_cmd_status(host);
> +		hinfc_write(host, flag, HINFC504_CON);
> +		break;
> +
> +	case NAND_CMD_RESET:
> +		hisi_nfc_send_cmd_reset(host, host->chipselect);
> +		break;
> +
> +	default:
> +		dev_err(host->dev, "Error: unsupported cmd(cmd=%x, col=%x, page=%x)\n",
> +			command, column, page_addr);
> +	}
> +
> +	if (is_cache_invalid) {
> +		host->cache_addr_value[0] = ~0;
> +		host->cache_addr_value[1] = ~0;
> +	}
> +}
> +
> +static irqreturn_t hinfc_irq_handle(int irq, void *devid)
> +{
> +	struct hinfc_host *host = devid;
> +	unsigned int flag;
> +
> +	flag = hinfc_read(host, HINFC504_INTS);
> +	/* store interrupts state */
> +	host->irq_status |= flag;
> +
> +	if (flag & HINFC504_INTS_DMA) {
> +		hinfc_write(host, HINFC504_INTCLR_DMA, HINFC504_INTCLR);
> +		complete(&host->cmd_complete);
> +	} else if (flag & HINFC504_INTS_CE) {
> +		hinfc_write(host, HINFC504_INTCLR_CE, HINFC504_INTCLR);
> +	} else if (flag & HINFC504_INTS_UE) {
> +		hinfc_write(host, HINFC504_INTCLR_UE, HINFC504_INTCLR);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int hisi_nand_read_page_hwecc(struct mtd_info *mtd,
> +	struct nand_chip *chip, uint8_t *buf, int oob_required, int page)
> +{
> +	struct hinfc_host *host = chip->priv;
> +	int max_bitflips = 0, stat = 0, stat_max, status_ecc;
> +	int stat_1, stat_2;
> +
> +	chip->read_buf(mtd, buf, mtd->writesize);
> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	/* errors which can not be corrected by ECC */
> +	if (host->irq_status & HINFC504_INTS_UE) {
> +		mtd->ecc_stats.failed++;
> +	} else if (host->irq_status & HINFC504_INTS_CE) {
> +		/* TODO: need add other ECC modes! */
> +		switch (chip->ecc.strength) {
> +		case 1:
> +			stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>>
> +						HINFC504_ECC_1_BIT_SHIFT);
> +			stat_max = 1;
> +			break;
> +		case 16:
> +			status_ecc = hinfc_read(host, HINFC504_ECC_STATUS) >>
> +					HINFC504_ECC_16_BIT_SHIFT & 0x0fff;
> +			stat_2 = status_ecc & 0x3f;
> +			stat_1 = status_ecc >> 6 & 0x3f;
> +			stat = stat_1 + stat_2;
> +			stat_max = max_t(int, stat_1, stat_2);
> +		}
> +		mtd->ecc_stats.corrected += stat;
> +		max_bitflips = max_t(int, max_bitflips, stat_max);
> +	}
> +	host->irq_status = 0;
> +
> +	return max_bitflips;
> +}
> +
> +static int hisi_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
> +				int page)
> +{
> +	struct hinfc_host *host = chip->priv;
> +
> +	chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	if (host->irq_status & HINFC504_INTS_UE) {
> +		host->irq_status = 0;
> +		return -EBADMSG;
> +	}
> +
> +	host->irq_status = 0;
> +	return 0;
> +}
> +
> +static int hisi_nand_write_page_hwecc(struct mtd_info *mtd,
> +		struct nand_chip *chip, const uint8_t *buf, int oob_required)
> +{
> +	chip->write_buf(mtd, buf, mtd->writesize);
> +	if (oob_required)
> +		chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
> +}
> +
> +static void hisi_nfc_host_init(struct hinfc_host *host)
> +{
> +	struct nand_chip *chip = &host->chip;
> +	unsigned int flag = 0;
> +
> +	host->version = hinfc_read(host, HINFC_VERSION);
> +	host->addr_cycle		= 0;
> +	host->addr_value[0]		= 0;
> +	host->addr_value[1]		= 0;
> +	host->cache_addr_value[0]	= ~0;
> +	host->cache_addr_value[1]	= ~0;
> +	host->chipselect		= 0;
> +
> +	/* default page size: 2K, ecc_none. need modify */
> +	flag = HINFC504_CON_OP_MODE_NORMAL | HINFC504_CON_READY_BUSY_SEL
> +		| ((0x001 & HINFC504_CON_PAGESIZE_MASK)
> +			<< HINFC504_CON_PAGEISZE_SHIFT)
> +		| ((0x0 & HINFC504_CON_ECCTYPE_MASK)
> +			<< HINFC504_CON_ECCTYPE_SHIFT)
> +		| ((chip->options & NAND_BUSWIDTH_16) ?
> +			HINFC504_CON_BUS_WIDTH : 0);
> +	hinfc_write(host, flag, HINFC504_CON);
> +
> +	memset(chip->IO_ADDR_R, 0xff, HINFC504_BUFFER_BASE_ADDRESS_LEN);
> +
> +	hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH,
> +		    HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH);
> +
> +	/* enable DMA irq */
> +	hinfc_write(host, HINFC504_INTEN_DMA, HINFC504_INTEN);
> +}
> +
> +static struct nand_ecclayout nand_ecc_2K_1bit = {
> +	.oobfree = { {24, 40} }
> +};
> +
> +static struct nand_ecclayout nand_ecc_2K_16bits = {
> +	.oobavail = 6,
> +	.oobfree = { {2, 6} },
> +};
> +
> +static int hisi_nfc_ecc_probe(struct hinfc_host *host)
> +{
> +	unsigned int flag;
> +	int size, strength, ecc_bits;
> +	struct device *dev = host->dev;
> +	struct nand_chip *chip = &host->chip;
> +	struct mtd_info *mtd = &host->mtd;
> +	struct device_node *np = host->dev->of_node;
> +
> +	size = of_get_nand_ecc_step_size(np);
> +	strength = of_get_nand_ecc_strength(np);
> +	if ((size != 512) && (size != 1024)) {
> +		dev_err(dev, "error ecc size: %d\n", size);
> +		return -EINVAL;
> +	}
> +
> +	if ((size == 512) && (strength != 1)) {
> +		dev_err(dev, "ecc size and strength do not match\n");
> +		return -EINVAL;
> +	}
> +	if ((size == 1024) && ((strength != 8) && (strength != 16) &&
> +				(strength != 24) && (strength != 40))) {
> +		dev_err(dev, "ecc size and strength do not match\n");
> +		return -EINVAL;
> +	}
> +
> +	chip->ecc.size = size;
> +	chip->ecc.strength = strength;
> +
> +	chip->ecc.read_page = hisi_nand_read_page_hwecc;
> +	chip->ecc.read_oob = hisi_nand_read_oob;
> +	chip->ecc.write_page = hisi_nand_write_page_hwecc;
> +
> +	switch (chip->ecc.strength) {
> +	case 1:
> +		ecc_bits = 1;
> +		if (mtd->writesize == 2048)
> +			chip->ecc.layout = &nand_ecc_2K_1bit;
> +
> +		/* TODO: add more page size support */
> +		break;
> +	case 16:
> +		ecc_bits = 6;
> +		if (mtd->writesize == 2048)
> +			chip->ecc.layout = &nand_ecc_2K_16bits;
> +
> +		/* TODO: add more page size support */
> +		break;
> +
> +	/* TODO: add more ecc strength support */
> +	default:
> +		dev_err(dev, "not support strength: %d\n", chip->ecc.strength);
> +		return -EINVAL;
> +	}
> +
> +	flag = hinfc_read(host, HINFC504_CON);
> +	/* add ecc type configure */
> +	flag |= ((ecc_bits & HINFC504_CON_ECCTYPE_MASK)
> +						<< HINFC504_CON_ECCTYPE_SHIFT);
> +	hinfc_write(host, flag, HINFC504_CON);
> +
> +	/* enable ecc irq */
> +	flag = hinfc_read(host, HINFC504_INTEN) & 0xfff;
> +	hinfc_write(host, flag | HINFC504_INTEN_UE | HINFC504_INTEN_CE,
> +		    HINFC504_INTEN);
> +
> +	return 0;
> +}
> +
> +static int hisi_nfc_probe(struct platform_device *pdev)
> +{
> +	int ret = 0, irq, buswidth, flag, max_chips = HINFC504_MAX_CHIP;
> +	struct device *dev = &pdev->dev;
> +	struct hinfc_host *host;
> +	struct nand_chip  *chip;
> +	struct mtd_info   *mtd;
> +	struct resource	  *res;
> +	struct device_node *np = dev->of_node;
> +	struct mtd_part_parser_data ppdata;
> +
> +	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return -ENOMEM;
> +	host->dev = dev;
> +
> +	platform_set_drvdata(pdev, host);
> +	chip = &host->chip;
> +	mtd  = &host->mtd;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(dev, "no IRQ resource defined\n");
> +		ret = -ENXIO;
> +		goto err_res;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	host->iobase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(host->iobase)) {
> +		ret = PTR_ERR(host->iobase);
> +		dev_err(dev, "devm_ioremap_resource[0] fail\n");

I don't think yhou really need this error print. devm_ioremap_resource()
should be descriptive enough, I think.

> +		goto err_res;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res);

Hmm, do you really have to reuse IO_ADDR_{R,W} here? Those are only
targeted for NAND systems which have a direct MMIO mapping to the NAND
I/O pins. See nand_base's {read,write}_buf() and read_{byte,word}()
implementations. But you override those.

It's best if it's obvious if nand_base is somehow inadvertently using
these pointers. So leaving them NULL is helpful.

As an alternative, you can just stash another private void __iomem
pointer in you your host struct.

> +	if (IS_ERR(chip->IO_ADDR_R)) {
> +		ret = PTR_ERR(chip->IO_ADDR_R);
> +		dev_err(dev, "devm_ioremap_resource[1] fail\n");
> +		goto err_res;
> +	}
> +
> +	mtd->priv		= chip;
> +	mtd->owner		= THIS_MODULE;
> +	mtd->name		= "hisi_nand";
> +	mtd->dev.parent         = &pdev->dev;
> +
> +	chip->priv		= host;
> +	chip->cmdfunc		= hisi_nfc_cmdfunc;
> +	chip->select_chip	= hisi_nfc_select_chip;
> +	chip->read_byte		= hisi_nfc_read_byte;
> +	chip->read_word		= hisi_nfc_read_word;
> +	chip->write_buf		= hisi_nfc_write_buf;
> +	chip->read_buf		= hisi_nfc_read_buf;
> +	chip->chip_delay	= HINFC504_CHIP_DELAY;
> +
> +	chip->ecc.mode = of_get_nand_ecc_mode(np);
> +
> +	buswidth = of_get_nand_bus_width(np);
> +	if (buswidth == 16)
> +		chip->options |= NAND_BUSWIDTH_16;
> +
> +	hisi_nfc_host_init(host);
> +
> +	ret = devm_request_irq(dev, irq, hinfc_irq_handle, IRQF_DISABLED,
> +				"nandc", host);
> +	if (ret) {
> +		dev_err(dev, "failed to request IRQ\n");
> +		goto err_res;
> +	}
> +
> +	ret = nand_scan_ident(mtd, max_chips, NULL);
> +	if (ret) {
> +		ret = -ENODEV;
> +		goto err_res;
> +	}
> +
> +	host->buffer = dmam_alloc_coherent(dev, mtd->writesize + mtd->oobsize,
> +		&host->dma_buffer, GFP_KERNEL);

You need to check this for allocation failures.

> +	host->dma_oob = host->dma_buffer + mtd->writesize;
> +	memset(host->buffer, 0xff, mtd->writesize + mtd->oobsize);
> +
> +	flag = hinfc_read(host, HINFC504_CON);
> +	flag &= ~(HINFC504_CON_PAGESIZE_MASK << HINFC504_CON_PAGEISZE_SHIFT);
> +	switch (mtd->writesize) {
> +	case 2048:
> +		flag |= (0x001 << HINFC504_CON_PAGEISZE_SHIFT); break;
> +	/*
> +	 * TODO: add more pagesize support,
> +	 * default pagesize has been set in hisi_nfc_host_init
> +	 */
> +	default:
> +		dev_err(dev, "NON-2KB page size nand flash\n");
> +		ret = -EINVAL;
> +		goto err_res;
> +	}
> +	hinfc_write(host, flag, HINFC504_CON);
> +
> +	if (chip->ecc.mode == NAND_ECC_HW)
> +		hisi_nfc_ecc_probe(host);
> +
> +	ret = nand_scan_tail(mtd);
> +	if (ret) {
> +		dev_err(dev, "nand_scan_tail failed: %d\n", ret);
> +		goto err_res;
> +	}
> +
> +	ppdata.of_node = np;
> +	ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
> +	if (ret) {
> +		dev_err(dev, "Err MTD partition=%d\n", ret);
> +		goto err_mtd;
> +	}
> +
> +	return 0;
> +
> +err_mtd:
> +	nand_release(mtd);
> +err_res:
> +	return ret;
> +}
> +
> +static int hisi_nfc_remove(struct platform_device *pdev)
> +{
> +	struct hinfc_host *host = platform_get_drvdata(pdev);
> +	struct mtd_info *mtd = &host->mtd;
> +
> +	nand_release(mtd);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int hisi_nfc_suspend(struct device *dev)
> +{
> +	struct hinfc_host *host = dev_get_drvdata(dev);
> +	unsigned long timeout = jiffies + HINFC504_NFC_PM_TIMEOUT;
> +
> +	while (time_before(jiffies, timeout)) {
> +		if (((hinfc_read(host, HINFC504_STATUS) & 0x1) == 0x0) &&
> +		    (hinfc_read(host, HINFC504_DMA_CTRL) &
> +		     HINFC504_DMA_CTRL_DMA_START)) {
> +			_cond_resched();

Why not just cond_resched()?

> +			return 0;
> +		}
> +	}
> +
> +	dev_err(host->dev, "nand controller suspend timeout.\n");
> +
> +	return -EAGAIN;
> +}
> +
> +static int hisi_nfc_resume(struct device *dev)
> +{
> +	int cs;
> +	struct hinfc_host *host = dev_get_drvdata(dev);
> +	struct nand_chip *chip = &host->chip;
> +
> +	for (cs = 0; cs < chip->numchips; cs++)
> +		hisi_nfc_send_cmd_reset(host, cs);
> +	hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH,
> +		    HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH);
> +
> +	return 0;
> +}
> +#endif
> +static SIMPLE_DEV_PM_OPS(hisi_nfc_pm_ops, hisi_nfc_suspend, hisi_nfc_resume);
> +
> +static const struct of_device_id nfc_id_table[] = {
> +	{ .compatible = "hisilicon,504-nfc" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, nfc_id_table);
> +
> +static struct platform_driver hisi_nfc_driver = {
> +	.driver = {
> +		.name  = "hisi_nand",
> +		.of_match_table = of_match_ptr(nfc_id_table),
> +		.pm = &hisi_nfc_pm_ops,
> +	},
> +	.probe		= hisi_nfc_probe,
> +	.remove		= hisi_nfc_remove,
> +};
> +
> +module_platform_driver(hisi_nfc_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Zhiyong Cai");
> +MODULE_AUTHOR("Zhou Wang");
> +MODULE_DESCRIPTION("Hisilicon Nand Flash Controller Driver");

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Zhou Wang]

On 2015/1/13 11:58, Brian Norris wrote:
> On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
>> Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> Mostly good. A few small comments.
> 
>> ---
>>  drivers/mtd/nand/Kconfig        |   5 +
>>  drivers/mtd/nand/Makefile       |   1 +
>>  drivers/mtd/nand/hisi504_nand.c | 907 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 913 insertions(+)
>>  create mode 100644 drivers/mtd/nand/hisi504_nand.c
>>
>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>> index 7d0150d..e1220fc 100644
>> --- a/drivers/mtd/nand/Kconfig
>> +++ b/drivers/mtd/nand/Kconfig
>> @@ -524,4 +524,9 @@ config MTD_NAND_SUNXI
>>  	help
>>  	  Enables support for NAND Flash chips on Allwinner SoCs.
>>  
>> +config MTD_NAND_HISI504
>> +	tristate "Support for NAND controller on Hisilicon SoC Hip04"
>> +	help
>> +	  Enables support for NAND controller on Hisilicon SoC Hip04.
>> +
>>  endif # MTD_NAND
>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index bd38f21..582bbd05 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -51,5 +51,6 @@ obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
>>  obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
>>  obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
>>  obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
>> +obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
>>  
>>  nand-objs := nand_base.o nand_bbt.o nand_timings.o
>> diff --git a/drivers/mtd/nand/hisi504_nand.c b/drivers/mtd/nand/hisi504_nand.c
>> new file mode 100644
>> index 0000000..2000f21
>> --- /dev/null
>> +++ b/drivers/mtd/nand/hisi504_nand.c
>> @@ -0,0 +1,907 @@
>> +/*
>> + * Hisilicon NAND Flash controller driver
>> + *
>> + * Copyright © 2012-2014 HiSilicon Technologies Co., Ltd.
>> + *              http://www.hisilicon.com
>> + *
>> + * Author: Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> + * The initial developer of the original code is Zhiyong Cai
>> + * <caizhiyong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +#include <linux/of.h>
>> +#include <linux/of_mtd.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/sizes.h>
>> +#include <linux/clk.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/mtd/nand.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/mtd/partitions.h>
>> +
>> +#define HINFC504_MAX_CHIP                               (4)
>> +#define HINFC504_W_LATCH                                (5)
>> +#define HINFC504_R_LATCH                                (7)
>> +#define HINFC504_RW_LATCH                               (3)
>> +
>> +#define HINFC504_NFC_TIMEOUT				(2 * HZ)
>> +#define HINFC504_NFC_PM_TIMEOUT				(1 * HZ)
>> +#define HINFC504_NFC_DMA_TIMEOUT			(5 * HZ)
>> +#define HINFC504_CHIP_DELAY				(25)
>> +
>> +#define HINFC504_REG_BASE_ADDRESS_LEN			(0x100)
>> +#define HINFC504_BUFFER_BASE_ADDRESS_LEN		(2048 + 128)
>> +
>> +#define HINFC504_ADDR_CYCLE_MASK			0x4
>> +
>> +#define HINFC504_CON					0x00
>> +#define HINFC504_CON_OP_MODE_NORMAL			BIT(0)
>> +#define HINFC504_CON_PAGEISZE_SHIFT			(1)
>> +#define HINFC504_CON_PAGESIZE_MASK			(0x07)
>> +#define HINFC504_CON_BUS_WIDTH				BIT(4)
>> +#define HINFC504_CON_READY_BUSY_SEL			BIT(8)
>> +#define HINFC504_CON_ECCTYPE_SHIFT			(9)
>> +#define HINFC504_CON_ECCTYPE_MASK			(0x07)
>> +
>> +#define HINFC504_PWIDTH					0x04
>> +#define SET_HINFC504_PWIDTH(_w_lcnt, _r_lcnt, _rw_hcnt) \
>> +	((_w_lcnt) | (((_r_lcnt) & 0x0F) << 4) | (((_rw_hcnt) & 0x0F) << 8))
>> +
>> +#define HINFC504_CMD					0x0C
>> +#define HINFC504_ADDRL					0x10
>> +#define HINFC504_ADDRH					0x14
>> +#define HINFC504_DATA_NUM				0x18
>> +
>> +#define HINFC504_OP					0x1C
>> +#define HINFC504_OP_READ_DATA_EN			BIT(1)
>> +#define HINFC504_OP_WAIT_READY_EN			BIT(2)
>> +#define HINFC504_OP_CMD2_EN				BIT(3)
>> +#define HINFC504_OP_WRITE_DATA_EN			BIT(4)
>> +#define HINFC504_OP_ADDR_EN				BIT(5)
>> +#define HINFC504_OP_CMD1_EN				BIT(6)
>> +#define HINFC504_OP_NF_CS_SHIFT                         (7)
>> +#define HINFC504_OP_NF_CS_MASK				(3)
>> +#define HINFC504_OP_ADDR_CYCLE_SHIFT			(9)
>> +#define HINFC504_OP_ADDR_CYCLE_MASK			(7)
>> +
>> +#define HINFC504_STATUS                                 0x20
>> +#define HINFC504_READY					BIT(0)
>> +
>> +#define HINFC504_INTEN					0x24
>> +#define HINFC504_INTEN_DMA				BIT(9)
>> +#define HINFC504_INTEN_UE				BIT(6)
>> +#define HINFC504_INTEN_CE				BIT(5)
>> +
>> +#define HINFC504_INTS					0x28
>> +#define HINFC504_INTS_DMA				BIT(9)
>> +#define HINFC504_INTS_UE				BIT(6)
>> +#define HINFC504_INTS_CE				BIT(5)
>> +
>> +#define HINFC504_INTCLR                                 0x2C
>> +#define HINFC504_INTCLR_DMA				BIT(9)
>> +#define HINFC504_INTCLR_UE				BIT(6)
>> +#define HINFC504_INTCLR_CE				BIT(5)
>> +
>> +#define HINFC504_ECC_STATUS                             0x5C
>> +#define HINFC504_ECC_1_BIT_SHIFT                        16
>> +#define HINFC504_ECC_16_BIT_SHIFT                       12
>> +
>> +#define HINFC504_DMA_CTRL				0x60
>> +#define HINFC504_DMA_CTRL_DMA_START			BIT(0)
>> +#define HINFC504_DMA_CTRL_WE				BIT(1)
>> +#define HINFC504_DMA_CTRL_DATA_AREA_EN			BIT(2)
>> +#define HINFC504_DMA_CTRL_OOB_AREA_EN			BIT(3)
>> +#define HINFC504_DMA_CTRL_BURST4_EN			BIT(4)
>> +#define HINFC504_DMA_CTRL_BURST8_EN			BIT(5)
>> +#define HINFC504_DMA_CTRL_BURST16_EN			BIT(6)
>> +#define HINFC504_DMA_CTRL_ADDR_NUM_SHIFT		(7)
>> +#define HINFC504_DMA_CTRL_ADDR_NUM_MASK                 (1)
>> +#define HINFC504_DMA_CTRL_CS_SHIFT			(8)
>> +#define HINFC504_DMA_CTRL_CS_MASK			(0x03)
>> +
>> +#define HINFC504_DMA_ADDR_DATA				0x64
>> +#define HINFC504_DMA_ADDR_OOB				0x68
>> +
>> +#define HINFC504_DMA_LEN				0x6C
>> +#define HINFC504_DMA_LEN_OOB_SHIFT			(16)
>> +#define HINFC504_DMA_LEN_OOB_MASK			(0xFFF)
>> +
>> +#define HINFC504_DMA_PARA				0x70
>> +#define HINFC504_DMA_PARA_DATA_RW_EN			BIT(0)
>> +#define HINFC504_DMA_PARA_OOB_RW_EN			BIT(1)
>> +#define HINFC504_DMA_PARA_DATA_EDC_EN			BIT(2)
>> +#define HINFC504_DMA_PARA_OOB_EDC_EN			BIT(3)
>> +#define HINFC504_DMA_PARA_DATA_ECC_EN			BIT(4)
>> +#define HINFC504_DMA_PARA_OOB_ECC_EN			BIT(5)
>> +
>> +#define HINFC_VERSION                                   0x74
>> +#define HINFC504_LOG_READ_ADDR				0x7C
>> +#define HINFC504_LOG_READ_LEN				0x80
>> +
>> +#define HINFC504_NANDINFO_LEN				0x10
>> +
>> +struct hinfc_host {
>> +	struct nand_chip	chip;
>> +	struct mtd_info		mtd;
>> +	struct device		*dev;
>> +	void __iomem		*iobase;
>> +	struct completion       cmd_complete;
>> +	unsigned int		offset;
>> +	unsigned int		command;
>> +	int			chipselect;
>> +	unsigned int		addr_cycle;
>> +	u32                     addr_value[2];
>> +	u32                     cache_addr_value[2];
>> +	char			*buffer;
>> +	dma_addr_t		dma_buffer;
>> +	dma_addr_t		dma_oob;
>> +	int			version;
>> +	unsigned int            irq_status; /* interrupt status */
>> +};
>> +
>> +static inline unsigned int hinfc_read(struct hinfc_host *host, unsigned int reg)
>> +{
>> +	return readl(host->iobase + reg);
>> +}
>> +
>> +static inline void hinfc_write(struct hinfc_host *host, unsigned int value,
>> +			       unsigned int reg)
>> +{
>> +	writel(value, host->iobase + reg);
>> +}
>> +
>> +static void wait_controller_finished(struct hinfc_host *host)
>> +{
>> +	unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT;
>> +	int val;
>> +
>> +	while (time_before(jiffies, timeout)) {
>> +		val = hinfc_read(host, HINFC504_STATUS);
>> +		if (host->command == NAND_CMD_ERASE2) {
>> +			/* nfc is ready */
>> +			while (!(val & HINFC504_READY))	{
>> +				usleep_range(500, 1000);
>> +				val = hinfc_read(host, HINFC504_STATUS);
>> +			}
>> +			return;
>> +		}
>> +
>> +		if (val & HINFC504_READY)
>> +			return;
>> +	}
>> +
>> +	/* wait cmd timeout */
>> +	dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n");
>> +}
>> +
>> +static void hisi_nfc_dma_transfer(struct hinfc_host *host, int todev)
>> +{
>> +	struct mtd_info	*mtd = &host->mtd;
>> +	struct nand_chip *chip = mtd->priv;
>> +	unsigned long val;
>> +	int ret;
>> +
>> +	hinfc_write(host, host->dma_buffer, HINFC504_DMA_ADDR_DATA);
>> +	hinfc_write(host, host->dma_oob, HINFC504_DMA_ADDR_OOB);
>> +
>> +	if (chip->ecc.mode == NAND_ECC_NONE) {
>> +		hinfc_write(host, ((mtd->oobsize & HINFC504_DMA_LEN_OOB_MASK)
>> +			<< HINFC504_DMA_LEN_OOB_SHIFT), HINFC504_DMA_LEN);
>> +
>> +		hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN
>> +			| HINFC504_DMA_PARA_OOB_RW_EN, HINFC504_DMA_PARA);
>> +	} else {
>> +		if (host->command == NAND_CMD_READOOB)
>> +			hinfc_write(host, HINFC504_DMA_PARA_OOB_RW_EN
>> +			| HINFC504_DMA_PARA_OOB_EDC_EN
>> +			| HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA);
>> +		else
>> +			hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN
>> +			| HINFC504_DMA_PARA_OOB_RW_EN
>> +			| HINFC504_DMA_PARA_DATA_EDC_EN
>> +			| HINFC504_DMA_PARA_OOB_EDC_EN
>> +			| HINFC504_DMA_PARA_DATA_ECC_EN
>> +			| HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA);
>> +
>> +	}
>> +
>> +	val = (HINFC504_DMA_CTRL_DMA_START | HINFC504_DMA_CTRL_BURST4_EN
>> +		| HINFC504_DMA_CTRL_BURST8_EN | HINFC504_DMA_CTRL_BURST16_EN
>> +		| HINFC504_DMA_CTRL_DATA_AREA_EN | HINFC504_DMA_CTRL_OOB_AREA_EN
>> +		| ((host->addr_cycle == 4 ? 1 : 0)
>> +			<< HINFC504_DMA_CTRL_ADDR_NUM_SHIFT)
>> +		| ((host->chipselect & HINFC504_DMA_CTRL_CS_MASK)
>> +			<< HINFC504_DMA_CTRL_CS_SHIFT));
>> +
>> +	if (todev)
>> +		val |= HINFC504_DMA_CTRL_WE;
>> +
>> +	init_completion(&host->cmd_complete);
>> +
>> +	hinfc_write(host, val, HINFC504_DMA_CTRL);
>> +	ret = wait_for_completion_timeout(&host->cmd_complete,
>> +			HINFC504_NFC_DMA_TIMEOUT);
>> +
>> +	if (!ret) {
>> +		dev_err(host->dev, "DMA operation(irq) timeout!\n");
>> +		/* sanity check */
>> +		val = hinfc_read(host, HINFC504_DMA_CTRL);
>> +		if (!(val & HINFC504_DMA_CTRL_DMA_START))
>> +			dev_err(host->dev, "DMA is already done but without irq ACK!\n");
>> +		else
>> +			dev_err(host->dev, "DMA is really timeout!\n");
>> +	}
>> +}
>> +
>> +static int hisi_nfc_send_cmd_pageprog(struct hinfc_host *host)
>> +{
>> +	host->addr_value[0] &= 0xffff0000;
>> +
>> +	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
>> +	hinfc_write(host, host->addr_value[1], HINFC504_ADDRH);
>> +	hinfc_write(host, NAND_CMD_PAGEPROG << 8 | NAND_CMD_SEQIN,
>> +		    HINFC504_CMD);
>> +
>> +	hisi_nfc_dma_transfer(host, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_nfc_send_cmd_readstart(struct hinfc_host *host)
>> +{
>> +	struct mtd_info	*mtd = &host->mtd;
>> +
>> +	if ((host->addr_value[0] == host->cache_addr_value[0]) &&
>> +	    (host->addr_value[1] == host->cache_addr_value[1]))
>> +		return 0;
>> +
>> +	host->addr_value[0] &= 0xffff0000;
>> +
>> +	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
>> +	hinfc_write(host, host->addr_value[1], HINFC504_ADDRH);
>> +	hinfc_write(host, NAND_CMD_READSTART << 8 | NAND_CMD_READ0,
>> +		    HINFC504_CMD);
>> +
>> +	hinfc_write(host, 0, HINFC504_LOG_READ_ADDR);
>> +	hinfc_write(host, mtd->writesize + mtd->oobsize,
>> +		    HINFC504_LOG_READ_LEN);
>> +
>> +	hisi_nfc_dma_transfer(host, 0);
>> +
>> +	host->cache_addr_value[0] = host->addr_value[0];
>> +	host->cache_addr_value[1] = host->addr_value[1];
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_nfc_send_cmd_erase(struct hinfc_host *host)
>> +{
>> +	hinfc_write(host, host->addr_value[0], HINFC504_ADDRL);
>> +	hinfc_write(host, (NAND_CMD_ERASE2 << 8) | NAND_CMD_ERASE1,
>> +		    HINFC504_CMD);
>> +
>> +	hinfc_write(host, HINFC504_OP_WAIT_READY_EN
>> +		| HINFC504_OP_CMD2_EN
>> +		| HINFC504_OP_CMD1_EN
>> +		| HINFC504_OP_ADDR_EN
>> +		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
>> +			<< HINFC504_OP_NF_CS_SHIFT)
>> +		| ((host->addr_cycle & HINFC504_OP_ADDR_CYCLE_MASK)
>> +			<< HINFC504_OP_ADDR_CYCLE_SHIFT),
>> +		HINFC504_OP);
>> +
>> +	wait_controller_finished(host);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_nfc_send_cmd_readid(struct hinfc_host *host)
>> +{
>> +	hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM);
>> +	hinfc_write(host, NAND_CMD_READID, HINFC504_CMD);
>> +	hinfc_write(host, 0, HINFC504_ADDRL);
>> +
>> +	hinfc_write(host, HINFC504_OP_CMD1_EN | HINFC504_OP_ADDR_EN
>> +		| HINFC504_OP_READ_DATA_EN
>> +		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
>> +			<< HINFC504_OP_NF_CS_SHIFT)
>> +		| 1 << HINFC504_OP_ADDR_CYCLE_SHIFT, HINFC504_OP);
>> +
>> +	wait_controller_finished(host);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_nfc_send_cmd_status(struct hinfc_host *host)
>> +{
>> +	hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM);
>> +	hinfc_write(host, NAND_CMD_STATUS, HINFC504_CMD);
>> +	hinfc_write(host, HINFC504_OP_CMD1_EN
>> +		| HINFC504_OP_READ_DATA_EN
>> +		| ((host->chipselect & HINFC504_OP_NF_CS_MASK)
>> +			<< HINFC504_OP_NF_CS_SHIFT),
>> +		HINFC504_OP);
>> +
>> +	wait_controller_finished(host);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_nfc_send_cmd_reset(struct hinfc_host *host, int chipselect)
>> +{
>> +	hinfc_write(host, NAND_CMD_RESET, HINFC504_CMD);
>> +
>> +	hinfc_write(host, HINFC504_OP_CMD1_EN
>> +		| ((chipselect & HINFC504_OP_NF_CS_MASK)
>> +			<< HINFC504_OP_NF_CS_SHIFT)
>> +		| HINFC504_OP_WAIT_READY_EN,
>> +		HINFC504_OP);
>> +
>> +	wait_controller_finished(host);
>> +
>> +	return 0;
>> +}
>> +
>> +static void hisi_nfc_select_chip(struct mtd_info *mtd, int chipselect)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +
>> +	if (chipselect < 0)
>> +		return;
>> +
>> +	host->chipselect = chipselect;
>> +}
>> +
>> +static uint8_t hisi_nfc_read_byte(struct mtd_info *mtd)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +
>> +	if (host->command == NAND_CMD_STATUS)
>> +		return readb(chip->IO_ADDR_R);
>> +
>> +	host->offset++;
>> +
>> +	if (host->command == NAND_CMD_READID)
>> +		return readb(chip->IO_ADDR_R + host->offset - 1);
>> +
>> +	return readb(host->buffer + host->offset - 1);
> 
> You're reading from memory, not IO. You don't need readb().

Yes, I will get the value directly. Thanks!

> 
>> +}
>> +
>> +static u16 hisi_nfc_read_word(struct mtd_info *mtd)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +
>> +	host->offset += 2;
>> +	return readw(host->buffer + host->offset - 2);
> 
> Same here, for readw().

Thanks!

> 
>> +}
>> +
>> +static void
>> +hisi_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +
>> +	memcpy(host->buffer + host->offset, buf, len);
>> +	host->offset += len;
>> +}
>> +
>> +static void hisi_nfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +
>> +	memcpy(buf, host->buffer + host->offset, len);
>> +	host->offset += len;
>> +}
>> +
>> +static void set_addr(struct mtd_info *mtd, int column, int page_addr)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +	unsigned int command = host->command;
>> +
>> +	host->addr_cycle    = 0;
>> +	host->addr_value[0] = 0;
>> +	host->addr_value[1] = 0;
>> +
>> +	/* Serially input address */
>> +	if (column != -1) {
>> +		/* Adjust columns for 16 bit buswidth */
>> +		if (chip->options & NAND_BUSWIDTH_16 &&
>> +				!nand_opcode_8bits(command))
>> +			column >>= 1;
>> +
>> +		host->addr_value[0] = column & 0xffff;
>> +		host->addr_cycle    = 2;
>> +	}
>> +	if (page_addr != -1) {
>> +		host->addr_value[0] |= (page_addr & 0xffff)
>> +			<< (host->addr_cycle * 8);
>> +		host->addr_cycle    += 2;
>> +		/* One more address cycle for devices > 128MiB */
>> +		if (chip->chipsize > (128 << 20)) {
>> +			host->addr_cycle += 1;
>> +			if (host->command == NAND_CMD_ERASE1)
>> +				host->addr_value[0] |= ((page_addr >> 16) & 0xff) << 16;
>> +			else
>> +				host->addr_value[1] |= ((page_addr >> 16) & 0xff);
>> +		}
>> +	}
>> +}
>> +
>> +static void hisi_nfc_cmdfunc(struct mtd_info *mtd, unsigned command, int column,
>> +		int page_addr)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct hinfc_host *host = chip->priv;
>> +	int is_cache_invalid = 1;
>> +	unsigned int flag = 0;
>> +
>> +	host->command =  command;
>> +
>> +	switch (command) {
>> +	case NAND_CMD_READ0:
>> +	case NAND_CMD_READOOB:
>> +		if (command == NAND_CMD_READ0)
>> +			host->offset = column;
>> +		else
>> +			host->offset = column + mtd->writesize;
>> +
>> +		is_cache_invalid = 0;
>> +		set_addr(mtd, column, page_addr);
>> +		hisi_nfc_send_cmd_readstart(host);
>> +		break;
>> +
>> +	case NAND_CMD_SEQIN:
>> +		host->offset = column;
>> +		set_addr(mtd, column, page_addr);
>> +		break;
>> +
>> +	case NAND_CMD_ERASE1:
>> +		set_addr(mtd, column, page_addr);
>> +		break;
>> +
>> +	case NAND_CMD_PAGEPROG:
>> +		hisi_nfc_send_cmd_pageprog(host);
>> +		break;
>> +
>> +	case NAND_CMD_ERASE2:
>> +		hisi_nfc_send_cmd_erase(host);
>> +		break;
>> +
>> +	case NAND_CMD_READID:
>> +		host->offset = column;
>> +		memset(chip->IO_ADDR_R, 0, 0x10);
>> +		hisi_nfc_send_cmd_readid(host);
>> +		break;
>> +
>> +	case NAND_CMD_STATUS:
>> +		flag = hinfc_read(host, HINFC504_CON);
>> +		if (chip->ecc.mode == NAND_ECC_HW)
>> +			hinfc_write(host,
>> +				    flag && ~(HINFC504_CON_ECCTYPE_MASK <<
>> +				    HINFC504_CON_ECCTYPE_SHIFT), HINFC504_CON);
>> +
>> +		host->offset = 0;
>> +		memset(chip->IO_ADDR_R, 0, 0x10);
>> +		hisi_nfc_send_cmd_status(host);
>> +		hinfc_write(host, flag, HINFC504_CON);
>> +		break;
>> +
>> +	case NAND_CMD_RESET:
>> +		hisi_nfc_send_cmd_reset(host, host->chipselect);
>> +		break;
>> +
>> +	default:
>> +		dev_err(host->dev, "Error: unsupported cmd(cmd=%x, col=%x, page=%x)\n",
>> +			command, column, page_addr);
>> +	}
>> +
>> +	if (is_cache_invalid) {
>> +		host->cache_addr_value[0] = ~0;
>> +		host->cache_addr_value[1] = ~0;
>> +	}
>> +}
>> +
>> +static irqreturn_t hinfc_irq_handle(int irq, void *devid)
>> +{
>> +	struct hinfc_host *host = devid;
>> +	unsigned int flag;
>> +
>> +	flag = hinfc_read(host, HINFC504_INTS);
>> +	/* store interrupts state */
>> +	host->irq_status |= flag;
>> +
>> +	if (flag & HINFC504_INTS_DMA) {
>> +		hinfc_write(host, HINFC504_INTCLR_DMA, HINFC504_INTCLR);
>> +		complete(&host->cmd_complete);
>> +	} else if (flag & HINFC504_INTS_CE) {
>> +		hinfc_write(host, HINFC504_INTCLR_CE, HINFC504_INTCLR);
>> +	} else if (flag & HINFC504_INTS_UE) {
>> +		hinfc_write(host, HINFC504_INTCLR_UE, HINFC504_INTCLR);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int hisi_nand_read_page_hwecc(struct mtd_info *mtd,
>> +	struct nand_chip *chip, uint8_t *buf, int oob_required, int page)
>> +{
>> +	struct hinfc_host *host = chip->priv;
>> +	int max_bitflips = 0, stat = 0, stat_max, status_ecc;
>> +	int stat_1, stat_2;
>> +
>> +	chip->read_buf(mtd, buf, mtd->writesize);
>> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>> +
>> +	/* errors which can not be corrected by ECC */
>> +	if (host->irq_status & HINFC504_INTS_UE) {
>> +		mtd->ecc_stats.failed++;
>> +	} else if (host->irq_status & HINFC504_INTS_CE) {
>> +		/* TODO: need add other ECC modes! */
>> +		switch (chip->ecc.strength) {
>> +		case 1:
>> +			stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>>
>> +						HINFC504_ECC_1_BIT_SHIFT);
>> +			stat_max = 1;
>> +			break;
>> +		case 16:
>> +			status_ecc = hinfc_read(host, HINFC504_ECC_STATUS) >>
>> +					HINFC504_ECC_16_BIT_SHIFT & 0x0fff;
>> +			stat_2 = status_ecc & 0x3f;
>> +			stat_1 = status_ecc >> 6 & 0x3f;
>> +			stat = stat_1 + stat_2;
>> +			stat_max = max_t(int, stat_1, stat_2);
>> +		}
>> +		mtd->ecc_stats.corrected += stat;
>> +		max_bitflips = max_t(int, max_bitflips, stat_max);
>> +	}
>> +	host->irq_status = 0;
>> +
>> +	return max_bitflips;
>> +}
>> +
>> +static int hisi_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>> +				int page)
>> +{
>> +	struct hinfc_host *host = chip->priv;
>> +
>> +	chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
>> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>> +
>> +	if (host->irq_status & HINFC504_INTS_UE) {
>> +		host->irq_status = 0;
>> +		return -EBADMSG;
>> +	}
>> +
>> +	host->irq_status = 0;
>> +	return 0;
>> +}
>> +
>> +static int hisi_nand_write_page_hwecc(struct mtd_info *mtd,
>> +		struct nand_chip *chip, const uint8_t *buf, int oob_required)
>> +{
>> +	chip->write_buf(mtd, buf, mtd->writesize);
>> +	if (oob_required)
>> +		chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
>> +
>> +	return 0;
>> +}
>> +
>> +static void hisi_nfc_host_init(struct hinfc_host *host)
>> +{
>> +	struct nand_chip *chip = &host->chip;
>> +	unsigned int flag = 0;
>> +
>> +	host->version = hinfc_read(host, HINFC_VERSION);
>> +	host->addr_cycle		= 0;
>> +	host->addr_value[0]		= 0;
>> +	host->addr_value[1]		= 0;
>> +	host->cache_addr_value[0]	= ~0;
>> +	host->cache_addr_value[1]	= ~0;
>> +	host->chipselect		= 0;
>> +
>> +	/* default page size: 2K, ecc_none. need modify */
>> +	flag = HINFC504_CON_OP_MODE_NORMAL | HINFC504_CON_READY_BUSY_SEL
>> +		| ((0x001 & HINFC504_CON_PAGESIZE_MASK)
>> +			<< HINFC504_CON_PAGEISZE_SHIFT)
>> +		| ((0x0 & HINFC504_CON_ECCTYPE_MASK)
>> +			<< HINFC504_CON_ECCTYPE_SHIFT)
>> +		| ((chip->options & NAND_BUSWIDTH_16) ?
>> +			HINFC504_CON_BUS_WIDTH : 0);
>> +	hinfc_write(host, flag, HINFC504_CON);
>> +
>> +	memset(chip->IO_ADDR_R, 0xff, HINFC504_BUFFER_BASE_ADDRESS_LEN);
>> +
>> +	hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH,
>> +		    HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH);
>> +
>> +	/* enable DMA irq */
>> +	hinfc_write(host, HINFC504_INTEN_DMA, HINFC504_INTEN);
>> +}
>> +
>> +static struct nand_ecclayout nand_ecc_2K_1bit = {
>> +	.oobfree = { {24, 40} }
>> +};
>> +
>> +static struct nand_ecclayout nand_ecc_2K_16bits = {
>> +	.oobavail = 6,
>> +	.oobfree = { {2, 6} },
>> +};
>> +
>> +static int hisi_nfc_ecc_probe(struct hinfc_host *host)
>> +{
>> +	unsigned int flag;
>> +	int size, strength, ecc_bits;
>> +	struct device *dev = host->dev;
>> +	struct nand_chip *chip = &host->chip;
>> +	struct mtd_info *mtd = &host->mtd;
>> +	struct device_node *np = host->dev->of_node;
>> +
>> +	size = of_get_nand_ecc_step_size(np);
>> +	strength = of_get_nand_ecc_strength(np);
>> +	if ((size != 512) && (size != 1024)) {
>> +		dev_err(dev, "error ecc size: %d\n", size);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if ((size == 512) && (strength != 1)) {
>> +		dev_err(dev, "ecc size and strength do not match\n");
>> +		return -EINVAL;
>> +	}
>> +	if ((size == 1024) && ((strength != 8) && (strength != 16) &&
>> +				(strength != 24) && (strength != 40))) {
>> +		dev_err(dev, "ecc size and strength do not match\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	chip->ecc.size = size;
>> +	chip->ecc.strength = strength;
>> +
>> +	chip->ecc.read_page = hisi_nand_read_page_hwecc;
>> +	chip->ecc.read_oob = hisi_nand_read_oob;
>> +	chip->ecc.write_page = hisi_nand_write_page_hwecc;
>> +
>> +	switch (chip->ecc.strength) {
>> +	case 1:
>> +		ecc_bits = 1;
>> +		if (mtd->writesize == 2048)
>> +			chip->ecc.layout = &nand_ecc_2K_1bit;
>> +
>> +		/* TODO: add more page size support */
>> +		break;
>> +	case 16:
>> +		ecc_bits = 6;
>> +		if (mtd->writesize == 2048)
>> +			chip->ecc.layout = &nand_ecc_2K_16bits;
>> +
>> +		/* TODO: add more page size support */
>> +		break;
>> +
>> +	/* TODO: add more ecc strength support */
>> +	default:
>> +		dev_err(dev, "not support strength: %d\n", chip->ecc.strength);
>> +		return -EINVAL;
>> +	}
>> +
>> +	flag = hinfc_read(host, HINFC504_CON);
>> +	/* add ecc type configure */
>> +	flag |= ((ecc_bits & HINFC504_CON_ECCTYPE_MASK)
>> +						<< HINFC504_CON_ECCTYPE_SHIFT);
>> +	hinfc_write(host, flag, HINFC504_CON);
>> +
>> +	/* enable ecc irq */
>> +	flag = hinfc_read(host, HINFC504_INTEN) & 0xfff;
>> +	hinfc_write(host, flag | HINFC504_INTEN_UE | HINFC504_INTEN_CE,
>> +		    HINFC504_INTEN);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hisi_nfc_probe(struct platform_device *pdev)
>> +{
>> +	int ret = 0, irq, buswidth, flag, max_chips = HINFC504_MAX_CHIP;
>> +	struct device *dev = &pdev->dev;
>> +	struct hinfc_host *host;
>> +	struct nand_chip  *chip;
>> +	struct mtd_info   *mtd;
>> +	struct resource	  *res;
>> +	struct device_node *np = dev->of_node;
>> +	struct mtd_part_parser_data ppdata;
>> +
>> +	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
>> +	if (!host)
>> +		return -ENOMEM;
>> +	host->dev = dev;
>> +
>> +	platform_set_drvdata(pdev, host);
>> +	chip = &host->chip;
>> +	mtd  = &host->mtd;
>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +	if (irq < 0) {
>> +		dev_err(dev, "no IRQ resource defined\n");
>> +		ret = -ENXIO;
>> +		goto err_res;
>> +	}
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	host->iobase = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(host->iobase)) {
>> +		ret = PTR_ERR(host->iobase);
>> +		dev_err(dev, "devm_ioremap_resource[0] fail\n");
> 
> I don't think yhou really need this error print. devm_ioremap_resource()
> should be descriptive enough, I think.

Will delete the error print here, thanks!

> 
>> +		goto err_res;
>> +	}
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res);
> 
> Hmm, do you really have to reuse IO_ADDR_{R,W} here? Those are only
> targeted for NAND systems which have a direct MMIO mapping to the NAND
> I/O pins. See nand_base's {read,write}_buf() and read_{byte,word}()
> implementations. But you override those.

There is a hardware buffer in this NAND controller, and the buffer can be
accessed as MMIO. IO_ADDR_R/W just indicates the base address of this buffer.
Maybe I need to use a void __iomem pointer stored in my host struct to use
this buffer instead of IO_ADDR_R/W as you said below here?

> 
> It's best if it's obvious if nand_base is somehow inadvertently using
> these pointers. So leaving them NULL is helpful.
> 
> As an alternative, you can just stash another private void __iomem
> pointer in you your host struct.
> 
>> +	if (IS_ERR(chip->IO_ADDR_R)) {
>> +		ret = PTR_ERR(chip->IO_ADDR_R);
>> +		dev_err(dev, "devm_ioremap_resource[1] fail\n");
>> +		goto err_res;
>> +	}
>> +
>> +	mtd->priv		= chip;
>> +	mtd->owner		= THIS_MODULE;
>> +	mtd->name		= "hisi_nand";
>> +	mtd->dev.parent         = &pdev->dev;
>> +
>> +	chip->priv		= host;
>> +	chip->cmdfunc		= hisi_nfc_cmdfunc;
>> +	chip->select_chip	= hisi_nfc_select_chip;
>> +	chip->read_byte		= hisi_nfc_read_byte;
>> +	chip->read_word		= hisi_nfc_read_word;
>> +	chip->write_buf		= hisi_nfc_write_buf;
>> +	chip->read_buf		= hisi_nfc_read_buf;
>> +	chip->chip_delay	= HINFC504_CHIP_DELAY;
>> +
>> +	chip->ecc.mode = of_get_nand_ecc_mode(np);
>> +
>> +	buswidth = of_get_nand_bus_width(np);
>> +	if (buswidth == 16)
>> +		chip->options |= NAND_BUSWIDTH_16;
>> +
>> +	hisi_nfc_host_init(host);
>> +
>> +	ret = devm_request_irq(dev, irq, hinfc_irq_handle, IRQF_DISABLED,
>> +				"nandc", host);
>> +	if (ret) {
>> +		dev_err(dev, "failed to request IRQ\n");
>> +		goto err_res;
>> +	}
>> +
>> +	ret = nand_scan_ident(mtd, max_chips, NULL);
>> +	if (ret) {
>> +		ret = -ENODEV;
>> +		goto err_res;
>> +	}
>> +
>> +	host->buffer = dmam_alloc_coherent(dev, mtd->writesize + mtd->oobsize,
>> +		&host->dma_buffer, GFP_KERNEL);
> 
> You need to check this for allocation failures.
>

Will add relative code to check this, thanks!

>> +	host->dma_oob = host->dma_buffer + mtd->writesize;
>> +	memset(host->buffer, 0xff, mtd->writesize + mtd->oobsize);
>> +
>> +	flag = hinfc_read(host, HINFC504_CON);
>> +	flag &= ~(HINFC504_CON_PAGESIZE_MASK << HINFC504_CON_PAGEISZE_SHIFT);
>> +	switch (mtd->writesize) {
>> +	case 2048:
>> +		flag |= (0x001 << HINFC504_CON_PAGEISZE_SHIFT); break;
>> +	/*
>> +	 * TODO: add more pagesize support,
>> +	 * default pagesize has been set in hisi_nfc_host_init
>> +	 */
>> +	default:
>> +		dev_err(dev, "NON-2KB page size nand flash\n");
>> +		ret = -EINVAL;
>> +		goto err_res;
>> +	}
>> +	hinfc_write(host, flag, HINFC504_CON);
>> +
>> +	if (chip->ecc.mode == NAND_ECC_HW)
>> +		hisi_nfc_ecc_probe(host);
>> +
>> +	ret = nand_scan_tail(mtd);
>> +	if (ret) {
>> +		dev_err(dev, "nand_scan_tail failed: %d\n", ret);
>> +		goto err_res;
>> +	}
>> +
>> +	ppdata.of_node = np;
>> +	ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Err MTD partition=%d\n", ret);
>> +		goto err_mtd;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_mtd:
>> +	nand_release(mtd);
>> +err_res:
>> +	return ret;
>> +}
>> +
>> +static int hisi_nfc_remove(struct platform_device *pdev)
>> +{
>> +	struct hinfc_host *host = platform_get_drvdata(pdev);
>> +	struct mtd_info *mtd = &host->mtd;
>> +
>> +	nand_release(mtd);
>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int hisi_nfc_suspend(struct device *dev)
>> +{
>> +	struct hinfc_host *host = dev_get_drvdata(dev);
>> +	unsigned long timeout = jiffies + HINFC504_NFC_PM_TIMEOUT;
>> +
>> +	while (time_before(jiffies, timeout)) {
>> +		if (((hinfc_read(host, HINFC504_STATUS) & 0x1) == 0x0) &&
>> +		    (hinfc_read(host, HINFC504_DMA_CTRL) &
>> +		     HINFC504_DMA_CTRL_DMA_START)) {
>> +			_cond_resched();
> 
> Why not just cond_resched()?
>

It is better to just use cond_resched(), will modify this. Thanks!

>> +			return 0;
>> +		}
>> +	}
>> +
>> +	dev_err(host->dev, "nand controller suspend timeout.\n");
>> +
>> +	return -EAGAIN;
>> +}
>> +
>> +static int hisi_nfc_resume(struct device *dev)
>> +{
>> +	int cs;
>> +	struct hinfc_host *host = dev_get_drvdata(dev);
>> +	struct nand_chip *chip = &host->chip;
>> +
>> +	for (cs = 0; cs < chip->numchips; cs++)
>> +		hisi_nfc_send_cmd_reset(host, cs);
>> +	hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH,
>> +		    HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +static SIMPLE_DEV_PM_OPS(hisi_nfc_pm_ops, hisi_nfc_suspend, hisi_nfc_resume);
>> +
>> +static const struct of_device_id nfc_id_table[] = {
>> +	{ .compatible = "hisilicon,504-nfc" },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, nfc_id_table);
>> +
>> +static struct platform_driver hisi_nfc_driver = {
>> +	.driver = {
>> +		.name  = "hisi_nand",
>> +		.of_match_table = of_match_ptr(nfc_id_table),
>> +		.pm = &hisi_nfc_pm_ops,
>> +	},
>> +	.probe		= hisi_nfc_probe,
>> +	.remove		= hisi_nfc_remove,
>> +};
>> +
>> +module_platform_driver(hisi_nfc_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Zhiyong Cai");
>> +MODULE_AUTHOR("Zhou Wang");
>> +MODULE_DESCRIPTION("Hisilicon Nand Flash Controller Driver");
> 
> Brian

Thanks again for your comments.
Zhou Wang

> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Brian Norris]

On Wed, Jan 14, 2015 at 08:34:39PM +0800, Zhou Wang wrote:
> On 2015/1/13 11:58, Brian Norris wrote:
> > On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> >> +	chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res);
> > 
> > Hmm, do you really have to reuse IO_ADDR_{R,W} here? Those are only
> > targeted for NAND systems which have a direct MMIO mapping to the NAND
> > I/O pins. See nand_base's {read,write}_buf() and read_{byte,word}()
> > implementations. But you override those.
> 
> There is a hardware buffer in this NAND controller, and the buffer can be
> accessed as MMIO.

Sure.

> IO_ADDR_R/W just indicates the base address of this buffer.

But I was noting that IO_ADDR_{R,W} actually serve a very particular
purpose in nand_base.c, which seems distinct from your HW buffer.

> Maybe I need to use a void __iomem pointer stored in my host struct to use
> this buffer instead of IO_ADDR_R/W as you said below here?

Yes, I think that would be better.

> > It's best if it's obvious if nand_base is somehow inadvertently using
> > these pointers. So leaving them NULL is helpful.
> > 
> > As an alternative, you can just stash another private void __iomem
> > pointer in you your host struct.

Thanks,
Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Zhou Wang]

On 2015/1/15 3:25, Brian Norris wrote:
> On Wed, Jan 14, 2015 at 08:34:39PM +0800, Zhou Wang wrote:
>> On 2015/1/13 11:58, Brian Norris wrote:
>>> On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>>> +	chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res);
>>>
>>> Hmm, do you really have to reuse IO_ADDR_{R,W} here? Those are only
>>> targeted for NAND systems which have a direct MMIO mapping to the NAND
>>> I/O pins. See nand_base's {read,write}_buf() and read_{byte,word}()
>>> implementations. But you override those.
>>
>> There is a hardware buffer in this NAND controller, and the buffer can be
>> accessed as MMIO.
> 
> Sure.
> 
>> IO_ADDR_R/W just indicates the base address of this buffer.
> 
> But I was noting that IO_ADDR_{R,W} actually serve a very particular
> purpose in nand_base.c, which seems distinct from your HW buffer.
> 
>> Maybe I need to use a void __iomem pointer stored in my host struct to use
>> this buffer instead of IO_ADDR_R/W as you said below here?
> 
> Yes, I think that would be better.

OK, I will do as this in next version, Thanks!

> 
>>> It's best if it's obvious if nand_base is somehow inadvertently using
>>> these pointers. So leaving them NULL is helpful.
>>>
>>> As an alternative, you can just stash another private void __iomem
>>> pointer in you your host struct.
> 
> Thanks,
> Brian

Thanks for your reply!
Zhou Wang

> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Brian Norris]

One more thing:

On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
> diff --git a/drivers/mtd/nand/hisi504_nand.c b/drivers/mtd/nand/hisi504_nand.c
> new file mode 100644
> index 0000000..2000f21
> --- /dev/null
> +++ b/drivers/mtd/nand/hisi504_nand.c
...
> +static int hisi_nand_read_page_hwecc(struct mtd_info *mtd,
> +	struct nand_chip *chip, uint8_t *buf, int oob_required, int page)
> +{
> +	struct hinfc_host *host = chip->priv;
> +	int max_bitflips = 0, stat = 0, stat_max, status_ecc;

    drivers/mtd/nand/hisi504_nand.c:547:34: warning: ‘stat_max’ may be used uninitialized in this function [-Wmaybe-uninitialized]
      int max_bitflips = 0, stat = 0, stat_max, status_ecc;
                                      ^

> +	int stat_1, stat_2;
> +
> +	chip->read_buf(mtd, buf, mtd->writesize);
> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	/* errors which can not be corrected by ECC */
> +	if (host->irq_status & HINFC504_INTS_UE) {
> +		mtd->ecc_stats.failed++;
> +	} else if (host->irq_status & HINFC504_INTS_CE) {
> +		/* TODO: need add other ECC modes! */

You may want a 'default' case that sets stat_max to zero, then.

> +		switch (chip->ecc.strength) {
> +		case 1:
> +			stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>>
> +						HINFC504_ECC_1_BIT_SHIFT);
> +			stat_max = 1;
> +			break;
> +		case 16:
> +			status_ecc = hinfc_read(host, HINFC504_ECC_STATUS) >>
> +					HINFC504_ECC_16_BIT_SHIFT & 0x0fff;
> +			stat_2 = status_ecc & 0x3f;
> +			stat_1 = status_ecc >> 6 & 0x3f;
> +			stat = stat_1 + stat_2;
> +			stat_max = max_t(int, stat_1, stat_2);
> +		}
> +		mtd->ecc_stats.corrected += stat;
> +		max_bitflips = max_t(int, max_bitflips, stat_max);
> +	}
> +	host->irq_status = 0;
> +
> +	return max_bitflips;
> +}

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] mtd: hisilicon: add a new NAND controller driver for hisilicon hip04 Soc

[Zhou Wang]

On 2015/1/13 12:02, Brian Norris wrote:
> One more thing:
> 
> On Mon, Jan 12, 2015 at 03:28:53PM +0800, Zhou Wang wrote:
>> diff --git a/drivers/mtd/nand/hisi504_nand.c b/drivers/mtd/nand/hisi504_nand.c
>> new file mode 100644
>> index 0000000..2000f21
>> --- /dev/null
>> +++ b/drivers/mtd/nand/hisi504_nand.c
> ...
>> +static int hisi_nand_read_page_hwecc(struct mtd_info *mtd,
>> +	struct nand_chip *chip, uint8_t *buf, int oob_required, int page)
>> +{
>> +	struct hinfc_host *host = chip->priv;
>> +	int max_bitflips = 0, stat = 0, stat_max, status_ecc;
> 
>     drivers/mtd/nand/hisi504_nand.c:547:34: warning: ‘stat_max’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>       int max_bitflips = 0, stat = 0, stat_max, status_ecc;

In fact, there is no chance to run "default case" there as hisi_nfc_ecc_probe
will check this and print error.
I will initialize stat_max with 0 to eliminate this warning.

>                                       ^
> 
>> +	int stat_1, stat_2;
>> +
>> +	chip->read_buf(mtd, buf, mtd->writesize);
>> +	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>> +
>> +	/* errors which can not be corrected by ECC */
>> +	if (host->irq_status & HINFC504_INTS_UE) {
>> +		mtd->ecc_stats.failed++;
>> +	} else if (host->irq_status & HINFC504_INTS_CE) {
>> +		/* TODO: need add other ECC modes! */
> 
> You may want a 'default' case that sets stat_max to zero, then.
> 
>> +		switch (chip->ecc.strength) {
>> +		case 1:
>> +			stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>>
>> +						HINFC504_ECC_1_BIT_SHIFT);
>> +			stat_max = 1;
>> +			break;
>> +		case 16:
>> +			status_ecc = hinfc_read(host, HINFC504_ECC_STATUS) >>
>> +					HINFC504_ECC_16_BIT_SHIFT & 0x0fff;
>> +			stat_2 = status_ecc & 0x3f;
>> +			stat_1 = status_ecc >> 6 & 0x3f;
>> +			stat = stat_1 + stat_2;
>> +			stat_max = max_t(int, stat_1, stat_2);
>> +		}
>> +		mtd->ecc_stats.corrected += stat;
>> +		max_bitflips = max_t(int, max_bitflips, stat_max);
>> +	}
>> +	host->irq_status = 0;
>> +
>> +	return max_bitflips;
>> +}
> 
> Brian

Thanks for your comment!
Zhou Wang
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 2/2] mtd: hisilicon: add device tree binding documentation

[Zhou Wang]

Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 .../devicetree/bindings/mtd/hisi504-nand.txt       | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/hisi504-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/hisi504-nand.txt b/Documentation/devicetree/bindings/mtd/hisi504-nand.txt
new file mode 100644
index 0000000..bee6a9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/hisi504-nand.txt
@@ -0,0 +1,48 @@
+Hisilicon Hip04 Soc NAND controller DT binding
+
+Required properties:
+
+- compatible:          Should be "hisilicon,504-nfc".
+- reg:                 The first contains base physical address and size of
+                       NAND controller's registers. The second contains base
+                       physical address and size of NAND controller's buffer.
+- interrupts:          Interrupt number for nfc.
+- nand-bus-width:      See nand.txt.
+- nand-ecc-mode:       Support none and hw ecc mode.
+- #address-cells:      Partition address, should be set 1.
+- #size-cells:         Partition size, should be set 1.
+
+Optional properties:
+
+- nand-ecc-strength:   Number of bits to correct per ECC step.
+- nand-ecc-step-size:  Number of data bytes covered by a single ECC step.
+
+The following ECC strength and step size are currently supported:
+
+ - nand-ecc-strength = <1>, nand-ecc-step-size = <512>
+ - nand-ecc-strength = <16>, nand-ecc-step-size = <1024>
+
+Flash chip may optionally contain additional sub-nodes describing partitions of
+the address space. See partition.txt for more detail.
+
+Example:
+
+	nand: nand@4020000 {
+		compatible = "hisilicon,504-nfc";
+		reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
+		interrupts = <0 379 4>;
+		nand-bus-width = <8>;
+		nand-ecc-mode = "hw";
+		nand-ecc-strength = <16>;
+		nand-ecc-step-size = <1024>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition@0 {
+			label = "nand_text";
+			reg = <0x00000000 0x00400000>;
+		};
+
+		...
+
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html