* [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc
@ 2014-06-30 8:03 Zhou Wang
2014-06-30 8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Zhou Wang @ 2014-06-30 8:03 UTC (permalink / raw)
To: linux-arm-kernel
This series patches add the support for NAND controller of hisilicon
hip04 Soc. These patches are base on branch integration-hilt-working-v3.14
in linaro landing-team git repository[1].
The NAND controller IP was developed by hisilicon and need a new driver
to support. The driver is usable and I tested it on hisilicon hip04-d01
board.
[1] ssh://git at git.linaro.org/landing-teams/working/hisilicon/kernel.git
Zhou Wang (3):
mtd: hisilicon: add device tree node for nand controller
mtd: hisilicon: add a new nand controller driver for hisilicon hip04
Soc
mtd: hisilicon: add device tree binding documentation
.../devicetree/bindings/mtd/hisi-nand.txt | 38 +
arch/arm/boot/dts/hip04.dtsi | 31 +
drivers/mtd/nand/Kconfig | 5 +
drivers/mtd/nand/Makefile | 1 +
drivers/mtd/nand/hisi_nand.c | 847 ++++++++++++++++++++
5 files changed, 922 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt
create mode 100644 drivers/mtd/nand/hisi_nand.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller 2014-06-30 8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang @ 2014-06-30 8:03 ` Zhou Wang 2014-07-09 7:08 ` Jerome FORISSIER 2014-06-30 8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang 2014-06-30 8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang 2 siblings, 1 reply; 16+ messages in thread From: Zhou Wang @ 2014-06-30 8:03 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> --- arch/arm/boot/dts/hip04.dtsi | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi index abb42ca..e63fc61 100644 --- a/arch/arm/boot/dts/hip04.dtsi +++ b/arch/arm/boot/dts/hip04.dtsi @@ -386,5 +386,36 @@ interrupts = <0 389 4>; }; }; + + nand: nand at 4020000 { + compatible = "hisilicon,504-nfc"; + reg = <0x4020000 0x10000>, <0x5000000 0x1000>; + interrupts = <0 379 4>; + nand-bus-width = <8>; + nand-ecc-mode = "none"; + hisi,nand-ecc-bits = <0>; + #address-cells = <1>; + #size-cells = <1>; + + partition at 0 { + label = "nand_text"; + reg = <0x00000000 0x00400000>; + }; + + partition at 00400000 { + label = "nand_monitor"; + reg = <0x00400000 0x00400000>; + }; + + partition at 00800000 { + label = "nand_kernel"; + reg = <0x00800000 0x00800000>; + }; + + partition at 01000000 { + label = "nand_fs"; + reg = <0x01000000 0x1f000000>; + }; + }; }; }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller 2014-06-30 8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang @ 2014-07-09 7:08 ` Jerome FORISSIER 2014-07-11 2:40 ` Zhou Wang 0 siblings, 1 reply; 16+ messages in thread From: Jerome FORISSIER @ 2014-07-09 7:08 UTC (permalink / raw) To: linux-arm-kernel On 30-Jun-14 10:03, Zhou Wang wrote: > Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> > --- > arch/arm/boot/dts/hip04.dtsi | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi > index abb42ca..e63fc61 100644 > --- a/arch/arm/boot/dts/hip04.dtsi > +++ b/arch/arm/boot/dts/hip04.dtsi > @@ -386,5 +386,36 @@ > interrupts = <0 389 4>; > }; > }; > + > + nand: nand at 4020000 { > + compatible = "hisilicon,504-nfc"; In PATCH 2/3 and 3/3 you are using "hisilicon,nfc504". > + reg = <0x4020000 0x10000>, <0x5000000 0x1000>; > + interrupts = <0 379 4>; > + nand-bus-width = <8>; > + nand-ecc-mode = "none"; > + hisi,nand-ecc-bits = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + partition at 0 { > + label = "nand_text"; > + reg = <0x00000000 0x00400000>; > + }; > + > + partition at 00400000 { > + label = "nand_monitor"; > + reg = <0x00400000 0x00400000>; > + }; > + > + partition at 00800000 { > + label = "nand_kernel"; > + reg = <0x00800000 0x00800000>; > + }; > + > + partition at 01000000 { > + label = "nand_fs"; > + reg = <0x01000000 0x1f000000>; > + }; > + }; > }; > }; > -- Jerome ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller 2014-07-09 7:08 ` Jerome FORISSIER @ 2014-07-11 2:40 ` Zhou Wang 0 siblings, 0 replies; 16+ messages in thread From: Zhou Wang @ 2014-07-11 2:40 UTC (permalink / raw) To: linux-arm-kernel On 2014?07?09? 15:08, Jerome FORISSIER wrote: > On 30-Jun-14 10:03, Zhou Wang wrote: >> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> >> --- >> arch/arm/boot/dts/hip04.dtsi | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi >> index abb42ca..e63fc61 100644 >> --- a/arch/arm/boot/dts/hip04.dtsi >> +++ b/arch/arm/boot/dts/hip04.dtsi >> @@ -386,5 +386,36 @@ >> interrupts = <0 389 4>; >> }; >> }; >> + >> + nand: nand at 4020000 { >> + compatible = "hisilicon,504-nfc"; > > In PATCH 2/3 and 3/3 you are using "hisilicon,nfc504". > It was my mistake. All should be "hisilicon,504-nfc". Thanks a lot. >> + reg = <0x4020000 0x10000>, <0x5000000 0x1000>; >> + interrupts = <0 379 4>; >> + nand-bus-width = <8>; >> + nand-ecc-mode = "none"; >> + hisi,nand-ecc-bits = <0>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + partition at 0 { >> + label = "nand_text"; >> + reg = <0x00000000 0x00400000>; >> + }; >> + >> + partition at 00400000 { >> + label = "nand_monitor"; >> + reg = <0x00400000 0x00400000>; >> + }; >> + >> + partition at 00800000 { >> + label = "nand_kernel"; >> + reg = <0x00800000 0x00800000>; >> + }; >> + >> + partition at 01000000 { >> + label = "nand_fs"; >> + reg = <0x01000000 0x1f000000>; >> + }; >> + }; >> }; >> }; >> > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang 2014-06-30 8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang @ 2014-06-30 8:03 ` Zhou Wang 2014-06-30 9:00 ` Arnd Bergmann ` (2 more replies) 2014-06-30 8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang 2 siblings, 3 replies; 16+ messages in thread From: Zhou Wang @ 2014-06-30 8:03 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> --- drivers/mtd/nand/Kconfig | 5 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/hisi_nand.c | 847 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 853 insertions(+) create mode 100644 drivers/mtd/nand/hisi_nand.c diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 90ff447..253f8c8 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -510,4 +510,9 @@ config MTD_NAND_XWAY Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached to the External Bus Unit (EBU). +config MTD_NAND_HISI + tristate "Support for NAND controller on Hisilicon SoC" + help + Enables support for NAND controller on Hisilicon SoC. + endif # MTD_NAND diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 542b568..d0881cf 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ +obj-$(CONFIG_MTD_NAND_HISI) += hisi_nand.o nand-objs := nand_base.o nand_bbt.o diff --git a/drivers/mtd/nand/hisi_nand.c b/drivers/mtd/nand/hisi_nand.c new file mode 100644 index 0000000..fbcb065 --- /dev/null +++ b/drivers/mtd/nand/hisi_nand.c @@ -0,0 +1,847 @@ +/* + * Hisilicon NAND Flash controller driver + * + * Copyright ? 2012-2014 HiSilicon Technologies Co., Ltd. + * http://www.hisilicon.com + * + * Author: Zhou Wang <wangzhou.bry@gmail.com> + * The initial developer of the original code is Zhiyong Cai + * <caizhiyong@huawei.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#include <linux/of.h> +#include <linux/of_mtd.h> +#include <linux/mtd/mtd.h> +#include <linux/sizes.h> +#include <linux/clk.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/mtd/nand.h> +#include <linux/dma-mapping.h> +#include <linux/platform_device.h> +#include <linux/mtd/partitions.h> + +#define HINFC504_MAX_CHIP (4) +#define HINFC504_W_LATCH (5) +#define HINFC504_R_LATCH (7) +#define HINFC504_RW_LATCH (3) + +#define HINFC504_NFC_TIMEOUT (2 * HZ) +#define HINFC504_NFC_DMA_TIMEOUT (5 * HZ) +#define HINFC504_CHIP_DELAY (25) + +#define HINFC504_REG_BASE_ADDRESS_LEN (0x100) +#define HINFC504_BUFFER_BASE_ADDRESS_LEN (2048 + 128) + +#define HINFC504_ADDR_CYCLE_MASK 0x4 + +#define HINFC504_CON 0x00 +#define HINFC504_CON_OP_MODE_NORMAL (1U << 0) +#define HINFC504_CON_PAGEISZE_SHIFT (1) +#define HINFC504_CON_PAGESIZE_MASK (0x07) +#define HINFC504_CON_BUS_WIDTH (1U << 4) +#define HINFC504_CON_READY_BUSY_SEL (1U << 8) +#define HINFC504_CON_ECCTYPE_SHIFT (9) +#define HINFC504_CON_ECCTYPE_MASK (0x07) + +#define HINFC504_PWIDTH 0x04 +#define SET_HINFC504_PWIDTH(_w_lcnt, _r_lcnt, _rw_hcnt) \ + ((_w_lcnt) | (((_r_lcnt) & 0x0F) << 4) | (((_rw_hcnt) & 0x0F) << 8)) + +#define HINFC504_CMD 0x0C +#define HINFC504_ADDRL 0x10 +#define HINFC504_ADDRH 0x14 +#define HINFC504_DATA_NUM 0x18 + +#define HINFC504_OP 0x1C +#define HINFC504_OP_READ_DATA_EN (1U << 1) +#define HINFC504_OP_WAIT_READY_EN (1U << 2) +#define HINFC504_OP_CMD2_EN (1U << 3) +#define HINFC504_OP_WRITE_DATA_EN (1U << 4) +#define HINFC504_OP_ADDR_EN (1U << 5) +#define HINFC504_OP_CMD1_EN (1U << 6) +#define HINFC504_OP_NF_CS_SHIFT (7) +#define HINFC504_OP_NF_CS_MASK (3) +#define HINFC504_OP_ADDR_CYCLE_SHIFT (9) +#define HINFC504_OP_ADDR_CYCLE_MASK (7) + +#define HINFC504_STATUS 0x20 +#define HINFC504_READY (1U << 0) + +#define HINFC504_INTEN 0x24 +#define HINFC504_INTEN_DMA (1U << 9) +#define HINFC504_INTEN_UE (1U << 6) +#define HINFC504_INTEN_CE (1U << 5) + +#define HINFC504_INTS 0x28 +#define HINFC504_INTS_DMA (1U << 9) +#define HINFC504_INTS_UE (1U << 6) +#define HINFC504_INTS_CE (1U << 5) + +#define HINFC504_INTCLR 0x2C +#define HINFC504_INTCLR_DMA (1U << 9) +#define HINFC504_INTCLR_UE (1U << 6) +#define HINFC504_INTCLR_CE (1U << 5) + +#define HINFC504_ECC_STATUS 0x5C +#define HINFC504_ECC_1_BIT_SHIFT 16 +#define HINFC504_ECC_16_BIT_SHIFT 12 + +#define HINFC504_DMA_CTRL 0x60 +#define HINFC504_DMA_CTRL_DMA_START (1U << 0) +#define HINFC504_DMA_CTRL_WE (1U << 1) +#define HINFC504_DMA_CTRL_DATA_AREA_EN (1U << 2) +#define HINFC504_DMA_CTRL_OOB_AREA_EN (1U << 3) +#define HINFC504_DMA_CTRL_BURST4_EN (1U << 4) +#define HINFC504_DMA_CTRL_BURST8_EN (1U << 5) +#define HINFC504_DMA_CTRL_BURST16_EN (1U << 6) +#define HINFC504_DMA_CTRL_ADDR_NUM_SHIFT (7) +#define HINFC504_DMA_CTRL_ADDR_NUM_MASK (1) +#define HINFC504_DMA_CTRL_CS_SHIFT (8) +#define HINFC504_DMA_CTRL_CS_MASK (0x03) + +#define HINFC504_DMA_ADDR_DATA 0x64 +#define HINFC504_DMA_ADDR_OOB 0x68 + +#define HINFC504_DMA_LEN 0x6C +#define HINFC504_DMA_LEN_OOB_SHIFT (16) +#define HINFC504_DMA_LEN_OOB_MASK (0xFFF) + +#define HINFC504_DMA_PARA 0x70 +#define HINFC504_DMA_PARA_DATA_RW_EN (1U << 0) +#define HINFC504_DMA_PARA_OOB_RW_EN (1U << 1) +#define HINFC504_DMA_PARA_DATA_EDC_EN (1U << 2) +#define HINFC504_DMA_PARA_OOB_EDC_EN (1U << 3) +#define HINFC504_DMA_PARA_DATA_ECC_EN (1U << 4) +#define HINFC504_DMA_PARA_OOB_ECC_EN (1U << 5) + +#define HINFC_VERSION 0x74 +#define HINFC504_LOG_READ_ADDR 0x7C +#define HINFC504_LOG_READ_LEN 0x80 + +#define HINFC504_NANDINFO_LEN 0x10 + +#define hinfc_read(_host, _reg) readl(_host->iobase + (_reg)) +#define hinfc_write(_host, _value, _reg)\ + writel((_value), _host->iobase + (_reg)) + +struct hinfc_host { + struct nand_chip *chip; + struct mtd_info *mtd; + struct device *dev; + void __iomem *iobase; + struct completion cmd_complete; + unsigned int offset; + unsigned int command; + int chipselect; + unsigned int addr_cycle; + unsigned int addr_value[2]; + unsigned int cache_addr_value[2]; + char *buffer; + dma_addr_t dma_buffer; + dma_addr_t dma_oob; + int version; + unsigned int ecc_bits; + unsigned int irq_status; /* interrupt status */ + + int (*send_cmd_pageprog)(struct hinfc_host *host); + int (*send_cmd_status)(struct hinfc_host *host); + int (*send_cmd_readstart)(struct hinfc_host *host); + int (*send_cmd_erase)(struct hinfc_host *host); + int (*send_cmd_readid)(struct hinfc_host *host); + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); +}; + +void wait_controller_finished(struct hinfc_host *host) +{ + unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT; + int val; + + while (time_before(jiffies, timeout)) { + val = hinfc_read(host, HINFC504_STATUS); + if (host->command == NAND_CMD_ERASE2) { + /* nfc is ready */ + while (!(val & HINFC504_READY)) { + usleep_range(500, 1000); + val = hinfc_read(host, HINFC504_STATUS); + } + return; + } else { + if (val & HINFC504_READY) + return; + } + } + + /* wait cmd timeout */ + dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n"); +} + +static void hisi_nfc_dma_transfer(struct hinfc_host *host, int todev) +{ + struct mtd_info *mtd = host->mtd; + struct nand_chip *chip = mtd->priv; + unsigned long val; + int ret; + + hinfc_write(host, host->dma_buffer, HINFC504_DMA_ADDR_DATA); + hinfc_write(host, host->dma_oob, HINFC504_DMA_ADDR_OOB); + + if (chip->ecc.mode == NAND_ECC_NONE) { + hinfc_write(host, ((mtd->oobsize & HINFC504_DMA_LEN_OOB_MASK) + << HINFC504_DMA_LEN_OOB_SHIFT), HINFC504_DMA_LEN); + + hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN + | HINFC504_DMA_PARA_OOB_RW_EN, HINFC504_DMA_PARA); + } else + hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN + | HINFC504_DMA_PARA_OOB_RW_EN | HINFC504_DMA_PARA_DATA_EDC_EN + | HINFC504_DMA_PARA_OOB_EDC_EN | HINFC504_DMA_PARA_DATA_ECC_EN + | HINFC504_DMA_PARA_OOB_ECC_EN, HINFC504_DMA_PARA); + + val = (HINFC504_DMA_CTRL_DMA_START | HINFC504_DMA_CTRL_BURST4_EN + | HINFC504_DMA_CTRL_BURST8_EN | HINFC504_DMA_CTRL_BURST16_EN + | HINFC504_DMA_CTRL_DATA_AREA_EN | HINFC504_DMA_CTRL_OOB_AREA_EN + | ((host->addr_cycle == 4 ? 1 : 0) + << HINFC504_DMA_CTRL_ADDR_NUM_SHIFT) + | ((host->chipselect & HINFC504_DMA_CTRL_CS_MASK) + << HINFC504_DMA_CTRL_CS_SHIFT)); + + if (todev) + val |= HINFC504_DMA_CTRL_WE; + + hinfc_write(host, val, HINFC504_DMA_CTRL); + + init_completion(&host->cmd_complete); + ret = wait_for_completion_timeout(&host->cmd_complete, + HINFC504_NFC_DMA_TIMEOUT); + + if (!ret) { + dev_err(host->dev, "DMA operation(irq) timeout!\n"); + /* sanity check */ + val = hinfc_read(host, HINFC504_DMA_CTRL); + if (!(val & HINFC504_DMA_CTRL_DMA_START)) + dev_err(host->dev, "dma is already done but without irq ACK!"); + else + dev_err(host->dev, "dma is really timeout!"); + } +} + +static int hisi_nfc_send_cmd_pageprog(struct hinfc_host *host) +{ + host->addr_value[0] &= 0xffff0000; + + hinfc_write(host, host->addr_value[0], HINFC504_ADDRL); + hinfc_write(host, host->addr_value[1], HINFC504_ADDRH); + hinfc_write(host, NAND_CMD_PAGEPROG << 8 | NAND_CMD_SEQIN, + HINFC504_CMD); + + hisi_nfc_dma_transfer(host, 1); + + return 0; +} + +static int hisi_nfc_send_cmd_readstart(struct hinfc_host *host) +{ + struct mtd_info *mtd = host->mtd; + + if ((host->addr_value[0] == host->cache_addr_value[0]) && + (host->addr_value[1] == host->cache_addr_value[1])) + return 0; + + host->addr_value[0] &= 0xffff0000; + + hinfc_write(host, host->addr_value[0], HINFC504_ADDRL); + hinfc_write(host, host->addr_value[1], HINFC504_ADDRH); + hinfc_write(host, NAND_CMD_READSTART << 8 | NAND_CMD_READ0, + HINFC504_CMD); + + hinfc_write(host, 0, HINFC504_LOG_READ_ADDR); + hinfc_write(host, mtd->writesize + mtd->oobsize, + HINFC504_LOG_READ_LEN); + + hisi_nfc_dma_transfer(host, 0); + + host->cache_addr_value[0] = host->addr_value[0]; + host->cache_addr_value[1] = host->addr_value[1]; + + return 0; +} + +static int hisi_nfc_send_cmd_erase(struct hinfc_host *host) +{ + hinfc_write(host, host->addr_value[0], HINFC504_ADDRL); + hinfc_write(host, (NAND_CMD_ERASE2 << 8) | NAND_CMD_ERASE1, + HINFC504_CMD); + + hinfc_write(host, HINFC504_OP_WAIT_READY_EN + | HINFC504_OP_CMD2_EN + | HINFC504_OP_CMD1_EN + | HINFC504_OP_ADDR_EN + | ((host->chipselect & HINFC504_OP_NF_CS_MASK) + << HINFC504_OP_NF_CS_SHIFT) + | ((host->addr_cycle & HINFC504_OP_ADDR_CYCLE_MASK) + << HINFC504_OP_ADDR_CYCLE_SHIFT), + HINFC504_OP); + + wait_controller_finished(host); + + return 0; +} + +static int hisi_nfc_send_cmd_readid(struct hinfc_host *host) +{ + hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM); + hinfc_write(host, NAND_CMD_READID, HINFC504_CMD); + hinfc_write(host, 0, HINFC504_ADDRL); + + hinfc_write(host, HINFC504_OP_CMD1_EN | HINFC504_OP_ADDR_EN + | HINFC504_OP_READ_DATA_EN + | ((host->chipselect & HINFC504_OP_NF_CS_MASK) + << HINFC504_OP_NF_CS_SHIFT) + | 1 << HINFC504_OP_ADDR_CYCLE_SHIFT, HINFC504_OP); + + wait_controller_finished(host); + + return 0; +} + +static int hisi_nfc_send_cmd_status(struct hinfc_host *host) +{ + hinfc_write(host, HINFC504_NANDINFO_LEN, HINFC504_DATA_NUM); + hinfc_write(host, NAND_CMD_STATUS, HINFC504_CMD); + hinfc_write(host, HINFC504_OP_CMD1_EN + | HINFC504_OP_READ_DATA_EN + | ((host->chipselect & HINFC504_OP_NF_CS_MASK) + << HINFC504_OP_NF_CS_SHIFT), + HINFC504_OP); + + wait_controller_finished(host); + + return 0; +} + +static int hisi_nfc_send_cmd_reset(struct hinfc_host *host, int chipselect) +{ + hinfc_write(host, NAND_CMD_RESET, HINFC504_CMD); + + hinfc_write(host, HINFC504_OP_CMD1_EN + | ((chipselect & HINFC504_OP_NF_CS_MASK) + << HINFC504_OP_NF_CS_SHIFT) + | HINFC504_OP_WAIT_READY_EN, + HINFC504_OP); + + wait_controller_finished(host); + + return 0; +} + +static void hisi_nfc_select_chip(struct mtd_info *mtd, int chipselect) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + + if (chipselect < 0) + return; + + host->chipselect = chipselect; +} + +static uint8_t hisi_nfc_read_byte(struct mtd_info *mtd) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + + if (host->command == NAND_CMD_STATUS) + return readb(chip->IO_ADDR_R); + + host->offset++; + + if (host->command == NAND_CMD_READID) + return readb(chip->IO_ADDR_R + host->offset - 1); + + return readb(host->buffer + host->offset - 1); +} + +static u16 hisi_nfc_read_word(struct mtd_info *mtd) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + + host->offset += 2; + return readw(host->buffer + host->offset - 2); +} + +static void hisi_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + + memcpy(host->buffer + host->offset, buf, len); + host->offset += len; +} + +static void hisi_nfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + + memcpy(buf, host->buffer + host->offset, len); + host->offset += len; +} + +static void set_addr(struct mtd_info *mtd, int column, int page_addr) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + + host->addr_cycle = 0; + host->addr_value[0] = 0; + host->addr_value[1] = 0; + + /* Serially input address */ + if (column != -1) { + /* Adjust columns for 16 bit buswidth */ + if (chip->options & NAND_BUSWIDTH_16) + column >>= 1; + + host->addr_value[0] = column & 0xffff; + host->addr_cycle = 2; + } + if (page_addr != -1) { + host->addr_value[0] |= (page_addr & 0xffff) + << (host->addr_cycle * 8); + host->addr_cycle += 2; + /* One more address cycle for devices > 128MiB */ + if (chip->chipsize > (128 << 20)) { + host->addr_cycle += 1; + if (host->command == NAND_CMD_ERASE1) + host->addr_value[0] |= ((page_addr >> 16) & 0xff) << 16; + else + host->addr_value[1] |= ((page_addr >> 16) & 0xff); + } + } +} + +static void hisi_nfc_cmdfunc(struct mtd_info *mtd, unsigned command, int column, + int page_addr) +{ + struct nand_chip *chip = mtd->priv; + struct hinfc_host *host = chip->priv; + int is_cache_invalid = 1; + unsigned int flag = 0; + host->command = command; + + switch (command) { + case NAND_CMD_READ0: + case NAND_CMD_READOOB: + if (command == NAND_CMD_READ0) + host->offset = column; + else + host->offset = column + mtd->writesize; + + is_cache_invalid = 0; + set_addr(mtd, column, page_addr); + host->send_cmd_readstart(host); + break; + + case NAND_CMD_SEQIN: + host->offset = column; + + case NAND_CMD_ERASE1: + set_addr(mtd, column, page_addr); + break; + + case NAND_CMD_PAGEPROG: + host->send_cmd_pageprog(host); + break; + + case NAND_CMD_ERASE2: + host->send_cmd_erase(host); + break; + + case NAND_CMD_READID: + host->offset = column; + memset(chip->IO_ADDR_R, 0, 0x10); + host->send_cmd_readid(host); + break; + + case NAND_CMD_STATUS: + flag = hinfc_read(host, HINFC504_CON); + if (chip->ecc.mode == NAND_ECC_HW) + hinfc_write(host, + flag && ~(HINFC504_CON_ECCTYPE_MASK << + HINFC504_CON_ECCTYPE_SHIFT), HINFC504_CON); + + host->offset = 0; + memset(chip->IO_ADDR_R, 0, 0x10); + host->send_cmd_status(host); + + hinfc_write(host, flag, HINFC504_CON); + break; + + case NAND_CMD_RESET: + host->send_cmd_reset(host, host->chipselect); + break; + + default: + dev_err(host->dev, "Error: unsupported cmd(cmd=%x, col=%x, page=%x)\n", + command, column, page_addr); + } + + if (is_cache_invalid) { + host->cache_addr_value[0] = ~0; + host->cache_addr_value[1] = ~0; + } +} + +static irqreturn_t hinfc_irq_handle(int irq, void *devid) +{ + struct hinfc_host *host = devid; + unsigned int flag; + + flag = hinfc_read(host, HINFC504_INTS); + /* store interrupts state */ + host->irq_status |= flag; + + if (flag & HINFC504_INTS_DMA) { + hinfc_write(host, HINFC504_INTCLR_DMA, HINFC504_INTCLR); + complete(&host->cmd_complete); + } else if (flag & HINFC504_INTS_CE) { + hinfc_write(host, HINFC504_INTCLR_CE, HINFC504_INTCLR); + } else if (flag & HINFC504_INTS_UE) { + hinfc_write(host, HINFC504_INTCLR_UE, HINFC504_INTCLR); + } + + return IRQ_HANDLED; +} + +static int hisi_nand_read_page_hwecc(struct mtd_info *mtd, + struct nand_chip *chip, uint8_t *buf, int oob_required, int page) +{ + struct hinfc_host *host = chip->priv; + int max_bitflips = 0, stat = 0; + + chip->read_buf(mtd, buf, mtd->writesize); + chip->read_buf(mtd, chip->oob_poi, mtd->oobsize); + + /* errors which can not be corrected by ECC */ + if (host->irq_status & HINFC504_INTS_UE) { + mtd->ecc_stats.failed++; + } else if (host->irq_status & HINFC504_INTS_CE) { + /* need add other ECC modes! */ + switch (host->ecc_bits) { + case 1: + stat = hweight8(hinfc_read(host, HINFC504_ECC_STATUS)>> + HINFC504_ECC_1_BIT_SHIFT); + break; + case 6: + stat = hweight16(hinfc_read(host, HINFC504_ECC_STATUS)>> + HINFC504_ECC_16_BIT_SHIFT & 0x0fff); + } + mtd->ecc_stats.corrected += stat; + max_bitflips = max_t(unsigned int, max_bitflips, stat); + } + host->irq_status = 0; + + return max_bitflips; +} + +static int hisi_nand_write_page_hwecc(struct mtd_info *mtd, + struct nand_chip *chip, const uint8_t *buf, int oob_required) +{ + chip->write_buf(mtd, buf, mtd->writesize); + if (oob_required) + chip->write_buf(mtd, chip->oob_poi, mtd->oobsize); + + return 0; +} + +static void hisi_nfc_host_init(struct mtd_info *mtd, struct hinfc_host *host) +{ + struct nand_chip *chip = mtd->priv; + host->version = hinfc_read(host, HINFC_VERSION); + unsigned int flag = 0; + + host->addr_cycle = 0; + host->addr_value[0] = 0; + host->addr_value[1] = 0; + host->cache_addr_value[0] = ~0; + host->cache_addr_value[1] = ~0; + host->chipselect = 0; + + host->send_cmd_pageprog = hisi_nfc_send_cmd_pageprog; + host->send_cmd_readstart = hisi_nfc_send_cmd_readstart; + host->send_cmd_erase = hisi_nfc_send_cmd_erase; + host->send_cmd_readid = hisi_nfc_send_cmd_readid; + host->send_cmd_status = hisi_nfc_send_cmd_status; + host->send_cmd_reset = hisi_nfc_send_cmd_reset; + + /* default page size: 2K, ecc_none. need modify */ + flag = HINFC504_CON_OP_MODE_NORMAL | HINFC504_CON_READY_BUSY_SEL + | ((0x001 & HINFC504_CON_PAGESIZE_MASK) + << HINFC504_CON_PAGEISZE_SHIFT) + | ((0x0 & HINFC504_CON_ECCTYPE_MASK) + << HINFC504_CON_ECCTYPE_SHIFT) + | ((chip->options & NAND_BUSWIDTH_16) ? + HINFC504_CON_BUS_WIDTH : 0); + hinfc_write(host, flag, HINFC504_CON); + + memset(host->chip->IO_ADDR_R, 0xff, HINFC504_BUFFER_BASE_ADDRESS_LEN); + + hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH, + HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH); + + /* enable dma irq */ + hinfc_write(host, HINFC504_INTEN_DMA, HINFC504_INTEN); +} + +static struct nand_ecclayout nand_ecc_2K_1bit = { + .oobfree = { {24, 40} } +}; + +static struct nand_ecclayout nand_ecc_2K_16bits = { + .oobavail = 6, + .oobfree = { {2, 6} }, +}; + +static int hisi_nfc_ecc_probe(struct mtd_info *mtd, struct hinfc_host *host) +{ + struct nand_chip *chip = host->chip; + unsigned int flag; + + chip->ecc.read_page = hisi_nand_read_page_hwecc; + chip->ecc.write_page = hisi_nand_write_page_hwecc; + + switch (host->ecc_bits) { + case 1: + chip->ecc.layout = &nand_ecc_2K_1bit; + chip->ecc.strength = 1; + chip->ecc.size = 512; + break; + case 6: + chip->ecc.layout = &nand_ecc_2K_16bits; + chip->ecc.strength = 16; + chip->ecc.size = 1024; + } + + flag = hinfc_read(host, HINFC504_CON); + /* add ecc type configure */ + flag |= ((host->ecc_bits & HINFC504_CON_ECCTYPE_MASK) + << HINFC504_CON_ECCTYPE_SHIFT); + hinfc_write(host, flag, HINFC504_CON); + + /* enable ecc irq */ + flag = hinfc_read(host, HINFC504_INTEN) & 0xfff; + hinfc_write(host, flag | HINFC504_INTEN_UE | HINFC504_INTEN_CE, + HINFC504_INTEN); + + return 0; +} + +static int hisi_nfc_probe(struct platform_device *pdev) +{ + int ret = 0, irq, buswidth, flag, max_chips = HINFC504_MAX_CHIP; + struct device *dev = &pdev->dev; + struct hinfc_host *host; + struct nand_chip *chip; + struct mtd_info *mtd; + struct resource *res; + struct device_node *np = dev->of_node; + struct mtd_part_parser_data ppdata; + + host = devm_kzalloc(dev, sizeof(*host) + sizeof(*chip) + sizeof(*mtd), + GFP_KERNEL); + if (!host) + return -ENOMEM; + host->dev = dev; + + platform_set_drvdata(pdev, host); + chip = (struct nand_chip *)&host[1]; + mtd = (struct mtd_info *)&chip[1]; + + irq = platform_get_irq(pdev, 0); + if (irq < 0) { + dev_err(dev, "no IRQ resource defined\n"); + ret = -ENXIO; + goto err_res; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + host->iobase = devm_ioremap_resource(dev, res); + if (IS_ERR(host->iobase)) { + ret = PTR_ERR(host->iobase); + dev_err(dev, "devm_ioremap_resource[0] fail\n"); + goto err_res; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + chip->IO_ADDR_R = chip->IO_ADDR_W = devm_ioremap_resource(dev, res); + if (IS_ERR(chip->IO_ADDR_R)) { + ret = PTR_ERR(chip->IO_ADDR_R); + dev_err(dev, "devm_ioremap_resource[1] fail\n"); + goto err_res; + } + + host->chip = chip; + host->mtd = mtd; + + mtd->priv = chip; + mtd->owner = THIS_MODULE; + mtd->name = "hisi_nand"; + + chip->priv = host; + chip->cmdfunc = hisi_nfc_cmdfunc; + chip->select_chip = hisi_nfc_select_chip; + chip->read_byte = hisi_nfc_read_byte; + chip->read_word = hisi_nfc_read_word; + chip->write_buf = hisi_nfc_write_buf; + chip->read_buf = hisi_nfc_read_buf; + chip->chip_delay = HINFC504_CHIP_DELAY; + + chip->ecc.mode = of_get_nand_ecc_mode(np); + /* read ecc-bits from dts */ + of_property_read_u32(np, "hisi,nand-ecc-bits", &host->ecc_bits); + + buswidth = of_get_nand_bus_width(np); + if (buswidth == 16) + chip->options |= NAND_BUSWIDTH_16; + + hisi_nfc_host_init(mtd, host); + + ret = devm_request_irq(dev, irq, hinfc_irq_handle, IRQF_DISABLED, + "nandc", host); + if (ret) { + dev_err(dev, "failed to request IRQ\n"); + goto err_irq; + } + + ret = nand_scan_ident(mtd, max_chips, NULL); + if (ret) { + ret = -ENODEV; + goto err_ident; + } + + host->buffer = dma_alloc_coherent(dev, mtd->writesize + mtd->oobsize, + &host->dma_buffer, GFP_KERNEL); + if (!host->buffer) { + dev_err(dev, "Can't malloc memory for NAND driver.\n"); + ret = -ENOMEM; + goto err_buf; + } + host->dma_oob = host->dma_buffer + mtd->writesize; + memset(host->buffer, 0xff, mtd->writesize + mtd->oobsize); + + flag = hinfc_read(host, HINFC504_CON); + flag &= ~(HINFC504_CON_PAGESIZE_MASK << HINFC504_CON_PAGEISZE_SHIFT); + switch (mtd->writesize) { + case 2048: + flag |= (0x001 << HINFC504_CON_PAGEISZE_SHIFT); + /* add more pagesize support + * default pagesize has been set in hisi_nfc_host_init + */ + } + hinfc_write(host, flag, HINFC504_CON); + + if (chip->ecc.mode == NAND_ECC_HW) + hisi_nfc_ecc_probe(mtd, host); + + nand_scan_tail(mtd); + + ppdata.of_node = np; + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0); + if (ret) { + dev_err(dev, "Err MTD partition=%d\n", ret); + goto err_mtd; + } + + return 0; + +err_mtd: + nand_release(mtd); +err_ident: +err_irq: +err_buf: + if (host->buffer) + dma_free_coherent(dev, mtd->writesize + mtd->oobsize, + host->buffer, host->dma_buffer); +err_res: + return ret; +} + +static int hisi_nfc_remove(struct platform_device *pdev) +{ + struct hinfc_host *host = platform_get_drvdata(pdev); + struct mtd_info *mtd = host->mtd; + + nand_release(host->mtd); + dma_free_coherent(&pdev->dev, mtd->writesize + mtd->oobsize, + host->buffer, host->dma_buffer); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int hisi_nfc_suspend(struct platform_device *pdev, + pm_message_t state) +{ + struct hinfc_host *host = platform_get_drvdata(pdev); + + while ((hinfc_read(host, HINFC504_STATUS) & 0x1) == 0x0) + ; + + while ((hinfc_read(host, HINFC504_DMA_CTRL)) + & HINFC504_DMA_CTRL_DMA_START) + _cond_resched(); + + return 0; +} + +static int hisi_nfc_resume(struct platform_device *pdev) +{ + int cs; + struct hinfc_host *host = platform_get_drvdata(pdev); + struct nand_chip *chip = host->chip; + + for (cs = 0; cs < chip->numchips; cs++) + host->send_cmd_reset(host, cs); + hinfc_write(host, SET_HINFC504_PWIDTH(HINFC504_W_LATCH, + HINFC504_R_LATCH, HINFC504_RW_LATCH), HINFC504_PWIDTH); + + return 0; +} +#endif +static SIMPLE_DEV_PM_OPS(hisi_nfc_pm_ops, hisi_nfc_suspend, hisi_nfc_resume); + +static const struct of_device_id nfc_id_table[] = { + { .compatible = "hisilicon,nfc504" }, + {} +}; +MODULE_DEVICE_TABLE(of, nfc_id_table); + +static struct platform_driver hisi_nfc_driver = { + .driver = { + .name = "hisi_nand", + .of_match_table = of_match_ptr(nfc_id_table), + .pm = &hisi_nfc_pm_ops, + }, + .probe = hisi_nfc_probe, + .remove = hisi_nfc_remove, +}; + +module_platform_driver(hisi_nfc_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Zhiyong Cai"); +MODULE_AUTHOR("Zhou Wang"); +MODULE_DESCRIPTION("Hisilicon Nand Flash Controller Driver"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang @ 2014-06-30 9:00 ` Arnd Bergmann 2014-06-30 9:59 ` Caizhiyong 2014-07-02 2:07 ` Zhou Wang 2014-06-30 9:45 ` Ivan Khoronzhuk 2014-06-30 10:00 ` Mark Rutland 2 siblings, 2 replies; 16+ messages in thread From: Arnd Bergmann @ 2014-06-30 9:00 UTC (permalink / raw) To: linux-arm-kernel Overall a very nice driver. I didn't find any real bugs, but a few things that could be changed for readability and micro-optimizations: On Monday 30 June 2014 16:03:28 Zhou Wang wrote: > +#define hinfc_read(_host, _reg) readl(_host->iobase + (_reg)) > +#define hinfc_write(_host, _value, _reg)\ > + writel((_value), _host->iobase + (_reg)) I'd recommend making these inline functions rather than macros. > +struct hinfc_host { > + struct nand_chip *chip; > + struct mtd_info *mtd; I notice that you allocate the nand_chip and mtd_info together with the hinfc_host and then assign these pointers. A preferred method is normally to just embed the structures in this case: struct hinfc_host { struct nand_chip chip; struct mtd_info mtd; so you avoid the pointer overhead, and can go back from these two structures to the hinfc_host using container_of(). > + struct device *dev; > + void __iomem *iobase; > + struct completion cmd_complete; > + unsigned int offset; > + unsigned int command; > + int chipselect; > + unsigned int addr_cycle; > + unsigned int addr_value[2]; > + unsigned int cache_addr_value[2]; > + char *buffer; > + dma_addr_t dma_buffer; > + dma_addr_t dma_oob; > + int version; > + unsigned int ecc_bits; > + unsigned int irq_status; /* interrupt status */ > + > + int (*send_cmd_pageprog)(struct hinfc_host *host); > + int (*send_cmd_status)(struct hinfc_host *host); > + int (*send_cmd_readstart)(struct hinfc_host *host); > + int (*send_cmd_erase)(struct hinfc_host *host); > + int (*send_cmd_readid)(struct hinfc_host *host); > + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); > +}; Why do you need function pointers here? The current version of the driver you posted always assigns these to the same functions, so it would be more efficient to just call those directly. If you plan to add another variant later that uses a different set of functions here, you can add the function pointers then, in a separate patch. Also, a common style element is to have a separate constant structure with the function pointers: const struct hinfc_host_ops ops = { .send_cmd_pageprog = hisi_nfc_send_cmd_pageprog, ... }; if you actually end up needing function pointers. That simplifies the code a bit and is slightly more robust to attacks overwriting the function pointers. > + > +void wait_controller_finished(struct hinfc_host *host) > +{ > + unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT; > + int val; > + > + while (time_before(jiffies, timeout)) { > + val = hinfc_read(host, HINFC504_STATUS); > + if (host->command == NAND_CMD_ERASE2) { > + /* nfc is ready */ > + while (!(val & HINFC504_READY)) { > + usleep_range(500, 1000); > + val = hinfc_read(host, HINFC504_STATUS); > + } > + return; > + } else { > + if (val & HINFC504_READY) > + return; > + } > + } > + > + /* wait cmd timeout */ > + dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n"); > +} Since the timeout is much bigger than the wait time here, I guess you can use a wider range for the usleep_range(), e.g. usleep_range(500, 50000); which is nicer to other tasks. You will normally be woken up much earlier. Arnd ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 9:00 ` Arnd Bergmann @ 2014-06-30 9:59 ` Caizhiyong 2014-07-02 2:12 ` Zhou Wang 2014-07-02 2:07 ` Zhou Wang 1 sibling, 1 reply; 16+ messages in thread From: Caizhiyong @ 2014-06-30 9:59 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Arnd Bergmann [mailto:arnd at arndb.de] > Sent: Monday, June 30, 2014 5:01 PM > To: linux-arm-kernel at lists.infradead.org > Cc: Zhou Wang; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar > Gala; Russell King; David Woodhouse; Brian Norris; Grant Likely; Ezequiel > Garcia; Pekon Gupta; Artem Bityutskiy; Alexander Shiyan; Ivan Khoronzhuk; > Jussi Kivilinna; Joern Engel; Randy Dunlap; devicetree at vger.kernel.org; > linux-mtd at lists.infradead.org; linux-doc at vger.kernel.org; > linux-kernel at vger.kernel.org; Caizhiyong; Wangzhou (B) > Subject: Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for > hisilicon hip04 Soc > > > > + struct device *dev; > > + void __iomem *iobase; > > + struct completion cmd_complete; > > + unsigned int offset; > > + unsigned int command; > > + int chipselect; > > + unsigned int addr_cycle; > > + unsigned int addr_value[2]; > > + unsigned int cache_addr_value[2]; > > + char *buffer; > > + dma_addr_t dma_buffer; > > + dma_addr_t dma_oob; > > + int version; > > + unsigned int ecc_bits; > > + unsigned int irq_status; /* interrupt status */ > > + > > + int (*send_cmd_pageprog)(struct hinfc_host *host); > > + int (*send_cmd_status)(struct hinfc_host *host); > > + int (*send_cmd_readstart)(struct hinfc_host *host); > > + int (*send_cmd_erase)(struct hinfc_host *host); > > + int (*send_cmd_readid)(struct hinfc_host *host); > > + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); > > +}; > > Why do you need function pointers here? The current version of the > driver you posted always assigns these to the same functions, so > it would be more efficient to just call those directly. > I agree with you. This feature was originally designed to support a variety version of NAND controllers. But in fact, this feature is not used now. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 9:59 ` Caizhiyong @ 2014-07-02 2:12 ` Zhou Wang 0 siblings, 0 replies; 16+ messages in thread From: Zhou Wang @ 2014-07-02 2:12 UTC (permalink / raw) To: linux-arm-kernel On 2014?06?30? 17:59, Caizhiyong wrote: >> -----Original Message----- >> From: Arnd Bergmann [mailto:arnd at arndb.de] >> Sent: Monday, June 30, 2014 5:01 PM >> To: linux-arm-kernel at lists.infradead.org >> Cc: Zhou Wang; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell; Kumar >> Gala; Russell King; David Woodhouse; Brian Norris; Grant Likely; Ezequiel >> Garcia; Pekon Gupta; Artem Bityutskiy; Alexander Shiyan; Ivan Khoronzhuk; >> Jussi Kivilinna; Joern Engel; Randy Dunlap; devicetree at vger.kernel.org; >> linux-mtd at lists.infradead.org; linux-doc at vger.kernel.org; >> linux-kernel at vger.kernel.org; Caizhiyong; Wangzhou (B) >> Subject: Re: [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for >> hisilicon hip04 Soc >> >> >>> + struct device *dev; >>> + void __iomem *iobase; >>> + struct completion cmd_complete; >>> + unsigned int offset; >>> + unsigned int command; >>> + int chipselect; >>> + unsigned int addr_cycle; >>> + unsigned int addr_value[2]; >>> + unsigned int cache_addr_value[2]; >>> + char *buffer; >>> + dma_addr_t dma_buffer; >>> + dma_addr_t dma_oob; >>> + int version; >>> + unsigned int ecc_bits; >>> + unsigned int irq_status; /* interrupt status */ >>> + >>> + int (*send_cmd_pageprog)(struct hinfc_host *host); >>> + int (*send_cmd_status)(struct hinfc_host *host); >>> + int (*send_cmd_readstart)(struct hinfc_host *host); >>> + int (*send_cmd_erase)(struct hinfc_host *host); >>> + int (*send_cmd_readid)(struct hinfc_host *host); >>> + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); >>> +}; >> >> Why do you need function pointers here? The current version of the >> driver you posted always assigns these to the same functions, so >> it would be more efficient to just call those directly. >> > > I agree with you. > This feature was originally designed to support a variety version of NAND controllers. > But in fact, this feature is not used now. > I will move them out the host, thanks for your opinion! ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 9:00 ` Arnd Bergmann 2014-06-30 9:59 ` Caizhiyong @ 2014-07-02 2:07 ` Zhou Wang 1 sibling, 0 replies; 16+ messages in thread From: Zhou Wang @ 2014-07-02 2:07 UTC (permalink / raw) To: linux-arm-kernel On 2014?06?30? 17:00, Arnd Bergmann wrote: > Overall a very nice driver. I didn't find any real bugs, but a few things > that could be changed for readability and micro-optimizations: > > On Monday 30 June 2014 16:03:28 Zhou Wang wrote: > >> +#define hinfc_read(_host, _reg) readl(_host->iobase + (_reg)) >> +#define hinfc_write(_host, _value, _reg)\ >> + writel((_value), _host->iobase + (_reg)) > > I'd recommend making these inline functions rather than macros. > I will change above macros to inline functions, thanks. >> +struct hinfc_host { >> + struct nand_chip *chip; >> + struct mtd_info *mtd; > > I notice that you allocate the nand_chip and mtd_info together > with the hinfc_host and then assign these pointers. A preferred > method is normally to just embed the structures in this case: > > struct hinfc_host { > struct nand_chip chip; > struct mtd_info mtd; > > so you avoid the pointer overhead, and can go back from these > two structures to the hinfc_host using container_of(). > It is better to embed chip and mtd in host, I will change this, thanks. >> + struct device *dev; >> + void __iomem *iobase; >> + struct completion cmd_complete; >> + unsigned int offset; >> + unsigned int command; >> + int chipselect; >> + unsigned int addr_cycle; >> + unsigned int addr_value[2]; >> + unsigned int cache_addr_value[2]; >> + char *buffer; >> + dma_addr_t dma_buffer; >> + dma_addr_t dma_oob; >> + int version; >> + unsigned int ecc_bits; >> + unsigned int irq_status; /* interrupt status */ >> + >> + int (*send_cmd_pageprog)(struct hinfc_host *host); >> + int (*send_cmd_status)(struct hinfc_host *host); >> + int (*send_cmd_readstart)(struct hinfc_host *host); >> + int (*send_cmd_erase)(struct hinfc_host *host); >> + int (*send_cmd_readid)(struct hinfc_host *host); >> + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); >> +}; > > Why do you need function pointers here? The current version of the > driver you posted always assigns these to the same functions, so > it would be more efficient to just call those directly. > > If you plan to add another variant later that uses a different set > of functions here, you can add the function pointers then, in a > separate patch. Also, a common style element is to have a separate > constant structure with the function pointers: > > const struct hinfc_host_ops ops = { > .send_cmd_pageprog = hisi_nfc_send_cmd_pageprog, > ... > }; > > if you actually end up needing function pointers. That simplifies > the code a bit and is slightly more robust to attacks overwriting > the function pointers. > I will move above function pointers out of host and directly call the functions, thanks for your good comment. >> + >> +void wait_controller_finished(struct hinfc_host *host) >> +{ >> + unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT; >> + int val; >> + >> + while (time_before(jiffies, timeout)) { >> + val = hinfc_read(host, HINFC504_STATUS); >> + if (host->command == NAND_CMD_ERASE2) { >> + /* nfc is ready */ >> + while (!(val & HINFC504_READY)) { >> + usleep_range(500, 1000); >> + val = hinfc_read(host, HINFC504_STATUS); >> + } >> + return; >> + } else { >> + if (val & HINFC504_READY) >> + return; >> + } >> + } >> + >> + /* wait cmd timeout */ >> + dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n"); >> +} > > Since the timeout is much bigger than the wait time here, I guess you > can use a wider range for the usleep_range(), e.g. > > usleep_range(500, 50000); > > which is nicer to other tasks. You will normally be woken up much earlier. > > Arnd > As I know that the operating time of NAND flash erasure is about ms-level, so I set sleep time as usleep_range(500, 1000) here. And the timeout means that relative operations don't finish in this time, so maybe there is something wrong with the operations. In normal situation, after detecting the operation finished, the above function will return before timeout arriving. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang 2014-06-30 9:00 ` Arnd Bergmann @ 2014-06-30 9:45 ` Ivan Khoronzhuk 2014-07-02 2:09 ` Zhou Wang 2014-06-30 10:00 ` Mark Rutland 2 siblings, 1 reply; 16+ messages in thread From: Ivan Khoronzhuk @ 2014-06-30 9:45 UTC (permalink / raw) To: linux-arm-kernel On 06/30/2014 11:03 AM, Zhou Wang wrote: > Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> > --- > drivers/mtd/nand/Kconfig | 5 + > drivers/mtd/nand/Makefile | 1 + > drivers/mtd/nand/hisi_nand.c | 847 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 853 insertions(+) > create mode 100644 drivers/mtd/nand/hisi_nand.c > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index 90ff447..253f8c8 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -510,4 +510,9 @@ config MTD_NAND_XWAY > Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached > to the External Bus Unit (EBU). > > +config MTD_NAND_HISI > + tristate "Support for NAND controller on Hisilicon SoC" > + help > + Enables support for NAND controller on Hisilicon SoC. > + > endif # MTD_NAND > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index 542b568..d0881cf 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o > obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ > obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o > obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ > +obj-$(CONFIG_MTD_NAND_HISI) += hisi_nand.o > > nand-objs := nand_base.o nand_bbt.o > diff --git a/drivers/mtd/nand/hisi_nand.c b/drivers/mtd/nand/hisi_nand.c > new file mode 100644 > index 0000000..fbcb065 > --- /dev/null > +++ b/drivers/mtd/nand/hisi_nand.c > @@ -0,0 +1,847 @@ > +/* > + * Hisilicon NAND Flash controller driver > + * > + * Copyright ? 2012-2014 HiSilicon Technologies Co., Ltd. > + * http://www.hisilicon.com > + * > + * Author: Zhou Wang <wangzhou.bry@gmail.com> > + * The initial developer of the original code is Zhiyong Cai > + * <caizhiyong@huawei.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#include <linux/of.h> > +#include <linux/of_mtd.h> > +#include <linux/mtd/mtd.h> > +#include <linux/sizes.h> > +#include <linux/clk.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/delay.h> > +#include <linux/interrupt.h> > +#include <linux/mtd/nand.h> > +#include <linux/dma-mapping.h> > +#include <linux/platform_device.h> > +#include <linux/mtd/partitions.h> > + > +#define HINFC504_MAX_CHIP (4) > +#define HINFC504_W_LATCH (5) > +#define HINFC504_R_LATCH (7) > +#define HINFC504_RW_LATCH (3) > + > +#define HINFC504_NFC_TIMEOUT (2 * HZ) > +#define HINFC504_NFC_DMA_TIMEOUT (5 * HZ) > +#define HINFC504_CHIP_DELAY (25) > + > +#define HINFC504_REG_BASE_ADDRESS_LEN (0x100) > +#define HINFC504_BUFFER_BASE_ADDRESS_LEN (2048 + 128) > + > +#define HINFC504_ADDR_CYCLE_MASK 0x4 > + > +#define HINFC504_CON 0x00 > +#define HINFC504_CON_OP_MODE_NORMAL (1U << 0) > +#define HINFC504_CON_PAGEISZE_SHIFT (1) > +#define HINFC504_CON_PAGESIZE_MASK (0x07) > +#define HINFC504_CON_BUS_WIDTH (1U << 4) > +#define HINFC504_CON_READY_BUSY_SEL (1U << 8) > +#define HINFC504_CON_ECCTYPE_SHIFT (9) > +#define HINFC504_CON_ECCTYPE_MASK (0x07) > + > +#define HINFC504_PWIDTH 0x04 > +#define SET_HINFC504_PWIDTH(_w_lcnt, _r_lcnt, _rw_hcnt) \ > + ((_w_lcnt) | (((_r_lcnt) & 0x0F) << 4) | (((_rw_hcnt) & 0x0F) << 8)) > + > +#define HINFC504_CMD 0x0C > +#define HINFC504_ADDRL 0x10 > +#define HINFC504_ADDRH 0x14 > +#define HINFC504_DATA_NUM 0x18 > + > +#define HINFC504_OP 0x1C > +#define HINFC504_OP_READ_DATA_EN (1U << 1) > +#define HINFC504_OP_WAIT_READY_EN (1U << 2) > +#define HINFC504_OP_CMD2_EN (1U << 3) > +#define HINFC504_OP_WRITE_DATA_EN (1U << 4) > +#define HINFC504_OP_ADDR_EN (1U << 5) > +#define HINFC504_OP_CMD1_EN (1U << 6) > +#define HINFC504_OP_NF_CS_SHIFT (7) > +#define HINFC504_OP_NF_CS_MASK (3) > +#define HINFC504_OP_ADDR_CYCLE_SHIFT (9) > +#define HINFC504_OP_ADDR_CYCLE_MASK (7) > + > +#define HINFC504_STATUS 0x20 > +#define HINFC504_READY (1U << 0) > + > +#define HINFC504_INTEN 0x24 > +#define HINFC504_INTEN_DMA (1U << 9) > +#define HINFC504_INTEN_UE (1U << 6) > +#define HINFC504_INTEN_CE (1U << 5) > + > +#define HINFC504_INTS 0x28 > +#define HINFC504_INTS_DMA (1U << 9) > +#define HINFC504_INTS_UE (1U << 6) > +#define HINFC504_INTS_CE (1U << 5) > + > +#define HINFC504_INTCLR 0x2C > +#define HINFC504_INTCLR_DMA (1U << 9) > +#define HINFC504_INTCLR_UE (1U << 6) > +#define HINFC504_INTCLR_CE (1U << 5) > + > +#define HINFC504_ECC_STATUS 0x5C > +#define HINFC504_ECC_1_BIT_SHIFT 16 > +#define HINFC504_ECC_16_BIT_SHIFT 12 > + > +#define HINFC504_DMA_CTRL 0x60 > +#define HINFC504_DMA_CTRL_DMA_START (1U << 0) > +#define HINFC504_DMA_CTRL_WE (1U << 1) > +#define HINFC504_DMA_CTRL_DATA_AREA_EN (1U << 2) > +#define HINFC504_DMA_CTRL_OOB_AREA_EN (1U << 3) > +#define HINFC504_DMA_CTRL_BURST4_EN (1U << 4) > +#define HINFC504_DMA_CTRL_BURST8_EN (1U << 5) > +#define HINFC504_DMA_CTRL_BURST16_EN (1U << 6) > +#define HINFC504_DMA_CTRL_ADDR_NUM_SHIFT (7) > +#define HINFC504_DMA_CTRL_ADDR_NUM_MASK (1) > +#define HINFC504_DMA_CTRL_CS_SHIFT (8) > +#define HINFC504_DMA_CTRL_CS_MASK (0x03) > + > +#define HINFC504_DMA_ADDR_DATA 0x64 > +#define HINFC504_DMA_ADDR_OOB 0x68 > + > +#define HINFC504_DMA_LEN 0x6C > +#define HINFC504_DMA_LEN_OOB_SHIFT (16) > +#define HINFC504_DMA_LEN_OOB_MASK (0xFFF) > + > +#define HINFC504_DMA_PARA 0x70 > +#define HINFC504_DMA_PARA_DATA_RW_EN (1U << 0) > +#define HINFC504_DMA_PARA_OOB_RW_EN (1U << 1) > +#define HINFC504_DMA_PARA_DATA_EDC_EN (1U << 2) > +#define HINFC504_DMA_PARA_OOB_EDC_EN (1U << 3) > +#define HINFC504_DMA_PARA_DATA_ECC_EN (1U << 4) > +#define HINFC504_DMA_PARA_OOB_ECC_EN (1U << 5) > + > +#define HINFC_VERSION 0x74 > +#define HINFC504_LOG_READ_ADDR 0x7C > +#define HINFC504_LOG_READ_LEN 0x80 > + > +#define HINFC504_NANDINFO_LEN 0x10 > + > +#define hinfc_read(_host, _reg) readl(_host->iobase + (_reg)) > +#define hinfc_write(_host, _value, _reg)\ > + writel((_value), _host->iobase + (_reg)) > + > +struct hinfc_host { > + struct nand_chip *chip; > + struct mtd_info *mtd; > + struct device *dev; > + void __iomem *iobase; > + struct completion cmd_complete; > + unsigned int offset; > + unsigned int command; > + int chipselect; > + unsigned int addr_cycle; > + unsigned int addr_value[2]; > + unsigned int cache_addr_value[2]; > + char *buffer; > + dma_addr_t dma_buffer; > + dma_addr_t dma_oob; > + int version; > + unsigned int ecc_bits; > + unsigned int irq_status; /* interrupt status */ > + > + int (*send_cmd_pageprog)(struct hinfc_host *host); > + int (*send_cmd_status)(struct hinfc_host *host); > + int (*send_cmd_readstart)(struct hinfc_host *host); > + int (*send_cmd_erase)(struct hinfc_host *host); > + int (*send_cmd_readid)(struct hinfc_host *host); > + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); > +}; > + > +void wait_controller_finished(struct hinfc_host *host) > +{ > + unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT; > + int val; > + > + while (time_before(jiffies, timeout)) { > + val = hinfc_read(host, HINFC504_STATUS); > + if (host->command == NAND_CMD_ERASE2) { > + /* nfc is ready */ > + while (!(val & HINFC504_READY)) { > + usleep_range(500, 1000); > + val = hinfc_read(host, HINFC504_STATUS); > + } > + return; > + } else { > + if (val & HINFC504_READY) > + return; > + } > + } > + > + /* wait cmd timeout */ > + dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n"); > +} > + > +static void hisi_nfc_dma_transfer(struct hinfc_host *host, int todev) > +{ > + struct mtd_info *mtd = host->mtd; > + struct nand_chip *chip = mtd->priv; > + unsigned long val; > + int ret; > + > + hinfc_write(host, host->dma_buffer, HINFC504_DMA_ADDR_DATA); > + hinfc_write(host, host->dma_oob, HINFC504_DMA_ADDR_OOB); > + > + if (chip->ecc.mode == NAND_ECC_NONE) { > + hinfc_write(host, ((mtd->oobsize & HINFC504_DMA_LEN_OOB_MASK) > + << HINFC504_DMA_LEN_OOB_SHIFT), HINFC504_DMA_LEN); > + > + hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN > + | HINFC504_DMA_PARA_OOB_RW_EN, HINFC504_DMA_PARA); > + } else Add {} for else also ./scripts/checkpatch.pl. -- Regards, Ivan Khoronzhuk ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 9:45 ` Ivan Khoronzhuk @ 2014-07-02 2:09 ` Zhou Wang 0 siblings, 0 replies; 16+ messages in thread From: Zhou Wang @ 2014-07-02 2:09 UTC (permalink / raw) To: linux-arm-kernel On 2014?06?30? 17:45, Ivan Khoronzhuk wrote: > > On 06/30/2014 11:03 AM, Zhou Wang wrote: >> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> >> --- >> drivers/mtd/nand/Kconfig | 5 + >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/hisi_nand.c | 847 >> ++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 853 insertions(+) >> create mode 100644 drivers/mtd/nand/hisi_nand.c >> >> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig >> index 90ff447..253f8c8 100644 >> --- a/drivers/mtd/nand/Kconfig >> +++ b/drivers/mtd/nand/Kconfig >> @@ -510,4 +510,9 @@ config MTD_NAND_XWAY >> Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND >> is attached >> to the External Bus Unit (EBU). >> +config MTD_NAND_HISI >> + tristate "Support for NAND controller on Hisilicon SoC" >> + help >> + Enables support for NAND controller on Hisilicon SoC. >> + >> endif # MTD_NAND >> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile >> index 542b568..d0881cf 100644 >> --- a/drivers/mtd/nand/Makefile >> +++ b/drivers/mtd/nand/Makefile >> @@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o >> obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ >> obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o >> obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ >> +obj-$(CONFIG_MTD_NAND_HISI) += hisi_nand.o >> nand-objs := nand_base.o nand_bbt.o >> diff --git a/drivers/mtd/nand/hisi_nand.c b/drivers/mtd/nand/hisi_nand.c >> new file mode 100644 >> index 0000000..fbcb065 >> --- /dev/null >> +++ b/drivers/mtd/nand/hisi_nand.c >> @@ -0,0 +1,847 @@ >> +/* >> + * Hisilicon NAND Flash controller driver >> + * >> + * Copyright ? 2012-2014 HiSilicon Technologies Co., Ltd. >> + * http://www.hisilicon.com >> + * >> + * Author: Zhou Wang <wangzhou.bry@gmail.com> >> + * The initial developer of the original code is Zhiyong Cai >> + * <caizhiyong@huawei.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> +#include <linux/of.h> >> +#include <linux/of_mtd.h> >> +#include <linux/mtd/mtd.h> >> +#include <linux/sizes.h> >> +#include <linux/clk.h> >> +#include <linux/slab.h> >> +#include <linux/module.h> >> +#include <linux/delay.h> >> +#include <linux/interrupt.h> >> +#include <linux/mtd/nand.h> >> +#include <linux/dma-mapping.h> >> +#include <linux/platform_device.h> >> +#include <linux/mtd/partitions.h> >> + >> +#define HINFC504_MAX_CHIP (4) >> +#define HINFC504_W_LATCH (5) >> +#define HINFC504_R_LATCH (7) >> +#define HINFC504_RW_LATCH (3) >> + >> +#define HINFC504_NFC_TIMEOUT (2 * HZ) >> +#define HINFC504_NFC_DMA_TIMEOUT (5 * HZ) >> +#define HINFC504_CHIP_DELAY (25) >> + >> +#define HINFC504_REG_BASE_ADDRESS_LEN (0x100) >> +#define HINFC504_BUFFER_BASE_ADDRESS_LEN (2048 + 128) >> + >> +#define HINFC504_ADDR_CYCLE_MASK 0x4 >> + >> +#define HINFC504_CON 0x00 >> +#define HINFC504_CON_OP_MODE_NORMAL (1U << 0) >> +#define HINFC504_CON_PAGEISZE_SHIFT (1) >> +#define HINFC504_CON_PAGESIZE_MASK (0x07) >> +#define HINFC504_CON_BUS_WIDTH (1U << 4) >> +#define HINFC504_CON_READY_BUSY_SEL (1U << 8) >> +#define HINFC504_CON_ECCTYPE_SHIFT (9) >> +#define HINFC504_CON_ECCTYPE_MASK (0x07) >> + >> +#define HINFC504_PWIDTH 0x04 >> +#define SET_HINFC504_PWIDTH(_w_lcnt, _r_lcnt, _rw_hcnt) \ >> + ((_w_lcnt) | (((_r_lcnt) & 0x0F) << 4) | (((_rw_hcnt) & 0x0F) << 8)) >> + >> +#define HINFC504_CMD 0x0C >> +#define HINFC504_ADDRL 0x10 >> +#define HINFC504_ADDRH 0x14 >> +#define HINFC504_DATA_NUM 0x18 >> + >> +#define HINFC504_OP 0x1C >> +#define HINFC504_OP_READ_DATA_EN (1U << 1) >> +#define HINFC504_OP_WAIT_READY_EN (1U << 2) >> +#define HINFC504_OP_CMD2_EN (1U << 3) >> +#define HINFC504_OP_WRITE_DATA_EN (1U << 4) >> +#define HINFC504_OP_ADDR_EN (1U << 5) >> +#define HINFC504_OP_CMD1_EN (1U << 6) >> +#define HINFC504_OP_NF_CS_SHIFT (7) >> +#define HINFC504_OP_NF_CS_MASK (3) >> +#define HINFC504_OP_ADDR_CYCLE_SHIFT (9) >> +#define HINFC504_OP_ADDR_CYCLE_MASK (7) >> + >> +#define HINFC504_STATUS 0x20 >> +#define HINFC504_READY (1U << 0) >> + >> +#define HINFC504_INTEN 0x24 >> +#define HINFC504_INTEN_DMA (1U << 9) >> +#define HINFC504_INTEN_UE (1U << 6) >> +#define HINFC504_INTEN_CE (1U << 5) >> + >> +#define HINFC504_INTS 0x28 >> +#define HINFC504_INTS_DMA (1U << 9) >> +#define HINFC504_INTS_UE (1U << 6) >> +#define HINFC504_INTS_CE (1U << 5) >> + >> +#define HINFC504_INTCLR 0x2C >> +#define HINFC504_INTCLR_DMA (1U << 9) >> +#define HINFC504_INTCLR_UE (1U << 6) >> +#define HINFC504_INTCLR_CE (1U << 5) >> + >> +#define HINFC504_ECC_STATUS 0x5C >> +#define HINFC504_ECC_1_BIT_SHIFT 16 >> +#define HINFC504_ECC_16_BIT_SHIFT 12 >> + >> +#define HINFC504_DMA_CTRL 0x60 >> +#define HINFC504_DMA_CTRL_DMA_START (1U << 0) >> +#define HINFC504_DMA_CTRL_WE (1U << 1) >> +#define HINFC504_DMA_CTRL_DATA_AREA_EN (1U << 2) >> +#define HINFC504_DMA_CTRL_OOB_AREA_EN (1U << 3) >> +#define HINFC504_DMA_CTRL_BURST4_EN (1U << 4) >> +#define HINFC504_DMA_CTRL_BURST8_EN (1U << 5) >> +#define HINFC504_DMA_CTRL_BURST16_EN (1U << 6) >> +#define HINFC504_DMA_CTRL_ADDR_NUM_SHIFT (7) >> +#define HINFC504_DMA_CTRL_ADDR_NUM_MASK (1) >> +#define HINFC504_DMA_CTRL_CS_SHIFT (8) >> +#define HINFC504_DMA_CTRL_CS_MASK (0x03) >> + >> +#define HINFC504_DMA_ADDR_DATA 0x64 >> +#define HINFC504_DMA_ADDR_OOB 0x68 >> + >> +#define HINFC504_DMA_LEN 0x6C >> +#define HINFC504_DMA_LEN_OOB_SHIFT (16) >> +#define HINFC504_DMA_LEN_OOB_MASK (0xFFF) >> + >> +#define HINFC504_DMA_PARA 0x70 >> +#define HINFC504_DMA_PARA_DATA_RW_EN (1U << 0) >> +#define HINFC504_DMA_PARA_OOB_RW_EN (1U << 1) >> +#define HINFC504_DMA_PARA_DATA_EDC_EN (1U << 2) >> +#define HINFC504_DMA_PARA_OOB_EDC_EN (1U << 3) >> +#define HINFC504_DMA_PARA_DATA_ECC_EN (1U << 4) >> +#define HINFC504_DMA_PARA_OOB_ECC_EN (1U << 5) >> + >> +#define HINFC_VERSION 0x74 >> +#define HINFC504_LOG_READ_ADDR 0x7C >> +#define HINFC504_LOG_READ_LEN 0x80 >> + >> +#define HINFC504_NANDINFO_LEN 0x10 >> + >> +#define hinfc_read(_host, _reg) readl(_host->iobase + (_reg)) >> +#define hinfc_write(_host, _value, _reg)\ >> + writel((_value), _host->iobase + (_reg)) >> + >> +struct hinfc_host { >> + struct nand_chip *chip; >> + struct mtd_info *mtd; >> + struct device *dev; >> + void __iomem *iobase; >> + struct completion cmd_complete; >> + unsigned int offset; >> + unsigned int command; >> + int chipselect; >> + unsigned int addr_cycle; >> + unsigned int addr_value[2]; >> + unsigned int cache_addr_value[2]; >> + char *buffer; >> + dma_addr_t dma_buffer; >> + dma_addr_t dma_oob; >> + int version; >> + unsigned int ecc_bits; >> + unsigned int irq_status; /* interrupt status */ >> + >> + int (*send_cmd_pageprog)(struct hinfc_host *host); >> + int (*send_cmd_status)(struct hinfc_host *host); >> + int (*send_cmd_readstart)(struct hinfc_host *host); >> + int (*send_cmd_erase)(struct hinfc_host *host); >> + int (*send_cmd_readid)(struct hinfc_host *host); >> + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); >> +}; >> + >> +void wait_controller_finished(struct hinfc_host *host) >> +{ >> + unsigned long timeout = jiffies + HINFC504_NFC_TIMEOUT; >> + int val; >> + >> + while (time_before(jiffies, timeout)) { >> + val = hinfc_read(host, HINFC504_STATUS); >> + if (host->command == NAND_CMD_ERASE2) { >> + /* nfc is ready */ >> + while (!(val & HINFC504_READY)) { >> + usleep_range(500, 1000); >> + val = hinfc_read(host, HINFC504_STATUS); >> + } >> + return; >> + } else { >> + if (val & HINFC504_READY) >> + return; >> + } >> + } >> + >> + /* wait cmd timeout */ >> + dev_err(host->dev, "Wait NAND controller exec cmd timeout.\n"); >> +} >> + >> +static void hisi_nfc_dma_transfer(struct hinfc_host *host, int todev) >> +{ >> + struct mtd_info *mtd = host->mtd; >> + struct nand_chip *chip = mtd->priv; >> + unsigned long val; >> + int ret; >> + >> + hinfc_write(host, host->dma_buffer, HINFC504_DMA_ADDR_DATA); >> + hinfc_write(host, host->dma_oob, HINFC504_DMA_ADDR_OOB); >> + >> + if (chip->ecc.mode == NAND_ECC_NONE) { >> + hinfc_write(host, ((mtd->oobsize & HINFC504_DMA_LEN_OOB_MASK) >> + << HINFC504_DMA_LEN_OOB_SHIFT), HINFC504_DMA_LEN); >> + >> + hinfc_write(host, HINFC504_DMA_PARA_DATA_RW_EN >> + | HINFC504_DMA_PARA_OOB_RW_EN, HINFC504_DMA_PARA); >> + } else > > Add {} for else also > ./scripts/checkpatch.pl. > ok, I will add them, thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang 2014-06-30 9:00 ` Arnd Bergmann 2014-06-30 9:45 ` Ivan Khoronzhuk @ 2014-06-30 10:00 ` Mark Rutland 2014-07-02 2:15 ` Zhou Wang 2 siblings, 1 reply; 16+ messages in thread From: Mark Rutland @ 2014-06-30 10:00 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 09:03:28AM +0100, Zhou Wang wrote: > Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> > --- > drivers/mtd/nand/Kconfig | 5 + > drivers/mtd/nand/Makefile | 1 + > drivers/mtd/nand/hisi_nand.c | 847 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 853 insertions(+) > create mode 100644 drivers/mtd/nand/hisi_nand.c [...] > +struct hinfc_host { > + struct nand_chip *chip; > + struct mtd_info *mtd; > + struct device *dev; > + void __iomem *iobase; > + struct completion cmd_complete; > + unsigned int offset; > + unsigned int command; > + int chipselect; > + unsigned int addr_cycle; > + unsigned int addr_value[2]; > + unsigned int cache_addr_value[2]; > + char *buffer; > + dma_addr_t dma_buffer; > + dma_addr_t dma_oob; > + int version; > + unsigned int ecc_bits; > + unsigned int irq_status; /* interrupt status */ > + > + int (*send_cmd_pageprog)(struct hinfc_host *host); > + int (*send_cmd_status)(struct hinfc_host *host); > + int (*send_cmd_readstart)(struct hinfc_host *host); > + int (*send_cmd_erase)(struct hinfc_host *host); > + int (*send_cmd_readid)(struct hinfc_host *host); > + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); > +}; [...] > +static int hisi_nfc_probe(struct platform_device *pdev) > +{ > + int ret = 0, irq, buswidth, flag, max_chips = HINFC504_MAX_CHIP; > + struct device *dev = &pdev->dev; > + struct hinfc_host *host; > + struct nand_chip *chip; > + struct mtd_info *mtd; > + struct resource *res; > + struct device_node *np = dev->of_node; > + struct mtd_part_parser_data ppdata; > + > + host = devm_kzalloc(dev, sizeof(*host) + sizeof(*chip) + sizeof(*mtd), > + GFP_KERNEL); > + if (!host) > + return -ENOMEM; > + host->dev = dev; > + > + platform_set_drvdata(pdev, host); > + chip = (struct nand_chip *)&host[1]; > + mtd = (struct mtd_info *)&chip[1]; Why not embed the whole struct rather than pointers? Then you can allocate just the host and extract pointers to the chip and mtd sub structures. Thanks, Mark. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc 2014-06-30 10:00 ` Mark Rutland @ 2014-07-02 2:15 ` Zhou Wang 0 siblings, 0 replies; 16+ messages in thread From: Zhou Wang @ 2014-07-02 2:15 UTC (permalink / raw) To: linux-arm-kernel On 2014?06?30? 18:00, Mark Rutland wrote: > On Mon, Jun 30, 2014 at 09:03:28AM +0100, Zhou Wang wrote: >> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> >> --- >> drivers/mtd/nand/Kconfig | 5 + >> drivers/mtd/nand/Makefile | 1 + >> drivers/mtd/nand/hisi_nand.c | 847 ++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 853 insertions(+) >> create mode 100644 drivers/mtd/nand/hisi_nand.c > > [...] > >> +struct hinfc_host { >> + struct nand_chip *chip; >> + struct mtd_info *mtd; >> + struct device *dev; >> + void __iomem *iobase; >> + struct completion cmd_complete; >> + unsigned int offset; >> + unsigned int command; >> + int chipselect; >> + unsigned int addr_cycle; >> + unsigned int addr_value[2]; >> + unsigned int cache_addr_value[2]; >> + char *buffer; >> + dma_addr_t dma_buffer; >> + dma_addr_t dma_oob; >> + int version; >> + unsigned int ecc_bits; >> + unsigned int irq_status; /* interrupt status */ >> + >> + int (*send_cmd_pageprog)(struct hinfc_host *host); >> + int (*send_cmd_status)(struct hinfc_host *host); >> + int (*send_cmd_readstart)(struct hinfc_host *host); >> + int (*send_cmd_erase)(struct hinfc_host *host); >> + int (*send_cmd_readid)(struct hinfc_host *host); >> + int (*send_cmd_reset)(struct hinfc_host *host, int chipselect); >> +}; > > [...] > >> +static int hisi_nfc_probe(struct platform_device *pdev) >> +{ >> + int ret = 0, irq, buswidth, flag, max_chips = HINFC504_MAX_CHIP; >> + struct device *dev = &pdev->dev; >> + struct hinfc_host *host; >> + struct nand_chip *chip; >> + struct mtd_info *mtd; >> + struct resource *res; >> + struct device_node *np = dev->of_node; >> + struct mtd_part_parser_data ppdata; >> + >> + host = devm_kzalloc(dev, sizeof(*host) + sizeof(*chip) + sizeof(*mtd), >> + GFP_KERNEL); >> + if (!host) >> + return -ENOMEM; >> + host->dev = dev; >> + >> + platform_set_drvdata(pdev, host); >> + chip = (struct nand_chip *)&host[1]; >> + mtd = (struct mtd_info *)&chip[1]; > > Why not embed the whole struct rather than pointers? Then you can > allocate just the host and extract pointers to the chip and mtd sub > structures. > > Thanks, > Mark. > I will change this as your comment, thanks. > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] mtd: hisilicon: add device tree binding documentation 2014-06-30 8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang 2014-06-30 8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang 2014-06-30 8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang @ 2014-06-30 8:03 ` Zhou Wang 2014-06-30 9:52 ` Mark Rutland 2 siblings, 1 reply; 16+ messages in thread From: Zhou Wang @ 2014-06-30 8:03 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> --- .../devicetree/bindings/mtd/hisi-nand.txt | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt diff --git a/Documentation/devicetree/bindings/mtd/hisi-nand.txt b/Documentation/devicetree/bindings/mtd/hisi-nand.txt new file mode 100644 index 0000000..1cc6470 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/hisi-nand.txt @@ -0,0 +1,38 @@ +Hisilicon Hip04 Soc NAND controller DT binding + +Required properties: +- compatible: Should be "hisilicon,nfc504". +- reg: Contain registers location and length for reg and data. +- interrupts: Interrupt number for nfc. +- nand-bus-width: See nand.txt. +- nand-ecc-mode: See nand.txt. +- hisi,nand-ecc-bits: ECC bits type support. + <0>: none ecc + <1>: Can correct 1bit per 512byte. + <6>: Can correct 16bits per 1K byte. +- #address-cells: partition address. +- #size-cells: partition size. + +Flash chip may optionally contain additional sub-nodes describing partitions of +the address space. See partition.txt for more detail. + +Example: + + nand: nand at 4020000 { + compatible = "hisilicon,nfc504"; + reg = <0x4020000 0x10000>, <0x5000000 0x1000>; + interrupts = <0 379 4>; + nand-bus-width = <8>; + nand-ecc-mode = "hw"; + hisi,nand-ecc-bits = <1>; + #address-cells = <1>; + #size-cells = <1>; + + partition at 0 { + label = "nand_text"; + reg = <0x00000000 0x00400000>; + }; + + ... + + }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] mtd: hisilicon: add device tree binding documentation 2014-06-30 8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang @ 2014-06-30 9:52 ` Mark Rutland 2014-07-02 2:46 ` Zhou Wang 0 siblings, 1 reply; 16+ messages in thread From: Mark Rutland @ 2014-06-30 9:52 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jun 30, 2014 at 09:03:29AM +0100, Zhou Wang wrote: > Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> > --- > .../devicetree/bindings/mtd/hisi-nand.txt | 38 ++++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt > > diff --git a/Documentation/devicetree/bindings/mtd/hisi-nand.txt b/Documentation/devicetree/bindings/mtd/hisi-nand.txt > new file mode 100644 > index 0000000..1cc6470 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/hisi-nand.txt > @@ -0,0 +1,38 @@ > +Hisilicon Hip04 Soc NAND controller DT binding > + > +Required properties: > +- compatible: Should be "hisilicon,nfc504". > +- reg: Contain registers location and length for reg and data. I'm not sure I follow. The example below has two reg entries. What does each entry represent, and what does the presence of multiple entries mean? > +- interrupts: Interrupt number for nfc. Just the one? > +- nand-bus-width: See nand.txt. > +- nand-ecc-mode: See nand.txt. > +- hisi,nand-ecc-bits: ECC bits type support. > + <0>: none ecc > + <1>: Can correct 1bit per 512byte. > + <6>: Can correct 16bits per 1K byte. Is this an enumeration, or a number of bits? > +- #address-cells: partition address. > +- #size-cells: partition size. Are these only allowed to be 1 cell, or can they be more? Thanks, Mark. > + > +Flash chip may optionally contain additional sub-nodes describing partitions of > +the address space. See partition.txt for more detail. > + > +Example: > + > + nand: nand at 4020000 { > + compatible = "hisilicon,nfc504"; > + reg = <0x4020000 0x10000>, <0x5000000 0x1000>; > + interrupts = <0 379 4>; > + nand-bus-width = <8>; > + nand-ecc-mode = "hw"; > + hisi,nand-ecc-bits = <1>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + partition at 0 { > + label = "nand_text"; > + reg = <0x00000000 0x00400000>; > + }; > + > + ... > + > + }; > -- > 1.7.9.5 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] mtd: hisilicon: add device tree binding documentation 2014-06-30 9:52 ` Mark Rutland @ 2014-07-02 2:46 ` Zhou Wang 0 siblings, 0 replies; 16+ messages in thread From: Zhou Wang @ 2014-07-02 2:46 UTC (permalink / raw) To: linux-arm-kernel On 2014?06?30? 17:52, Mark Rutland wrote: > On Mon, Jun 30, 2014 at 09:03:29AM +0100, Zhou Wang wrote: >> Signed-off-by: Zhou Wang <wangzhou.bry@gmail.com> >> --- >> .../devicetree/bindings/mtd/hisi-nand.txt | 38 ++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mtd/hisi-nand.txt >> >> diff --git a/Documentation/devicetree/bindings/mtd/hisi-nand.txt b/Documentation/devicetree/bindings/mtd/hisi-nand.txt >> new file mode 100644 >> index 0000000..1cc6470 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mtd/hisi-nand.txt >> @@ -0,0 +1,38 @@ >> +Hisilicon Hip04 Soc NAND controller DT binding >> + >> +Required properties: >> +- compatible: Should be "hisilicon,nfc504". >> +- reg: Contain registers location and length for reg and data. > > I'm not sure I follow. The example below has two reg entries. What does > each entry represent, and what does the presence of multiple entries > mean? > Maybe I could not express it clearly here. The first reg entry represents the basic address of registers of NAND controller, and the second represents the basic address of buffer of NAND controller. >> +- interrupts: Interrupt number for nfc. > > Just the one? > Yes, Just one interrupt number. And there is an interrupt relative register in the NAND controller, we can get information about interrupt from the register after an interrupt triggered. >> +- nand-bus-width: See nand.txt. >> +- nand-ecc-mode: See nand.txt. >> +- hisi,nand-ecc-bits: ECC bits type support. >> + <0>: none ecc >> + <1>: Can correct 1bit per 512byte. >> + <6>: Can correct 16bits per 1K byte. > > Is this an enumeration, or a number of bits? > This is an enumeration which is just relative register configure, so we can directly use it to configure relative register bits in driver. >> +- #address-cells: partition address. >> +- #size-cells: partition size. > > Are these only allowed to be 1 cell, or can they be more? > > Thanks, > Mark. > These are only allowed to be 1 cell. Address-cells represents basic address of a partition, and size-cells represents the size of a partition. I will rewrite the unclear parts as your comments. Thanks for your comments. -Zhou Wang >> + >> +Flash chip may optionally contain additional sub-nodes describing partitions of >> +the address space. See partition.txt for more detail. >> + >> +Example: >> + >> + nand: nand at 4020000 { >> + compatible = "hisilicon,nfc504"; >> + reg = <0x4020000 0x10000>, <0x5000000 0x1000>; >> + interrupts = <0 379 4>; >> + nand-bus-width = <8>; >> + nand-ecc-mode = "hw"; >> + hisi,nand-ecc-bits = <1>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + partition at 0 { >> + label = "nand_text"; >> + reg = <0x00000000 0x00400000>; >> + }; >> + >> + ... >> + >> + }; >> -- >> 1.7.9.5 >> >> > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-07-11 2:40 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-30 8:03 [PATCH 0/3] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc Zhou Wang 2014-06-30 8:03 ` [PATCH 1/3] mtd: hisilicon: add device tree node for nand controller Zhou Wang 2014-07-09 7:08 ` Jerome FORISSIER 2014-07-11 2:40 ` Zhou Wang 2014-06-30 8:03 ` [PATCH 2/3] mtd: hisilicon: add a new nand controller driver for hisilicon hip04 Soc Zhou Wang 2014-06-30 9:00 ` Arnd Bergmann 2014-06-30 9:59 ` Caizhiyong 2014-07-02 2:12 ` Zhou Wang 2014-07-02 2:07 ` Zhou Wang 2014-06-30 9:45 ` Ivan Khoronzhuk 2014-07-02 2:09 ` Zhou Wang 2014-06-30 10:00 ` Mark Rutland 2014-07-02 2:15 ` Zhou Wang 2014-06-30 8:03 ` [PATCH 3/3] mtd: hisilicon: add device tree binding documentation Zhou Wang 2014-06-30 9:52 ` Mark Rutland 2014-07-02 2:46 ` Zhou Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).