linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc
@ 2014-06-30  8:03 Zhou Wang
  2014-06-30  8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Zhou Wang @ 2014-06-30  8:03 UTC (permalink / raw)
  To: linux-arm-kernel

This series patches add the support for NAND controller of hisilicon
hip04 Soc. These patches are base on branch integration-hilt-working-v3.14 
in linaro landing-team git repository[1].

The NAND controller IP was developed by hisilicon and need a new driver
to support. The driver is usable and I tested it on hisilicon hip04-d01
board.

[1] ssh://git at git.linaro.org/landing-teams/working/hisilicon/kernel.git

Zhou Wang (3):
  mtd: hisilicon: add device tree node for nand controller
  mtd: hisilicon: add a new nand controller driver for hisilicon hip04
    Soc
  mtd: hisilicon: add device tree binding documentation

 .../devicetree/bindings/mtd/hisi-nand.txt          |   38 +
 arch/arm/boot/dts/hip04.dtsi                       |   31 +
 drivers/mtd/nand/Kconfig                           |    5 +
 drivers/mtd/nand/Makefile                          |    1 +
 drivers/mtd/nand/hisi_nand.c                       |  847 ++++++++++++++++++++
 5 files changed, 922 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt
 create mode 100644 drivers/mtd/nand/hisi_nand.c

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller
  2014-06-30  8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang
@ 2014-06-30  8:03 ` Zhou Wang
  2014-07-09  7:08   ` Jerome FORISSIER
  2014-06-30  8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang
  2014-06-30  8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang
  2 siblings, 1 reply; 16+ messages in thread
From: Zhou Wang @ 2014-06-30  8:03 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 arch/arm/boot/dts/hip04.dtsi |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
index abb42ca..e63fc61 100644
--- a/arch/arm/boot/dts/hip04.dtsi
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -386,5 +386,36 @@
 				interrupts = <0 389 4>;
 			};
 		};
+
+		nand: nand at 4020000 {
+			compatible = "hisilicon,504-nfc";
+			reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
+			interrupts = <0 379 4>;
+			nand-bus-width = <8>;
+			nand-ecc-mode = "none";
+			hisi,nand-ecc-bits = <0>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			partition at 0 {
+				label = "nand_text";
+				reg = <0x00000000 0x00400000>;
+			};
+
+			partition at 00400000 {
+				label = "nand_monitor";
+				reg = <0x00400000 0x00400000>;
+			};
+
+			partition at 00800000 {
+				label = "nand_kernel";
+				reg = <0x00800000 0x00800000>;
+			};
+
+			partition at 01000000 {
+				label = "nand_fs";
+				reg = <0x01000000 0x1f000000>;
+			};
+		};
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang
  2014-06-30  8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang
@ 2014-06-30  8:03 ` Zhou Wang
  2014-06-30  9:00   ` Arnd Bergmann
                     ` (2 more replies)
  2014-06-30  8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang
  2 siblings, 3 replies; 16+ messages in thread
From: Zhou Wang @ 2014-06-30  8:03 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 drivers/mtd/nand/Kconfig     |    5 +
 drivers/mtd/nand/Makefile    |    1 +
 drivers/mtd/nand/hisi_nand.c |  847 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 853 insertions(+)
 create mode 100644 drivers/mtd/nand/hisi_nand.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 90ff447..253f8c8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -510,4 +510,9 @@ config MTD_NAND_XWAY
 	  Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached
 	  to the External Bus Unit (EBU).
 
+config MTD_NAND_HISI
+	tristate "Support for NAND controller on Hisilicon SoC"
+	help
+	  Enables support for NAND controller on Hisilicon SoC.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 542b568..d0881cf 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740)		+= jz4740_nand.o
 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_HISI)	        += hisi_nand.o
 
 nand-objs := nand_base.o nand_bbt.o
diff --git a/drivers/mtd/nand/hisi_nand.c b/drivers/mtd/nand/hisi_nand.c
new file mode 100644
index 0000000..fbcb065
--- /dev/null
+++ b/drivers/mtd/nand/hisi_nand.c
@@ -0,0 +1,847 @@
+/*
+ * Hisilicon NAND Flash controller driver
+ *
+ * Copyright ? 2012-2014 HiSilicon Technologies Co., Ltd.
+ *              http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou.bry@gmail.com>
+ * The initial developer of the original code is Zhiyong Cai
+ * <caizhiyong@huawei.com>
+ *
+ * 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_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			(1U << 0)
+#define HINFC504_CON_PAGEISZE_SHIFT			(1)
+#define HINFC504_CON_PAGESIZE_MASK			(0x07)
+#define HINFC504_CON_BUS_WIDTH				(1U << 4)
+#define HINFC504_CON_READY_BUSY_SEL			(1U << 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			(1U << 1)
+#define HINFC504_OP_WAIT_READY_EN			(1U << 2)
+#define HINFC504_OP_CMD2_EN				(1U << 3)
+#define HINFC504_OP_WRITE_DATA_EN			(1U << 4)
+#define HINFC504_OP_ADDR_EN				(1U << 5)
+#define HINFC504_OP_CMD1_EN				(1U << 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					(1U << 0)
+
+#define HINFC504_INTEN					0x24
+#define HINFC504_INTEN_DMA				(1U << 9)
+#define HINFC504_INTEN_UE				(1U << 6)
+#define HINFC504_INTEN_CE				(1U << 5)
+
+#define HINFC504_INTS					0x28
+#define HINFC504_INTS_DMA				(1U << 9)
+#define HINFC504_INTS_UE				(1U << 6)
+#define HINFC504_INTS_CE				(1U << 5)
+
+#define HINFC504_INTCLR					0x2C
+#define HINFC504_INTCLR_DMA				(1U << 9)
+#define HINFC504_INTCLR_UE				(1U << 6)
+#define HINFC504_INTCLR_CE				(1U << 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			(1U << 0)
+#define HINFC504_DMA_CTRL_WE				(1U << 1)
+#define HINFC504_DMA_CTRL_DATA_AREA_EN			(1U << 2)
+#define HINFC504_DMA_CTRL_OOB_AREA_EN			(1U << 3)
+#define HINFC504_DMA_CTRL_BURST4_EN			(1U << 4)
+#define HINFC504_DMA_CTRL_BURST8_EN			(1U << 5)
+#define HINFC504_DMA_CTRL_BURST16_EN			(1U << 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			(1U << 0)
+#define HINFC504_DMA_PARA_OOB_RW_EN			(1U << 1)
+#define HINFC504_DMA_PARA_DATA_EDC_EN			(1U << 2)
+#define HINFC504_DMA_PARA_OOB_EDC_EN			(1U << 3)
+#define HINFC504_DMA_PARA_DATA_ECC_EN			(1U << 4)
+#define HINFC504_DMA_PARA_OOB_ECC_EN			(1U << 5)
+
+#define HINFC_VERSION                                   0x74
+#define HINFC504_LOG_READ_ADDR				0x7C
+#define HINFC504_LOG_READ_LEN				0x80
+
+#define HINFC504_NANDINFO_LEN				0x10
+
+#define hinfc_read(_host, _reg)	readl(_host->iobase + (_reg))
+#define hinfc_write(_host, _value, _reg)\
+	writel((_value), _host->iobase + (_reg))
+
+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;
+	unsigned int		addr_value[2];
+	unsigned int		cache_addr_value[2];
+	char			*buffer;
+	dma_addr_t		dma_buffer;
+	dma_addr_t		dma_oob;
+	int			version;
+	unsigned int            ecc_bits;
+	unsigned int            irq_status; /* interrupt status */
+
+	int (*send_cmd_pageprog)(struct hinfc_host *host);
+	int (*send_cmd_status)(struct hinfc_host *host);
+	int (*send_cmd_readstart)(struct hinfc_host *host);
+	int (*send_cmd_erase)(struct hinfc_host *host);
+	int (*send_cmd_readid)(struct hinfc_host *host);
+	int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
+};
+
+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;
+		} else {
+			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
+		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;
+
+	hinfc_write(host, val, HINFC504_DMA_CTRL);
+
+	init_completion(&host->cmd_complete);
+	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!");
+		else
+			dev_err(host->dev, "dma is really timeout!");
+	}
+}
+
+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;
+
+	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)
+			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);
+		host->send_cmd_readstart(host);
+		break;
+
+	case NAND_CMD_SEQIN:
+		host->offset = column;
+
+	case NAND_CMD_ERASE1:
+		set_addr(mtd, column, page_addr);
+		break;
+
+	case NAND_CMD_PAGEPROG:
+		host->send_cmd_pageprog(host);
+		break;
+
+	case NAND_CMD_ERASE2:
+		host->send_cmd_erase(host);
+		break;
+
+	case NAND_CMD_READID:
+		host->offset = column;
+		memset(chip->IO_ADDR_R, 0, 0x10);
+		host->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);
+		host->send_cmd_status(host);
+
+		hinfc_write(host, flag, HINFC504_CON);
+		break;
+
+	case NAND_CMD_RESET:
+		host->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;
+
+	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) {
+		/* need add other ECC modes! */
+		switch (host->ecc_bits) {
+		case 1:
+			stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>>
+						HINFC504_ECC_1_BIT_SHIFT);
+			break;
+		case 6:
+			stat = hweight16(hinfc_read(host, HINFC504_ECC_STATUS)>>
+			HINFC504_ECC_16_BIT_SHIFT & 0x0fff);
+		}
+		mtd->ecc_stats.corrected += stat;
+		max_bitflips = max_t(unsigned int, max_bitflips, stat);
+	}
+	host->irq_status = 0;
+
+	return max_bitflips;
+}
+
+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 mtd_info *mtd, struct hinfc_host *host)
+{
+	struct nand_chip *chip = mtd->priv;
+	host->version = hinfc_read(host, HINFC_VERSION);
+	unsigned int flag = 0;
+
+	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;
+
+	host->send_cmd_pageprog		= hisi_nfc_send_cmd_pageprog;
+	host->send_cmd_readstart	= hisi_nfc_send_cmd_readstart;
+	host->send_cmd_erase		= hisi_nfc_send_cmd_erase;
+	host->send_cmd_readid		= hisi_nfc_send_cmd_readid;
+	host->send_cmd_status		= hisi_nfc_send_cmd_status;
+	host->send_cmd_reset		= hisi_nfc_send_cmd_reset;
+
+	/* 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(host->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 mtd_info *mtd, struct hinfc_host *host)
+{
+	struct nand_chip *chip = host->chip;
+	unsigned int flag;
+
+	chip->ecc.read_page = hisi_nand_read_page_hwecc;
+	chip->ecc.write_page = hisi_nand_write_page_hwecc;
+
+	switch (host->ecc_bits) {
+	case 1:
+		chip->ecc.layout = &nand_ecc_2K_1bit;
+		chip->ecc.strength = 1;
+		chip->ecc.size = 512;
+		break;
+	case 6:
+		chip->ecc.layout = &nand_ecc_2K_16bits;
+		chip->ecc.strength = 16;
+		chip->ecc.size = 1024;
+	}
+
+	flag = hinfc_read(host, HINFC504_CON);
+	/* add ecc type configure */
+	flag |= ((host->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) + sizeof(*chip) + sizeof(*mtd),
+			GFP_KERNEL);
+	if (!host)
+		return -ENOMEM;
+	host->dev = dev;
+
+	platform_set_drvdata(pdev, host);
+	chip = (struct nand_chip *)&host[1];
+	mtd  = (struct mtd_info *)&chip[1];
+
+	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;
+	}
+
+	host->chip		= chip;
+	host->mtd		= mtd;
+
+	mtd->priv		= chip;
+	mtd->owner		= THIS_MODULE;
+	mtd->name		= "hisi_nand";
+
+	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);
+	/* read ecc-bits from dts */
+	of_property_read_u32(np, "hisi,nand-ecc-bits", &host->ecc_bits);
+
+	buswidth = of_get_nand_bus_width(np);
+	if (buswidth == 16)
+		chip->options |= NAND_BUSWIDTH_16;
+
+	hisi_nfc_host_init(mtd, 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_irq;
+	}
+
+	ret = nand_scan_ident(mtd, max_chips, NULL);
+	if (ret) {
+		ret = -ENODEV;
+		goto err_ident;
+	}
+
+	host->buffer = dma_alloc_coherent(dev, mtd->writesize + mtd->oobsize,
+		&host->dma_buffer, GFP_KERNEL);
+	if (!host->buffer) {
+		dev_err(dev, "Can't malloc memory for NAND driver.\n");
+		ret = -ENOMEM;
+		goto err_buf;
+	}
+	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);
+	/* add more pagesize support
+	 * default pagesize has been set in hisi_nfc_host_init
+	 */
+	}
+	hinfc_write(host, flag, HINFC504_CON);
+
+	if (chip->ecc.mode == NAND_ECC_HW)
+		hisi_nfc_ecc_probe(mtd, host);
+
+	nand_scan_tail(mtd);
+
+	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_ident:
+err_irq:
+err_buf:
+	if (host->buffer)
+		dma_free_coherent(dev, mtd->writesize + mtd->oobsize,
+				  host->buffer, host->dma_buffer);
+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(host->mtd);
+	dma_free_coherent(&pdev->dev, mtd->writesize + mtd->oobsize,
+			  host->buffer, host->dma_buffer);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int hisi_nfc_suspend(struct platform_device *pdev,
+			       pm_message_t state)
+{
+	struct hinfc_host *host = platform_get_drvdata(pdev);
+
+	while ((hinfc_read(host, HINFC504_STATUS) & 0x1) == 0x0)
+		;
+
+	while ((hinfc_read(host, HINFC504_DMA_CTRL))
+		& HINFC504_DMA_CTRL_DMA_START)
+		_cond_resched();
+
+	return 0;
+}
+
+static int hisi_nfc_resume(struct platform_device *pdev)
+{
+	int cs;
+	struct hinfc_host *host = platform_get_drvdata(pdev);
+	struct nand_chip *chip = host->chip;
+
+	for (cs = 0; cs < chip->numchips; cs++)
+		host->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,nfc504" },
+	{}
+};
+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.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] mtd: hisilicon: add device tree binding documentation
  2014-06-30  8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang
  2014-06-30  8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang
  2014-06-30  8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang
@ 2014-06-30  8:03 ` Zhou Wang
  2014-06-30  9:52   ` Mark Rutland
  2 siblings, 1 reply; 16+ messages in thread
From: Zhou Wang @ 2014-06-30  8:03 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
---
 .../devicetree/bindings/mtd/hisi-nand.txt          |   38 ++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/hisi-nand.txt b/Documentation/devicetree/bindings/mtd/hisi-nand.txt
new file mode 100644
index 0000000..1cc6470
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/hisi-nand.txt
@@ -0,0 +1,38 @@
+Hisilicon Hip04 Soc NAND controller DT binding
+
+Required properties:
+- compatible:          Should be "hisilicon,nfc504".
+- reg:                 Contain registers location and length for reg and data.
+- interrupts:          Interrupt number for nfc.
+- nand-bus-width:      See nand.txt.
+- nand-ecc-mode:       See nand.txt.
+- hisi,nand-ecc-bits:  ECC bits type support.
+		<0>:   none ecc
+		<1>:   Can correct 1bit per 512byte.
+		<6>:   Can correct 16bits per 1K byte.
+- #address-cells:      partition address.
+- #size-cells:         partition size.
+
+Flash chip may optionally contain additional sub-nodes describing partitions of
+the address space. See partition.txt for more detail.
+
+Example:
+
+	nand: nand at 4020000 {
+		compatible = "hisilicon,nfc504";
+		reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
+		interrupts = <0 379 4>;
+		nand-bus-width = <8>;
+		nand-ecc-mode = "hw";
+		hisi,nand-ecc-bits = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition at 0 {
+			label = "nand_text";
+			reg = <0x00000000 0x00400000>;
+		};
+
+		...
+
+	};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang
@ 2014-06-30  9:00   ` Arnd Bergmann
  2014-06-30  9:59     ` Caizhiyong
  2014-07-02  2:07     ` Zhou Wang
  2014-06-30  9:45   ` Ivan Khoronzhuk
  2014-06-30 10:00   ` Mark Rutland
  2 siblings, 2 replies; 16+ messages in thread
From: Arnd Bergmann @ 2014-06-30  9:00 UTC (permalink / raw)
  To: linux-arm-kernel

Overall a very nice driver. I didn't find any real bugs, but a few things
that could be changed for readability and micro-optimizations:

On Monday 30 June 2014 16:03:28 Zhou Wang wrote:

> +#define hinfc_read(_host, _reg)	readl(_host->iobase + (_reg))
> +#define hinfc_write(_host, _value, _reg)\
> +	writel((_value), _host->iobase + (_reg))

I'd recommend making these inline functions rather than macros.

> +struct hinfc_host {
> +	struct nand_chip	*chip;
> +	struct mtd_info		*mtd;

I notice that you allocate the nand_chip and mtd_info together
with the hinfc_host and then assign these pointers. A preferred
method is normally to just embed the structures in this case:

struct hinfc_host {
	struct nand_chip	chip;
	struct mtd_info		mtd;

so you avoid the pointer overhead, and can go back from these
two structures to the hinfc_host using container_of().

> +	struct device		*dev;
> +	void __iomem		*iobase;
> +	struct completion       cmd_complete;
> +	unsigned int		offset;
> +	unsigned int		command;
> +	int			chipselect;
> +	unsigned int		addr_cycle;
> +	unsigned int		addr_value[2];
> +	unsigned int		cache_addr_value[2];
> +	char			*buffer;
> +	dma_addr_t		dma_buffer;
> +	dma_addr_t		dma_oob;
> +	int			version;
> +	unsigned int            ecc_bits;
> +	unsigned int            irq_status; /* interrupt status */
> +
> +	int (*send_cmd_pageprog)(struct hinfc_host *host);
> +	int (*send_cmd_status)(struct hinfc_host *host);
> +	int (*send_cmd_readstart)(struct hinfc_host *host);
> +	int (*send_cmd_erase)(struct hinfc_host *host);
> +	int (*send_cmd_readid)(struct hinfc_host *host);
> +	int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
> +};

Why do you need function pointers here? The current version of the
driver you posted always assigns these to the same functions, so
it would be more efficient to just call those directly.

If you plan to add another variant later that uses a different set
of functions here, you can add the function pointers then, in a
separate patch. Also, a common style element is to have a separate
constant structure with the function pointers:

const struct hinfc_host_ops ops = {
	.send_cmd_pageprog = hisi_nfc_send_cmd_pageprog,
	...
};

if you actually end up needing function pointers. That simplifies
the code a bit and is slightly more robust to attacks overwriting
the function pointers.

> +
> +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;
> +		} else {
> +			if (val & HINFC504_READY)
> +				return;
> +		}
> +	}
> +
> +	/* wait cmd timeout */
> +	dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n");
> +}

Since the timeout is much bigger than the wait time here, I guess you
can use a wider range for the usleep_range(), e.g.

	usleep_range(500, 50000);

which is nicer to other tasks. You will normally be woken up much earlier.

	Arnd

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang
  2014-06-30  9:00   ` Arnd Bergmann
@ 2014-06-30  9:45   ` Ivan Khoronzhuk
  2014-07-02  2:09     ` Zhou Wang
  2014-06-30 10:00   ` Mark Rutland
  2 siblings, 1 reply; 16+ messages in thread
From: Ivan Khoronzhuk @ 2014-06-30  9:45 UTC (permalink / raw)
  To: linux-arm-kernel


On 06/30/2014 11:03 AM, Zhou Wang wrote:
> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
> ---
>   drivers/mtd/nand/Kconfig     |    5 +
>   drivers/mtd/nand/Makefile    |    1 +
>   drivers/mtd/nand/hisi_nand.c |  847 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 853 insertions(+)
>   create mode 100644 drivers/mtd/nand/hisi_nand.c
>
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 90ff447..253f8c8 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -510,4 +510,9 @@ config MTD_NAND_XWAY
>   	  Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached
>   	  to the External Bus Unit (EBU).
>   
> +config MTD_NAND_HISI
> +	tristate "Support for NAND controller on Hisilicon SoC"
> +	help
> +	  Enables support for NAND controller on Hisilicon SoC.
> +
>   endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 542b568..d0881cf 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740)		+= jz4740_nand.o
>   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_HISI)	        += hisi_nand.o
>   
>   nand-objs := nand_base.o nand_bbt.o
> diff --git a/drivers/mtd/nand/hisi_nand.c b/drivers/mtd/nand/hisi_nand.c
> new file mode 100644
> index 0000000..fbcb065
> --- /dev/null
> +++ b/drivers/mtd/nand/hisi_nand.c
> @@ -0,0 +1,847 @@
> +/*
> + * Hisilicon NAND Flash controller driver
> + *
> + * Copyright ? 2012-2014 HiSilicon Technologies Co., Ltd.
> + *              http://www.hisilicon.com
> + *
> + * Author: Zhou Wang <wangzhou.bry@gmail.com>
> + * The initial developer of the original code is Zhiyong Cai
> + * <caizhiyong@huawei.com>
> + *
> + * 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_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			(1U << 0)
> +#define HINFC504_CON_PAGEISZE_SHIFT			(1)
> +#define HINFC504_CON_PAGESIZE_MASK			(0x07)
> +#define HINFC504_CON_BUS_WIDTH				(1U << 4)
> +#define HINFC504_CON_READY_BUSY_SEL			(1U << 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			(1U << 1)
> +#define HINFC504_OP_WAIT_READY_EN			(1U << 2)
> +#define HINFC504_OP_CMD2_EN				(1U << 3)
> +#define HINFC504_OP_WRITE_DATA_EN			(1U << 4)
> +#define HINFC504_OP_ADDR_EN				(1U << 5)
> +#define HINFC504_OP_CMD1_EN				(1U << 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					(1U << 0)
> +
> +#define HINFC504_INTEN					0x24
> +#define HINFC504_INTEN_DMA				(1U << 9)
> +#define HINFC504_INTEN_UE				(1U << 6)
> +#define HINFC504_INTEN_CE				(1U << 5)
> +
> +#define HINFC504_INTS					0x28
> +#define HINFC504_INTS_DMA				(1U << 9)
> +#define HINFC504_INTS_UE				(1U << 6)
> +#define HINFC504_INTS_CE				(1U << 5)
> +
> +#define HINFC504_INTCLR					0x2C
> +#define HINFC504_INTCLR_DMA				(1U << 9)
> +#define HINFC504_INTCLR_UE				(1U << 6)
> +#define HINFC504_INTCLR_CE				(1U << 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			(1U << 0)
> +#define HINFC504_DMA_CTRL_WE				(1U << 1)
> +#define HINFC504_DMA_CTRL_DATA_AREA_EN			(1U << 2)
> +#define HINFC504_DMA_CTRL_OOB_AREA_EN			(1U << 3)
> +#define HINFC504_DMA_CTRL_BURST4_EN			(1U << 4)
> +#define HINFC504_DMA_CTRL_BURST8_EN			(1U << 5)
> +#define HINFC504_DMA_CTRL_BURST16_EN			(1U << 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			(1U << 0)
> +#define HINFC504_DMA_PARA_OOB_RW_EN			(1U << 1)
> +#define HINFC504_DMA_PARA_DATA_EDC_EN			(1U << 2)
> +#define HINFC504_DMA_PARA_OOB_EDC_EN			(1U << 3)
> +#define HINFC504_DMA_PARA_DATA_ECC_EN			(1U << 4)
> +#define HINFC504_DMA_PARA_OOB_ECC_EN			(1U << 5)
> +
> +#define HINFC_VERSION                                   0x74
> +#define HINFC504_LOG_READ_ADDR				0x7C
> +#define HINFC504_LOG_READ_LEN				0x80
> +
> +#define HINFC504_NANDINFO_LEN				0x10
> +
> +#define hinfc_read(_host, _reg)	readl(_host->iobase + (_reg))
> +#define hinfc_write(_host, _value, _reg)\
> +	writel((_value), _host->iobase + (_reg))
> +
> +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;
> +	unsigned int		addr_value[2];
> +	unsigned int		cache_addr_value[2];
> +	char			*buffer;
> +	dma_addr_t		dma_buffer;
> +	dma_addr_t		dma_oob;
> +	int			version;
> +	unsigned int            ecc_bits;
> +	unsigned int            irq_status; /* interrupt status */
> +
> +	int (*send_cmd_pageprog)(struct hinfc_host *host);
> +	int (*send_cmd_status)(struct hinfc_host *host);
> +	int (*send_cmd_readstart)(struct hinfc_host *host);
> +	int (*send_cmd_erase)(struct hinfc_host *host);
> +	int (*send_cmd_readid)(struct hinfc_host *host);
> +	int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
> +};
> +
> +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;
> +		} else {
> +			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

Add {} for else also
./scripts/checkpatch.pl.

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/3] mtd: hisilicon: add device tree binding documentation
  2014-06-30  8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang
@ 2014-06-30  9:52   ` Mark Rutland
  2014-07-02  2:46     ` Zhou Wang
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2014-06-30  9:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 30, 2014 at 09:03:29AM +0100, Zhou Wang wrote:
> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
> ---
>  .../devicetree/bindings/mtd/hisi-nand.txt          |   38 ++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/hisi-nand.txt b/Documentation/devicetree/bindings/mtd/hisi-nand.txt
> new file mode 100644
> index 0000000..1cc6470
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/hisi-nand.txt
> @@ -0,0 +1,38 @@
> +Hisilicon Hip04 Soc NAND controller DT binding
> +
> +Required properties:
> +- compatible:          Should be "hisilicon,nfc504".
> +- reg:                 Contain registers location and length for reg and data.

I'm not sure I follow. The example below has two reg entries. What does
each entry represent, and what does the presence of multiple entries
mean?

> +- interrupts:          Interrupt number for nfc.

Just the one?

> +- nand-bus-width:      See nand.txt.
> +- nand-ecc-mode:       See nand.txt.
> +- hisi,nand-ecc-bits:  ECC bits type support.
> +		<0>:   none ecc
> +		<1>:   Can correct 1bit per 512byte.
> +		<6>:   Can correct 16bits per 1K byte.

Is this an enumeration, or a number of bits?

> +- #address-cells:      partition address.
> +- #size-cells:         partition size.

Are these only allowed to be 1 cell, or can they be more?

Thanks,
Mark.

> +
> +Flash chip may optionally contain additional sub-nodes describing partitions of
> +the address space. See partition.txt for more detail.
> +
> +Example:
> +
> +	nand: nand at 4020000 {
> +		compatible = "hisilicon,nfc504";
> +		reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
> +		interrupts = <0 379 4>;
> +		nand-bus-width = <8>;
> +		nand-ecc-mode = "hw";
> +		hisi,nand-ecc-bits = <1>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		partition at 0 {
> +			label = "nand_text";
> +			reg = <0x00000000 0x00400000>;
> +		};
> +
> +		...
> +
> +	};
> -- 
> 1.7.9.5
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  9:00   ` Arnd Bergmann
@ 2014-06-30  9:59     ` Caizhiyong
  2014-07-02  2:12       ` Zhou Wang
  2014-07-02  2:07     ` Zhou Wang
  1 sibling, 1 reply; 16+ messages in thread
From: Caizhiyong @ 2014-06-30  9:59 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: Monday, June 30, 2014 5:01 PM
> To: linux-arm-kernel at lists.infradead.org
> Cc: Zhou Wang; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar
> Gala; Russell King; David Woodhouse; Brian Norris; Grant Likely; Ezequiel
> Garcia; Pekon Gupta; Artem Bityutskiy; Alexander Shiyan; Ivan Khoronzhuk;
> Jussi Kivilinna; Joern Engel; Randy Dunlap; devicetree at vger.kernel.org;
> linux-mtd at lists.infradead.org; linux-doc at vger.kernel.org;
> linux-kernel at vger.kernel.org; Caizhiyong; Wangzhou (B)
> Subject: Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for
> hisilicon hip04 Soc
> 
> 
> > +	struct device		*dev;
> > +	void __iomem		*iobase;
> > +	struct completion       cmd_complete;
> > +	unsigned int		offset;
> > +	unsigned int		command;
> > +	int			chipselect;
> > +	unsigned int		addr_cycle;
> > +	unsigned int		addr_value[2];
> > +	unsigned int		cache_addr_value[2];
> > +	char			*buffer;
> > +	dma_addr_t		dma_buffer;
> > +	dma_addr_t		dma_oob;
> > +	int			version;
> > +	unsigned int            ecc_bits;
> > +	unsigned int            irq_status; /* interrupt status */
> > +
> > +	int (*send_cmd_pageprog)(struct hinfc_host *host);
> > +	int (*send_cmd_status)(struct hinfc_host *host);
> > +	int (*send_cmd_readstart)(struct hinfc_host *host);
> > +	int (*send_cmd_erase)(struct hinfc_host *host);
> > +	int (*send_cmd_readid)(struct hinfc_host *host);
> > +	int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
> > +};
> 
> Why do you need function pointers here? The current version of the
> driver you posted always assigns these to the same functions, so
> it would be more efficient to just call those directly.
> 

I agree with you.
This feature was originally designed to support a variety version of NAND controllers.
But in fact, this feature is not used now.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang
  2014-06-30  9:00   ` Arnd Bergmann
  2014-06-30  9:45   ` Ivan Khoronzhuk
@ 2014-06-30 10:00   ` Mark Rutland
  2014-07-02  2:15     ` Zhou Wang
  2 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2014-06-30 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 30, 2014 at 09:03:28AM +0100, Zhou Wang wrote:
> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
> ---
>  drivers/mtd/nand/Kconfig     |    5 +
>  drivers/mtd/nand/Makefile    |    1 +
>  drivers/mtd/nand/hisi_nand.c |  847 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 853 insertions(+)
>  create mode 100644 drivers/mtd/nand/hisi_nand.c

[...]

> +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;
> +       unsigned int            addr_value[2];
> +       unsigned int            cache_addr_value[2];
> +       char                    *buffer;
> +       dma_addr_t              dma_buffer;
> +       dma_addr_t              dma_oob;
> +       int                     version;
> +       unsigned int            ecc_bits;
> +       unsigned int            irq_status; /* interrupt status */
> +
> +       int (*send_cmd_pageprog)(struct hinfc_host *host);
> +       int (*send_cmd_status)(struct hinfc_host *host);
> +       int (*send_cmd_readstart)(struct hinfc_host *host);
> +       int (*send_cmd_erase)(struct hinfc_host *host);
> +       int (*send_cmd_readid)(struct hinfc_host *host);
> +       int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
> +};

[...]

> +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) + sizeof(*chip) + sizeof(*mtd),
> +                       GFP_KERNEL);
> +       if (!host)
> +               return -ENOMEM;
> +       host->dev = dev;
> +
> +       platform_set_drvdata(pdev, host);
> +       chip = (struct nand_chip *)&host[1];
> +       mtd  = (struct mtd_info *)&chip[1];

Why not embed the whole struct rather than pointers? Then you can
allocate just the host and extract pointers to the chip and mtd sub
structures.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  9:00   ` Arnd Bergmann
  2014-06-30  9:59     ` Caizhiyong
@ 2014-07-02  2:07     ` Zhou Wang
  1 sibling, 0 replies; 16+ messages in thread
From: Zhou Wang @ 2014-07-02  2:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014?06?30? 17:00, Arnd Bergmann wrote:
> Overall a very nice driver. I didn't find any real bugs, but a few things
> that could be changed for readability and micro-optimizations:
>
> On Monday 30 June 2014 16:03:28 Zhou Wang wrote:
>
>> +#define hinfc_read(_host, _reg)	readl(_host->iobase + (_reg))
>> +#define hinfc_write(_host, _value, _reg)\
>> +	writel((_value), _host->iobase + (_reg))
>
> I'd recommend making these inline functions rather than macros.
>

I will change above macros to inline functions, thanks.

>> +struct hinfc_host {
>> +	struct nand_chip	*chip;
>> +	struct mtd_info		*mtd;
>
> I notice that you allocate the nand_chip and mtd_info together
> with the hinfc_host and then assign these pointers. A preferred
> method is normally to just embed the structures in this case:
>
> struct hinfc_host {
> 	struct nand_chip	chip;
> 	struct mtd_info		mtd;
>
> so you avoid the pointer overhead, and can go back from these
> two structures to the hinfc_host using container_of().
>

It is better to embed chip and mtd in host, I will change this, thanks.

>> +	struct device		*dev;
>> +	void __iomem		*iobase;
>> +	struct completion       cmd_complete;
>> +	unsigned int		offset;
>> +	unsigned int		command;
>> +	int			chipselect;
>> +	unsigned int		addr_cycle;
>> +	unsigned int		addr_value[2];
>> +	unsigned int		cache_addr_value[2];
>> +	char			*buffer;
>> +	dma_addr_t		dma_buffer;
>> +	dma_addr_t		dma_oob;
>> +	int			version;
>> +	unsigned int            ecc_bits;
>> +	unsigned int            irq_status; /* interrupt status */
>> +
>> +	int (*send_cmd_pageprog)(struct hinfc_host *host);
>> +	int (*send_cmd_status)(struct hinfc_host *host);
>> +	int (*send_cmd_readstart)(struct hinfc_host *host);
>> +	int (*send_cmd_erase)(struct hinfc_host *host);
>> +	int (*send_cmd_readid)(struct hinfc_host *host);
>> +	int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
>> +};
>
> Why do you need function pointers here? The current version of the
> driver you posted always assigns these to the same functions, so
> it would be more efficient to just call those directly.
>
> If you plan to add another variant later that uses a different set
> of functions here, you can add the function pointers then, in a
> separate patch. Also, a common style element is to have a separate
> constant structure with the function pointers:
>
> const struct hinfc_host_ops ops = {
> 	.send_cmd_pageprog = hisi_nfc_send_cmd_pageprog,
> 	...
> };
>
> if you actually end up needing function pointers. That simplifies
> the code a bit and is slightly more robust to attacks overwriting
> the function pointers.
>

I will move above function pointers out of host and directly call the 
functions, thanks for your good comment.

>> +
>> +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;
>> +		} else {
>> +			if (val & HINFC504_READY)
>> +				return;
>> +		}
>> +	}
>> +
>> +	/* wait cmd timeout */
>> +	dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n");
>> +}
>
> Since the timeout is much bigger than the wait time here, I guess you
> can use a wider range for the usleep_range(), e.g.
>
> 	usleep_range(500, 50000);
>
> which is nicer to other tasks. You will normally be woken up much earlier.
>
> 	Arnd
>
As I know that the operating time of NAND flash erasure is about 
ms-level, so I set sleep time as usleep_range(500, 1000) here. And the 
timeout means that relative operations don't finish in this time, so 
maybe there is something wrong with the operations. In normal situation, 
after detecting the operation finished, the above function will return 
before timeout arriving.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  9:45   ` Ivan Khoronzhuk
@ 2014-07-02  2:09     ` Zhou Wang
  0 siblings, 0 replies; 16+ messages in thread
From: Zhou Wang @ 2014-07-02  2:09 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014?06?30? 17:45, Ivan Khoronzhuk wrote:
>
> On 06/30/2014 11:03 AM, Zhou Wang wrote:
>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> ---
>>   drivers/mtd/nand/Kconfig     |    5 +
>>   drivers/mtd/nand/Makefile    |    1 +
>>   drivers/mtd/nand/hisi_nand.c |  847
>> ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 853 insertions(+)
>>   create mode 100644 drivers/mtd/nand/hisi_nand.c
>>
>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>> index 90ff447..253f8c8 100644
>> --- a/drivers/mtd/nand/Kconfig
>> +++ b/drivers/mtd/nand/Kconfig
>> @@ -510,4 +510,9 @@ config MTD_NAND_XWAY
>>         Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND
>> is attached
>>         to the External Bus Unit (EBU).
>> +config MTD_NAND_HISI
>> +    tristate "Support for NAND controller on Hisilicon SoC"
>> +    help
>> +      Enables support for NAND controller on Hisilicon SoC.
>> +
>>   endif # MTD_NAND
>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index 542b568..d0881cf 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740)        += jz4740_nand.o
>>   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_HISI)            += hisi_nand.o
>>   nand-objs := nand_base.o nand_bbt.o
>> diff --git a/drivers/mtd/nand/hisi_nand.c b/drivers/mtd/nand/hisi_nand.c
>> new file mode 100644
>> index 0000000..fbcb065
>> --- /dev/null
>> +++ b/drivers/mtd/nand/hisi_nand.c
>> @@ -0,0 +1,847 @@
>> +/*
>> + * Hisilicon NAND Flash controller driver
>> + *
>> + * Copyright ? 2012-2014 HiSilicon Technologies Co., Ltd.
>> + *              http://www.hisilicon.com
>> + *
>> + * Author: Zhou Wang <wangzhou.bry@gmail.com>
>> + * The initial developer of the original code is Zhiyong Cai
>> + * <caizhiyong@huawei.com>
>> + *
>> + * 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_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            (1U << 0)
>> +#define HINFC504_CON_PAGEISZE_SHIFT            (1)
>> +#define HINFC504_CON_PAGESIZE_MASK            (0x07)
>> +#define HINFC504_CON_BUS_WIDTH                (1U << 4)
>> +#define HINFC504_CON_READY_BUSY_SEL            (1U << 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            (1U << 1)
>> +#define HINFC504_OP_WAIT_READY_EN            (1U << 2)
>> +#define HINFC504_OP_CMD2_EN                (1U << 3)
>> +#define HINFC504_OP_WRITE_DATA_EN            (1U << 4)
>> +#define HINFC504_OP_ADDR_EN                (1U << 5)
>> +#define HINFC504_OP_CMD1_EN                (1U << 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                    (1U << 0)
>> +
>> +#define HINFC504_INTEN                    0x24
>> +#define HINFC504_INTEN_DMA                (1U << 9)
>> +#define HINFC504_INTEN_UE                (1U << 6)
>> +#define HINFC504_INTEN_CE                (1U << 5)
>> +
>> +#define HINFC504_INTS                    0x28
>> +#define HINFC504_INTS_DMA                (1U << 9)
>> +#define HINFC504_INTS_UE                (1U << 6)
>> +#define HINFC504_INTS_CE                (1U << 5)
>> +
>> +#define HINFC504_INTCLR                    0x2C
>> +#define HINFC504_INTCLR_DMA                (1U << 9)
>> +#define HINFC504_INTCLR_UE                (1U << 6)
>> +#define HINFC504_INTCLR_CE                (1U << 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            (1U << 0)
>> +#define HINFC504_DMA_CTRL_WE                (1U << 1)
>> +#define HINFC504_DMA_CTRL_DATA_AREA_EN            (1U << 2)
>> +#define HINFC504_DMA_CTRL_OOB_AREA_EN            (1U << 3)
>> +#define HINFC504_DMA_CTRL_BURST4_EN            (1U << 4)
>> +#define HINFC504_DMA_CTRL_BURST8_EN            (1U << 5)
>> +#define HINFC504_DMA_CTRL_BURST16_EN            (1U << 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            (1U << 0)
>> +#define HINFC504_DMA_PARA_OOB_RW_EN            (1U << 1)
>> +#define HINFC504_DMA_PARA_DATA_EDC_EN            (1U << 2)
>> +#define HINFC504_DMA_PARA_OOB_EDC_EN            (1U << 3)
>> +#define HINFC504_DMA_PARA_DATA_ECC_EN            (1U << 4)
>> +#define HINFC504_DMA_PARA_OOB_ECC_EN            (1U << 5)
>> +
>> +#define HINFC_VERSION                                   0x74
>> +#define HINFC504_LOG_READ_ADDR                0x7C
>> +#define HINFC504_LOG_READ_LEN                0x80
>> +
>> +#define HINFC504_NANDINFO_LEN                0x10
>> +
>> +#define hinfc_read(_host, _reg)    readl(_host->iobase + (_reg))
>> +#define hinfc_write(_host, _value, _reg)\
>> +    writel((_value), _host->iobase + (_reg))
>> +
>> +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;
>> +    unsigned int        addr_value[2];
>> +    unsigned int        cache_addr_value[2];
>> +    char            *buffer;
>> +    dma_addr_t        dma_buffer;
>> +    dma_addr_t        dma_oob;
>> +    int            version;
>> +    unsigned int            ecc_bits;
>> +    unsigned int            irq_status; /* interrupt status */
>> +
>> +    int (*send_cmd_pageprog)(struct hinfc_host *host);
>> +    int (*send_cmd_status)(struct hinfc_host *host);
>> +    int (*send_cmd_readstart)(struct hinfc_host *host);
>> +    int (*send_cmd_erase)(struct hinfc_host *host);
>> +    int (*send_cmd_readid)(struct hinfc_host *host);
>> +    int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
>> +};
>> +
>> +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;
>> +        } else {
>> +            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
>
> Add {} for else also
> ./scripts/checkpatch.pl.
>
ok, I will add them, thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30  9:59     ` Caizhiyong
@ 2014-07-02  2:12       ` Zhou Wang
  0 siblings, 0 replies; 16+ messages in thread
From: Zhou Wang @ 2014-07-02  2:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014?06?30? 17:59, Caizhiyong wrote:
>> -----Original Message-----
>> From: Arnd Bergmann [mailto:arnd at arndb.de]
>> Sent: Monday, June 30, 2014 5:01 PM
>> To: linux-arm-kernel at lists.infradead.org
>> Cc: Zhou Wang; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar
>> Gala; Russell King; David Woodhouse; Brian Norris; Grant Likely; Ezequiel
>> Garcia; Pekon Gupta; Artem Bityutskiy; Alexander Shiyan; Ivan Khoronzhuk;
>> Jussi Kivilinna; Joern Engel; Randy Dunlap; devicetree at vger.kernel.org;
>> linux-mtd at lists.infradead.org; linux-doc at vger.kernel.org;
>> linux-kernel at vger.kernel.org; Caizhiyong; Wangzhou (B)
>> Subject: Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for
>> hisilicon hip04 Soc
>>
>>
>>> +	struct device		*dev;
>>> +	void __iomem		*iobase;
>>> +	struct completion       cmd_complete;
>>> +	unsigned int		offset;
>>> +	unsigned int		command;
>>> +	int			chipselect;
>>> +	unsigned int		addr_cycle;
>>> +	unsigned int		addr_value[2];
>>> +	unsigned int		cache_addr_value[2];
>>> +	char			*buffer;
>>> +	dma_addr_t		dma_buffer;
>>> +	dma_addr_t		dma_oob;
>>> +	int			version;
>>> +	unsigned int            ecc_bits;
>>> +	unsigned int            irq_status; /* interrupt status */
>>> +
>>> +	int (*send_cmd_pageprog)(struct hinfc_host *host);
>>> +	int (*send_cmd_status)(struct hinfc_host *host);
>>> +	int (*send_cmd_readstart)(struct hinfc_host *host);
>>> +	int (*send_cmd_erase)(struct hinfc_host *host);
>>> +	int (*send_cmd_readid)(struct hinfc_host *host);
>>> +	int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
>>> +};
>>
>> Why do you need function pointers here? The current version of the
>> driver you posted always assigns these to the same functions, so
>> it would be more efficient to just call those directly.
>>
>
> I agree with you.
> This feature was originally designed to support a variety version of NAND controllers.
> But in fact, this feature is not used now.
>
I will move them out the host, thanks for your opinion!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
  2014-06-30 10:00   ` Mark Rutland
@ 2014-07-02  2:15     ` Zhou Wang
  0 siblings, 0 replies; 16+ messages in thread
From: Zhou Wang @ 2014-07-02  2:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014?06?30? 18:00, Mark Rutland wrote:
> On Mon, Jun 30, 2014 at 09:03:28AM +0100, Zhou Wang wrote:
>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> ---
>>   drivers/mtd/nand/Kconfig     |    5 +
>>   drivers/mtd/nand/Makefile    |    1 +
>>   drivers/mtd/nand/hisi_nand.c |  847 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 853 insertions(+)
>>   create mode 100644 drivers/mtd/nand/hisi_nand.c
>
> [...]
>
>> +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;
>> +       unsigned int            addr_value[2];
>> +       unsigned int            cache_addr_value[2];
>> +       char                    *buffer;
>> +       dma_addr_t              dma_buffer;
>> +       dma_addr_t              dma_oob;
>> +       int                     version;
>> +       unsigned int            ecc_bits;
>> +       unsigned int            irq_status; /* interrupt status */
>> +
>> +       int (*send_cmd_pageprog)(struct hinfc_host *host);
>> +       int (*send_cmd_status)(struct hinfc_host *host);
>> +       int (*send_cmd_readstart)(struct hinfc_host *host);
>> +       int (*send_cmd_erase)(struct hinfc_host *host);
>> +       int (*send_cmd_readid)(struct hinfc_host *host);
>> +       int (*send_cmd_reset)(struct hinfc_host *host, int chipselect);
>> +};
>
> [...]
>
>> +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) + sizeof(*chip) + sizeof(*mtd),
>> +                       GFP_KERNEL);
>> +       if (!host)
>> +               return -ENOMEM;
>> +       host->dev = dev;
>> +
>> +       platform_set_drvdata(pdev, host);
>> +       chip = (struct nand_chip *)&host[1];
>> +       mtd  = (struct mtd_info *)&chip[1];
>
> Why not embed the whole struct rather than pointers? Then you can
> allocate just the host and extract pointers to the chip and mtd sub
> structures.
>
> Thanks,
> Mark.
>

I will change this as your comment, thanks.

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/3] mtd: hisilicon: add device tree binding documentation
  2014-06-30  9:52   ` Mark Rutland
@ 2014-07-02  2:46     ` Zhou Wang
  0 siblings, 0 replies; 16+ messages in thread
From: Zhou Wang @ 2014-07-02  2:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014?06?30? 17:52, Mark Rutland wrote:
> On Mon, Jun 30, 2014 at 09:03:29AM +0100, Zhou Wang wrote:
>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> ---
>>   .../devicetree/bindings/mtd/hisi-nand.txt          |   38 ++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/hisi-nand.txt b/Documentation/devicetree/bindings/mtd/hisi-nand.txt
>> new file mode 100644
>> index 0000000..1cc6470
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/hisi-nand.txt
>> @@ -0,0 +1,38 @@
>> +Hisilicon Hip04 Soc NAND controller DT binding
>> +
>> +Required properties:
>> +- compatible:          Should be "hisilicon,nfc504".
>> +- reg:                 Contain registers location and length for reg and data.
>
> I'm not sure I follow. The example below has two reg entries. What does
> each entry represent, and what does the presence of multiple entries
> mean?
>

Maybe I could not express it clearly here. The first reg entry 
represents the basic address of registers of NAND controller, and the 
second represents the basic address of buffer of NAND controller.

>> +- interrupts:          Interrupt number for nfc.
>
> Just the one?
>

Yes, Just one interrupt number. And there is an interrupt relative 
register in the NAND controller, we can get information about interrupt 
from the register after an interrupt triggered.

>> +- nand-bus-width:      See nand.txt.
>> +- nand-ecc-mode:       See nand.txt.
>> +- hisi,nand-ecc-bits:  ECC bits type support.
>> +		<0>:   none ecc
>> +		<1>:   Can correct 1bit per 512byte.
>> +		<6>:   Can correct 16bits per 1K byte.
>
> Is this an enumeration, or a number of bits?
>

This is an enumeration which is just relative register configure, so we 
can directly use it to configure relative register bits in driver.

>> +- #address-cells:      partition address.
>> +- #size-cells:         partition size.
>
> Are these only allowed to be 1 cell, or can they be more?
>
> Thanks,
> Mark.
>

These are only allowed to be 1 cell. Address-cells represents basic 
address of a partition, and size-cells represents the size of a partition.

I will rewrite the unclear parts as your comments. Thanks for your comments.

-Zhou Wang

>> +
>> +Flash chip may optionally contain additional sub-nodes describing partitions of
>> +the address space. See partition.txt for more detail.
>> +
>> +Example:
>> +
>> +	nand: nand at 4020000 {
>> +		compatible = "hisilicon,nfc504";
>> +		reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
>> +		interrupts = <0 379 4>;
>> +		nand-bus-width = <8>;
>> +		nand-ecc-mode = "hw";
>> +		hisi,nand-ecc-bits = <1>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +
>> +		partition at 0 {
>> +			label = "nand_text";
>> +			reg = <0x00000000 0x00400000>;
>> +		};
>> +
>> +		...
>> +
>> +	};
>> --
>> 1.7.9.5
>>
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller
  2014-06-30  8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang
@ 2014-07-09  7:08   ` Jerome FORISSIER
  2014-07-11  2:40     ` Zhou Wang
  0 siblings, 1 reply; 16+ messages in thread
From: Jerome FORISSIER @ 2014-07-09  7:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 30-Jun-14 10:03, Zhou Wang wrote:
> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
> ---
>  arch/arm/boot/dts/hip04.dtsi |   31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
> index abb42ca..e63fc61 100644
> --- a/arch/arm/boot/dts/hip04.dtsi
> +++ b/arch/arm/boot/dts/hip04.dtsi
> @@ -386,5 +386,36 @@
>  				interrupts = <0 389 4>;
>  			};
>  		};
> +
> +		nand: nand at 4020000 {
> +			compatible = "hisilicon,504-nfc";

In PATCH 2/3 and 3/3 you are using "hisilicon,nfc504".

> +			reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
> +			interrupts = <0 379 4>;
> +			nand-bus-width = <8>;
> +			nand-ecc-mode = "none";
> +			hisi,nand-ecc-bits = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition at 0 {
> +				label = "nand_text";
> +				reg = <0x00000000 0x00400000>;
> +			};
> +
> +			partition at 00400000 {
> +				label = "nand_monitor";
> +				reg = <0x00400000 0x00400000>;
> +			};
> +
> +			partition at 00800000 {
> +				label = "nand_kernel";
> +				reg = <0x00800000 0x00800000>;
> +			};
> +
> +			partition at 01000000 {
> +				label = "nand_fs";
> +				reg = <0x01000000 0x1f000000>;
> +			};
> +		};
>  	};
>  };
> 

-- 
Jerome

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller
  2014-07-09  7:08   ` Jerome FORISSIER
@ 2014-07-11  2:40     ` Zhou Wang
  0 siblings, 0 replies; 16+ messages in thread
From: Zhou Wang @ 2014-07-11  2:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 2014?07?09? 15:08, Jerome FORISSIER wrote:
> On 30-Jun-14 10:03, Zhou Wang wrote:
>> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com>
>> ---
>>   arch/arm/boot/dts/hip04.dtsi |   31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
>> index abb42ca..e63fc61 100644
>> --- a/arch/arm/boot/dts/hip04.dtsi
>> +++ b/arch/arm/boot/dts/hip04.dtsi
>> @@ -386,5 +386,36 @@
>>   				interrupts = <0 389 4>;
>>   			};
>>   		};
>> +
>> +		nand: nand at 4020000 {
>> +			compatible = "hisilicon,504-nfc";
>
> In PATCH 2/3 and 3/3 you are using "hisilicon,nfc504".
>

It was my mistake. All should be "hisilicon,504-nfc". Thanks a lot.

>> +			reg = <0x4020000 0x10000>, <0x5000000 0x1000>;
>> +			interrupts = <0 379 4>;
>> +			nand-bus-width = <8>;
>> +			nand-ecc-mode = "none";
>> +			hisi,nand-ecc-bits = <0>;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			partition at 0 {
>> +				label = "nand_text";
>> +				reg = <0x00000000 0x00400000>;
>> +			};
>> +
>> +			partition at 00400000 {
>> +				label = "nand_monitor";
>> +				reg = <0x00400000 0x00400000>;
>> +			};
>> +
>> +			partition at 00800000 {
>> +				label = "nand_kernel";
>> +				reg = <0x00800000 0x00800000>;
>> +			};
>> +
>> +			partition at 01000000 {
>> +				label = "nand_fs";
>> +				reg = <0x01000000 0x1f000000>;
>> +			};
>> +		};
>>   	};
>>   };
>>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-07-11  2:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-30  8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang
2014-06-30  8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang
2014-07-09  7:08   ` Jerome FORISSIER
2014-07-11  2:40     ` Zhou Wang
2014-06-30  8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang
2014-06-30  9:00   ` Arnd Bergmann
2014-06-30  9:59     ` Caizhiyong
2014-07-02  2:12       ` Zhou Wang
2014-07-02  2:07     ` Zhou Wang
2014-06-30  9:45   ` Ivan Khoronzhuk
2014-07-02  2:09     ` Zhou Wang
2014-06-30 10:00   ` Mark Rutland
2014-07-02  2:15     ` Zhou Wang
2014-06-30  8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang
2014-06-30  9:52   ` Mark Rutland
2014-07-02  2:46     ` Zhou Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).