* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 8:03 ` Zhou Wang
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 8:03 ` Zhou Wang
0 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-06-30 8:03 UTC (permalink / raw)
To: 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,
linux-arm-kernel, devicetree, linux-mtd, linux-doc, linux-kernel
Cc: caizhiyong, Zhou Wang, wangzhou1
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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
2014-06-30 8:03 ` Zhou Wang
(?)
@ 2014-06-30 9:00 ` Arnd Bergmann
-1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2014-06-30 9:00 UTC (permalink / raw)
To: linux-arm-kernel
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, linux-mtd, linux-doc, linux-kernel,
caizhiyong, wangzhou1
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] 51+ 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
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 9:00 ` Arnd Bergmann
0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2014-06-30 9:00 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Alexander Shiyan, Ezequiel Garcia,
Grant Likely, devicetree, Pawel Moll, Ian Campbell, Joern Engel,
Rob Herring, Pekon Gupta, Zhou Wang, Randy Dunlap, linux-kernel,
wangzhou1, Kumar Gala, caizhiyong, Ivan Khoronzhuk, Brian Norris,
David Woodhouse
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] 51+ messages in thread* RE: [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
-1 siblings, 0 replies; 51+ messages in thread
From: Caizhiyong @ 2014-06-30 9:59 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Alexander Shiyan, Ezequiel Garcia,
Quyaxin, Grant Likely, devicetree, Pawel Moll, Ian Campbell,
Joern Engel, Rob Herring, Pekon Gupta, Wanli (welly),
Zhou Wang, Randy Dunlap, linux-kernel, Wangzhou
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Monday, June 30, 2014 5:01 PM
> To: linux-arm-kernel@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@vger.kernel.org;
> linux-mtd@lists.infradead.org; linux-doc@vger.kernel.org;
> linux-kernel@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] 51+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 9:59 ` Caizhiyong
0 siblings, 0 replies; 51+ 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] 51+ messages in thread
* RE: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 9:59 ` Caizhiyong
0 siblings, 0 replies; 51+ messages in thread
From: Caizhiyong @ 2014-06-30 9:59 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Alexander Shiyan, Ezequiel Garcia,
Quyaxin, Grant Likely, devicetree, Pawel Moll, Ian Campbell,
Joern Engel, Rob Herring, Pekon Gupta, Wanli (welly),
Zhou Wang, Randy Dunlap, linux-kernel, Wangzhou (B),
Kumar Gala, Ivan Khoronzhuk, Brian Norris, David Woodhouse
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Monday, June 30, 2014 5:01 PM
> To: linux-arm-kernel@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@vger.kernel.org;
> linux-mtd@lists.infradead.org; linux-doc@vger.kernel.org;
> linux-kernel@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] 51+ messages in thread
* Re: [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
-1 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:12 UTC (permalink / raw)
To: Caizhiyong
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Pawel Moll, Alexander Shiyan,
Ezequiel Garcia, Quyaxin, Grant Likely, devicetree,
Arnd Bergmann, Ian Campbell, Joern Engel, Rob Herring,
Pekon Gupta, Wanli (welly),
linux-arm-kernel, Randy Dunlap
On 2014年06月30日 17:59, Caizhiyong wrote:
>> -----Original Message-----
>> From: Arnd Bergmann [mailto:arnd@arndb.de]
>> Sent: Monday, June 30, 2014 5:01 PM
>> To: linux-arm-kernel@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@vger.kernel.org;
>> linux-mtd@lists.infradead.org; linux-doc@vger.kernel.org;
>> linux-kernel@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!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:12 ` Zhou Wang
0 siblings, 0 replies; 51+ 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] 51+ messages in thread
* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:12 ` Zhou Wang
0 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:12 UTC (permalink / raw)
To: Caizhiyong
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Pawel Moll, Alexander Shiyan,
Ezequiel Garcia, Quyaxin, Grant Likely, devicetree,
Arnd Bergmann, Ian Campbell, Joern Engel, Rob Herring,
Pekon Gupta, Wanli (welly),
linux-arm-kernel, Randy Dunlap, linux-kernel, Wangzhou (B),
Kumar Gala, Ivan Khoronzhuk, Brian Norris, David Woodhouse
On 2014年06月30日 17:59, Caizhiyong wrote:
>> -----Original Message-----
>> From: Arnd Bergmann [mailto:arnd@arndb.de]
>> Sent: Monday, June 30, 2014 5:01 PM
>> To: linux-arm-kernel@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@vger.kernel.org;
>> linux-mtd@lists.infradead.org; linux-doc@vger.kernel.org;
>> linux-kernel@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] 51+ messages in thread
* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
2014-06-30 9:00 ` Arnd Bergmann
(?)
(?)
@ 2014-07-02 2:07 ` Zhou Wang
-1 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, 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,
linux-mtd, linux-doc, linux-kernel, caizhiyong, wangzhou1
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] 51+ messages in thread* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:07 ` Zhou Wang
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:07 ` Zhou Wang
0 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Alexander Shiyan, Ezequiel Garcia,
Grant Likely, devicetree, Pawel Moll, Ian Campbell, Joern Engel,
Rob Herring, Pekon Gupta, linux-arm-kernel, Randy Dunlap,
linux-kernel, wangzhou1, Kumar Gala, caizhiyong, Ivan Khoronzhuk,
Brian Norris, David Woodhouse
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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:07 ` Zhou Wang
0 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, 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-u79uwXL29TY76Z2rM5mHXA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
caizhiyong-hv44wF8Li93QT0dZR+AlfA,
wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q
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.
--
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
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
2014-06-30 8:03 ` Zhou Wang
(?)
(?)
@ 2014-06-30 9:45 ` Ivan Khoronzhuk
-1 siblings, 0 replies; 51+ messages in thread
From: Ivan Khoronzhuk @ 2014-06-30 9:45 UTC (permalink / raw)
To: 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, Jussi Kivilinna, Joern Engel, Randy Dunlap,
linux-arm-kernel, devicetree, linux-mtd, linux-doc, linux-kernel
Cc: wangzhou1, caizhiyong
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] 51+ 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
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 9:45 ` Ivan Khoronzhuk
0 siblings, 0 replies; 51+ messages in thread
From: Ivan Khoronzhuk @ 2014-06-30 9:45 UTC (permalink / raw)
To: 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, Jussi Kivilinna, Joern Engel, Randy Dunlap,
linux-arm-kernel, devicetree, linux-mtd, linux-doc, linux-kernel
Cc: caizhiyong, wangzhou1
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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 9:45 ` Ivan Khoronzhuk
0 siblings, 0 replies; 51+ messages in thread
From: Ivan Khoronzhuk @ 2014-06-30 9:45 UTC (permalink / raw)
To: 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, Jussi Kivilinna, Joern Engel, Randy Dunlap,
linux-arm-kernel, devicetree, linux-mtd, linux-doc, linux-kernel
Cc: wangzhou1, caizhiyong
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] 51+ messages in thread* Re: [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
-1 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:09 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: 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,
Jussi Kivilinna, Joern Engel, Randy Dunlap, linux-arm-kernel,
devicetree, linux-mtd, linux-doc, linux-kernel, wangzhou1,
caizhiyong
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] 51+ messages in thread* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:09 ` Zhou Wang
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:09 ` Zhou Wang
0 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:09 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Mark Rutland, Jussi Kivilinna, linux-doc, Artem Bityutskiy,
linux-mtd, Russell King, Alexander Shiyan, Ezequiel Garcia,
Grant Likely, devicetree, Pawel Moll, Ian Campbell, Joern Engel,
Rob Herring, Pekon Gupta, linux-arm-kernel, Randy Dunlap,
linux-kernel, wangzhou1, Kumar Gala, caizhiyong, Brian Norris,
David Woodhouse
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] 51+ messages in thread
* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
2014-06-30 8:03 ` Zhou Wang
(?)
@ 2014-06-30 10:00 ` Mark Rutland
-1 siblings, 0 replies; 51+ messages in thread
From: Mark Rutland @ 2014-06-30 10:00 UTC (permalink / raw)
To: Zhou Wang
Cc: Jussi Kivilinna, linux-doc, Artem Bityutskiy, linux-mtd,
Russell King, Alexander Shiyan, Ezequiel Garcia, grant.likely,
devicetree, Pawel Moll, Ian Campbell, Joern Engel, Rob Herring,
Pekon Gupta, linux-arm-kernel, Randy Dunlap, linux-kernel,
wangzhou1, Kumar Gala
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] 51+ 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
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-06-30 10:00 ` Mark Rutland
0 siblings, 0 replies; 51+ messages in thread
From: Mark Rutland @ 2014-06-30 10:00 UTC (permalink / raw)
To: Zhou Wang
Cc: Jussi Kivilinna, linux-doc, Artem Bityutskiy, linux-mtd,
Russell King, Alexander Shiyan, Ezequiel Garcia, grant.likely,
devicetree, Pawel Moll, Ian Campbell, Joern Engel, Rob Herring,
Pekon Gupta, linux-arm-kernel, Randy Dunlap, linux-kernel,
wangzhou1, Kumar Gala, caizhiyong, Ivan Khoronzhuk, Brian Norris,
David Woodhouse
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] 51+ messages in thread* Re: [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
-1 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:15 UTC (permalink / raw)
To: Mark Rutland
Cc: Jussi Kivilinna, linux-doc, Artem Bityutskiy, linux-mtd,
Russell King, Alexander Shiyan, Ezequiel Garcia, grant.likely,
devicetree, Pawel Moll, Ian Campbell, Joern Engel, Rob Herring,
Pekon Gupta, linux-arm-kernel, Randy Dunlap, linux-kernel,
wangzhou1, Kumar Gala
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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 51+ messages in thread* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:15 ` Zhou Wang
0 siblings, 0 replies; 51+ 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] 51+ messages in thread* Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc
@ 2014-07-02 2:15 ` Zhou Wang
0 siblings, 0 replies; 51+ messages in thread
From: Zhou Wang @ 2014-07-02 2:15 UTC (permalink / raw)
To: Mark Rutland
Cc: Jussi Kivilinna, linux-doc, Artem Bityutskiy, linux-mtd,
Russell King, Alexander Shiyan, Ezequiel Garcia, grant.likely,
devicetree, Pawel Moll, Ian Campbell, Joern Engel, Rob Herring,
Pekon Gupta, linux-arm-kernel, Randy Dunlap, linux-kernel,
wangzhou1, Kumar Gala, caizhiyong, Ivan Khoronzhuk, Brian Norris,
David Woodhouse
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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 51+ messages in thread