diff mbox

[v6,2/2] mtd: nand: Add support for Arasan Nand Flash Controller

Message ID 1480911066-26157-2-git-send-email-punnaia@xilinx.com
State Superseded
Headers show

Commit Message

Punnaiah Choudary Kalluri Dec. 5, 2016, 4:11 a.m. UTC
Added the basic driver for Arasan Nand Flash Controller used in
Zynq UltraScale+ MPSoC. It supports only Hw Ecc and upto 24bit
correction.

Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
---
Chnages in v6:
- Addressed most of the Brian and Boris comments
- Separated the nandchip from the nand controller
- Removed the ecc lookup table from driver
- Now use framework nand waitfunction and readoob
- Fixed the compiler warning
- Adapted the new frameowrk changes related to ecc and ooblayout
- Disabled the clocks after the nand_reelase
- Now using only one completion object
- Boris suggessions like adapting cmd_ctrl and rework on read/write byte
  are not implemented and i will patch them later
- Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr will
  implement later once the basic driver is mainlined. 
Changes in v5:
- Renamed the driver filei as arasan_nand.c
- Fixed all comments relaqted coding style
- Fixed comments related to propagating the errors
- Modified the anfc_write_page_hwecc as per the write_page
  prototype
Changes in v4:
- Added support for onfi timing mode configuration
- Added clock supppport
- Added support for multiple chipselects
Changes in v3:
- Removed unused variables
- Avoided busy loop and used jifies based implementation
- Fixed compiler warnings "right shift count >= width of type"
- Removed unneeded codei and improved error reporting
- Added onfi version check to ensure reading the valid address cycles
Changes in v2:
- Added missing of.h to avoid kbuild system report erro
---
 drivers/mtd/nand/Kconfig       |   8 +
 drivers/mtd/nand/Makefile      |   1 +
 drivers/mtd/nand/arasan_nand.c | 974 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 983 insertions(+)
 create mode 100644 drivers/mtd/nand/arasan_nand.c

Comments

Marek Vasut Dec. 5, 2016, 4:40 a.m. UTC | #1
On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> Added the basic driver for Arasan Nand Flash Controller used in
> Zynq UltraScale+ MPSoC. It supports only Hw Ecc and upto 24bit
> correction.

Ummm, NAND, ECC, can you fix the acronyms to be in caps ?

> Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> ---
> Chnages in v6:
> - Addressed most of the Brian and Boris comments
> - Separated the nandchip from the nand controller
> - Removed the ecc lookup table from driver
> - Now use framework nand waitfunction and readoob
> - Fixed the compiler warning
> - Adapted the new frameowrk changes related to ecc and ooblayout
> - Disabled the clocks after the nand_reelase
> - Now using only one completion object
> - Boris suggessions like adapting cmd_ctrl and rework on read/write byte
>   are not implemented and i will patch them later
> - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr will
>   implement later once the basic driver is mainlined. 
> Changes in v5:
> - Renamed the driver filei as arasan_nand.c
> - Fixed all comments relaqted coding style
> - Fixed comments related to propagating the errors
> - Modified the anfc_write_page_hwecc as per the write_page
>   prototype
> Changes in v4:
> - Added support for onfi timing mode configuration
> - Added clock supppport
> - Added support for multiple chipselects
> Changes in v3:
> - Removed unused variables
> - Avoided busy loop and used jifies based implementation
> - Fixed compiler warnings "right shift count >= width of type"
> - Removed unneeded codei and improved error reporting
> - Added onfi version check to ensure reading the valid address cycles
> Changes in v2:
> - Added missing of.h to avoid kbuild system report erro
> ---
>  drivers/mtd/nand/Kconfig       |   8 +
>  drivers/mtd/nand/Makefile      |   1 +
>  drivers/mtd/nand/arasan_nand.c | 974 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 983 insertions(+)
>  create mode 100644 drivers/mtd/nand/arasan_nand.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 7b7a887..e831f4e 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -569,4 +569,12 @@ config MTD_NAND_MTK
>  	  Enables support for NAND controller on MTK SoCs.
>  	  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
>  
> +config MTD_NAND_ARASAN
> +	tristate "Support for Arasan Nand Flash controller"
> +	depends on HAS_IOMEM
> +	depends on HAS_DMA
> +	help
> +	  Enables the driver for the Arasan Nand Flash controller on
> +	  Zynq UltraScale+ MPSoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index cafde6f..44b8b00 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -58,5 +58,6 @@ obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
>  obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
>  obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_nand.o mtk_ecc.o
> +obj-$(CONFIG_MTD_NAND_ARASAN)		+= arasan_nand.o

Keep the list at least reasonably sorted.

>  nand-objs := nand_base.o nand_bbt.o nand_timings.o
> diff --git a/drivers/mtd/nand/arasan_nand.c b/drivers/mtd/nand/arasan_nand.c
> new file mode 100644
> index 0000000..6b0670e
> --- /dev/null
> +++ b/drivers/mtd/nand/arasan_nand.c
> @@ -0,0 +1,974 @@
> +/*
> + * Arasan Nand Flash Controller Driver

NAND

> + * Copyright (C) 2014 - 2015 Xilinx, Inc.

It's 2016 now ...

> + * This program is free software; you can redistribute it and/or modify it under
> + * the terms of the GNU General Public License version 2 as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME			"arasan_nand"
> +#define EVNT_TIMEOUT			1000

Rename to EVENT_TIMEOUT_<units> to make this less cryptic

> +#define STATUS_TIMEOUT			2000

DTTO

> +#define PKT_OFST			0x00
> +#define MEM_ADDR1_OFST			0x04
> +#define MEM_ADDR2_OFST			0x08
> +#define CMD_OFST			0x0C
> +#define PROG_OFST			0x10
> +#define INTR_STS_EN_OFST		0x14
> +#define INTR_SIG_EN_OFST		0x18
> +#define INTR_STS_OFST			0x1C
> +#define READY_STS_OFST			0x20
> +#define DMA_ADDR1_OFST			0x24
> +#define FLASH_STS_OFST			0x28
> +#define DATA_PORT_OFST			0x30
> +#define ECC_OFST			0x34
> +#define ECC_ERR_CNT_OFST		0x38
> +#define ECC_SPR_CMD_OFST		0x3C
> +#define ECC_ERR_CNT_1BIT_OFST		0x40
> +#define ECC_ERR_CNT_2BIT_OFST		0x44
> +#define DMA_ADDR0_OFST			0x50
> +#define DATA_INTERFACE_REG		0x6C

Why are some things suffixed with _OFST and some with _REG ? Consistency
please. Using ARASAN_ prefix, ie. #define ARASAN_FOO 0xbar to define
regs would be nice.

> +#define PKT_CNT_SHIFT			12
> +
> +#define ECC_ENABLE			BIT(31)
> +#define DMA_EN_MASK			GENMASK(27, 26)
> +#define DMA_ENABLE			0x2
> +#define DMA_EN_SHIFT			26
> +#define REG_PAGE_SIZE_MASK		GENMASK(25, 23)
> +#define REG_PAGE_SIZE_SHIFT		23
> +#define REG_PAGE_SIZE_512		0
> +#define REG_PAGE_SIZE_1K		5
> +#define REG_PAGE_SIZE_2K		1
> +#define REG_PAGE_SIZE_4K		2
> +#define REG_PAGE_SIZE_8K		3
> +#define REG_PAGE_SIZE_16K		4
> +#define CMD2_SHIFT			8
> +#define ADDR_CYCLES_SHIFT		28
> +
> +#define XFER_COMPLETE			BIT(2)
> +#define READ_READY			BIT(1)
> +#define WRITE_READY			BIT(0)
> +#define MBIT_ERROR			BIT(3)
> +#define ERR_INTRPT			BIT(4)
> +
> +#define PROG_PGRD			BIT(0)
> +#define PROG_ERASE			BIT(2)
> +#define PROG_STATUS			BIT(3)
> +#define PROG_PGPROG			BIT(4)
> +#define PROG_RDID			BIT(6)
> +#define PROG_RDPARAM			BIT(7)
> +#define PROG_RST			BIT(8)
> +#define PROG_GET_FEATURE		BIT(9)
> +#define PROG_SET_FEATURE		BIT(10)
> +
> +#define ONFI_STATUS_FAIL		BIT(0)
> +#define ONFI_STATUS_READY		BIT(6)
> +
> +#define PG_ADDR_SHIFT			16
> +#define BCH_MODE_SHIFT			25
> +#define BCH_EN_SHIFT			27
> +#define ECC_SIZE_SHIFT			16
> +
> +#define MEM_ADDR_MASK			GENMASK(7, 0)
> +#define BCH_MODE_MASK			GENMASK(27, 25)
> +
> +#define CS_MASK				GENMASK(31, 30)
> +#define CS_SHIFT			30
> +
> +#define PAGE_ERR_CNT_MASK		GENMASK(16, 8)
> +#define PKT_ERR_CNT_MASK		GENMASK(7, 0)
> +
> +#define NVDDR_MODE			BIT(9)
> +#define NVDDR_TIMING_MODE_SHIFT		3
> +
> +#define ONFI_ID_LEN			8
> +#define TEMP_BUF_SIZE			512
> +#define NVDDR_MODE_PACKET_SIZE		8
> +#define SDR_MODE_PACKET_SIZE		4
> +
> +#define ONFI_DATA_INTERFACE_NVDDR      (1 << 4)

BIT() ?


[...]

> +struct anfc {
> +	struct nand_hw_control controller;
> +	struct list_head chips;
> +	struct device *dev;
> +	void __iomem *base;
> +	int curr_cmd;
> +	struct clk *clk_sys;
> +	struct clk *clk_flash;
> +	bool dma;
> +	bool err;
> +	bool iswriteoob;
> +	u8 buf[TEMP_BUF_SIZE];
> +	int irq;
> +	u32 bufshift;
> +	int csnum;
> +	struct completion evnt;

event ?

> +};

[...]

> +static void anfc_prepare_cmd(struct anfc *nfc, u8 cmd1, u8 cmd2, u8 dmamode,
> +			     u32 pagesize, u8 addrcycles)
> +{
> +	u32 regval;
> +
> +	regval = cmd1 | (cmd2 << CMD2_SHIFT);
> +	if (dmamode && nfc->dma)
> +		regval |= DMA_ENABLE << DMA_EN_SHIFT;
> +	if (addrcycles)
> +		regval |= addrcycles << ADDR_CYCLES_SHIFT;
> +	if (pagesize)
> +		regval |= anfc_page(pagesize) << REG_PAGE_SIZE_SHIFT;

Drop the if (foo), if it's zero, the regval would be OR'd with zero.

> +	writel(regval, nfc->base + CMD_OFST);
> +}
> +
> +static int anfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> +			  int page)
> +{
> +	struct anfc *nfc = to_anfc(chip->controller);
> +
> +	nfc->iswriteoob = true;
> +	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page);
> +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +	nfc->iswriteoob = false;
> +
> +	return 0;
> +}
> +
> +static void anfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	u32 pktcount, pktsize, eccintr = 0;
> +	unsigned int buf_rd_cnt = 0;
> +	u32 *bufptr = (u32 *)buf;
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> +	struct anfc *nfc = to_anfc(chip->controller);
> +	dma_addr_t paddr;
> +
> +	if (nfc->curr_cmd == NAND_CMD_READ0) {
> +		pktsize = achip->pktsize;
> +		pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
> +		if (!achip->bch)
> +			eccintr = MBIT_ERROR;
> +	} else {
> +		pktsize = len;
> +		pktcount = 1;
> +	}
> +
> +	anfc_setpktszcnt(nfc, pktsize, pktcount);
> +
> +	if (nfc->dma) {
> +		paddr = dma_map_single(nfc->dev, buf, len, DMA_FROM_DEVICE);
> +		if (dma_mapping_error(nfc->dev, paddr)) {
> +			dev_err(nfc->dev, "Read buffer mapping error");
> +			return;
> +		}
> +		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> +		anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
> +		writel(PROG_PGRD, nfc->base + PROG_OFST);
> +		anfc_wait_for_event(nfc);
> +		dma_unmap_single(nfc->dev, paddr, len, DMA_FROM_DEVICE);

Split this function into anfc_read_buf() and then anfc_read_buf_pio()
and anfc_read_buf_dma() to avoid this ugliness. Also, does this handle
any errors in any way ? Looks like it ignores all errors, so please fix.

> +		return;
> +	}
> +
> +	anfc_enable_intrs(nfc, (READ_READY | eccintr));
> +	writel(PROG_PGRD, nfc->base + PROG_OFST);
> +
> +	while (buf_rd_cnt < pktcount) {
> +		anfc_wait_for_event(nfc);
> +		buf_rd_cnt++;
> +
> +		if (buf_rd_cnt == pktcount)
> +			anfc_enable_intrs(nfc, XFER_COMPLETE);
> +
> +		readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> +		bufptr += (pktsize / 4);
> +
> +		if (buf_rd_cnt < pktcount)
> +			anfc_enable_intrs(nfc, (READ_READY | eccintr));
> +	}
> +
> +	anfc_wait_for_event(nfc);
> +}
> +
> +static void anfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> +{
> +	u32 pktcount, pktsize;
> +	unsigned int buf_wr_cnt = 0;
> +	u32 *bufptr = (u32 *)buf;
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> +	struct anfc *nfc = to_anfc(chip->controller);
> +	dma_addr_t paddr;
> +
> +	if (nfc->iswriteoob) {
> +		pktsize = len;
> +		pktcount = 1;
> +	} else {
> +		pktsize = achip->pktsize;
> +		pktcount = mtd->writesize / pktsize;
> +	}

This looks like a copy of the read path. Can these two functions be
parametrized and merged ?

> +	anfc_setpktszcnt(nfc, pktsize, pktcount);
> +
> +	if (nfc->dma) {
> +		paddr = dma_map_single(nfc->dev, (void *)buf, len,
> +				       DMA_TO_DEVICE);
> +		if (dma_mapping_error(nfc->dev, paddr)) {
> +			dev_err(nfc->dev, "Write buffer mapping error");
> +			return;
> +		}
> +		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> +		anfc_enable_intrs(nfc, XFER_COMPLETE);
> +		writel(PROG_PGPROG, nfc->base + PROG_OFST);
> +		anfc_wait_for_event(nfc);
> +		dma_unmap_single(nfc->dev, paddr, len, DMA_TO_DEVICE);
> +		return;
> +	}
> +
> +	anfc_enable_intrs(nfc, WRITE_READY);
> +	writel(PROG_PGPROG, nfc->base + PROG_OFST);
> +
> +	while (buf_wr_cnt < pktcount) {
> +		anfc_wait_for_event(nfc);
> +		buf_wr_cnt++;
> +		if (buf_wr_cnt == pktcount)
> +			anfc_enable_intrs(nfc, XFER_COMPLETE);
> +
> +		writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> +		bufptr += (pktsize / 4);
> +
> +		if (buf_wr_cnt < pktcount)
> +			anfc_enable_intrs(nfc, WRITE_READY);
> +	}
> +
> +	anfc_wait_for_event(nfc);
> +}


[...]

> +static void anfc_writefifo(struct anfc *nfc, u32 prog, u32 size, u8 *buf)
> +{
> +	u32 *bufptr = (u32 *)buf;
> +
> +	anfc_enable_intrs(nfc, WRITE_READY);
> +
> +	writel(prog, nfc->base + PROG_OFST);
> +	anfc_wait_for_event(nfc);
> +
> +	anfc_enable_intrs(nfc, XFER_COMPLETE);
> +	writesl(nfc->base + DATA_PORT_OFST, bufptr, size/4);

use ioread32_rep and iowrite32_rep , otherwise this won't compile on x86
with COMPILE_TEST.

> +	anfc_wait_for_event(nfc);
> +}
> +
> +static void anfc_readfifo(struct anfc *nfc, u32 prog, u32 size)
> +{
> +	u32 *bufptr = (u32 *)nfc->buf;
> +
> +	anfc_enable_intrs(nfc, READ_READY);
> +
> +	writel(prog, nfc->base + PROG_OFST);
> +	anfc_wait_for_event(nfc);
> +
> +	anfc_enable_intrs(nfc, XFER_COMPLETE);
> +	readsl(nfc->base + DATA_PORT_OFST, bufptr, size/4);

See above

> +	anfc_wait_for_event(nfc);
> +}


[...]

> +static void anfc_select_chip(struct mtd_info *mtd, int num)
> +{
> +	u32 val;
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> +	struct anfc *nfc = to_anfc(chip->controller);
> +
> +	if (num == -1)
> +		return;
> +
> +	val = readl(nfc->base + MEM_ADDR2_OFST);
> +	val = (val & ~(CS_MASK)) | (achip->csnum << CS_SHIFT);
> +	val = (val & ~(BCH_MODE_MASK)) | (achip->bchmode << BCH_MODE_SHIFT);

Just rewrite this as a series of val &= ~(foo | bar); val |= baz | quux;
for clarity sake.

> +	writel(val, nfc->base + MEM_ADDR2_OFST);
> +	nfc->csnum = achip->csnum;
> +	writel(achip->eccval, nfc->base + ECC_OFST);
> +	writel(achip->inftimeval, nfc->base + DATA_INTERFACE_REG);
> +}
> +
> +static irqreturn_t anfc_irq_handler(int irq, void *ptr)
> +{
> +	struct anfc *nfc = ptr;
> +	u32 regval = 0, status;
> +
> +	status = readl(nfc->base + INTR_STS_OFST);
> +	if (status & XFER_COMPLETE) {
> +		complete(&nfc->evnt);
> +		regval |= XFER_COMPLETE;

Can the complete() be invoked multiple times ? That seems a bit odd.

> +	}
> +
> +	if (status & READ_READY) {
> +		complete(&nfc->evnt);
> +		regval |= READ_READY;
> +	}
> +
> +	if (status & WRITE_READY) {
> +		complete(&nfc->evnt);
> +		regval |= WRITE_READY;
> +	}
> +
> +	if (status & MBIT_ERROR) {
> +		nfc->err = true;
> +		complete(&nfc->evnt);
> +		regval |= MBIT_ERROR;
> +	}
> +
> +	if (regval) {
> +		writel(regval, nfc->base + INTR_STS_OFST);
> +		writel(0, nfc->base + INTR_STS_EN_OFST);
> +		writel(0, nfc->base + INTR_SIG_EN_OFST);
> +
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static int anfc_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> +				int addr, uint8_t *subfeature_param)
> +{
> +	struct anfc *nfc = to_anfc(chip->controller);
> +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> +	int status;
> +
> +	if (!chip->onfi_version || !(le16_to_cpu(chip->onfi_params.opt_cmd)
> +		& ONFI_OPT_CMD_SET_GET_FEATURES))

Split this into two conditions to improve readability.

> +		return -EINVAL;
> +
> +	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
> +	anfc_writefifo(nfc, PROG_SET_FEATURE, achip->spktsize,
> +			subfeature_param);
> +
> +	status = chip->waitfunc(mtd, chip);
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int anfc_init_timing_mode(struct anfc *nfc,
> +				 struct anfc_nand_chip *achip)
> +{
> +	int mode, err;
> +	unsigned int feature[2];
> +	u32 inftimeval;
> +	struct nand_chip *chip = &achip->chip;
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +
> +	memset(feature, 0, NVDDR_MODE_PACKET_SIZE);
> +	/* Get nvddr timing modes */
> +	mode = onfi_get_sync_timing_mode(chip) & 0xff;
> +	if (!mode) {
> +		mode = fls(onfi_get_async_timing_mode(chip)) - 1;
> +		inftimeval = mode;
> +	} else {
> +		mode = fls(mode) - 1;
> +		inftimeval = NVDDR_MODE | (mode << NVDDR_TIMING_MODE_SHIFT);
> +		mode |= ONFI_DATA_INTERFACE_NVDDR;
> +	}
> +
> +	feature[0] = mode;
> +	chip->select_chip(mtd, achip->csnum);
> +	err = chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_TIMING_MODE,
> +				      (uint8_t *)feature);
> +	chip->select_chip(mtd, -1);
> +	if (err)
> +		return err;
> +
> +	achip->inftimeval = inftimeval;
> +
> +	if (mode & ONFI_DATA_INTERFACE_NVDDR)
> +		achip->spktsize = NVDDR_MODE_PACKET_SIZE;
> +
> +	return 0;
> +}

[...]

> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Xilinx, Inc");

There should be a contact with email address here.

> +MODULE_DESCRIPTION("Arasan NAND Flash Controller Driver");
>
kernel test robot Dec. 5, 2016, 9:16 a.m. UTC | #2
Hi Punnaiah,

[auto build test ERROR on mtd/master]
[also build test ERROR on v4.9-rc8 next-20161205]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Punnaiah-Choudary-Kalluri/mtd-arasan-Add-device-tree-binding-documentation/20161205-162929
base:   git://git.infradead.org/linux-mtd.git master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/mtd/nand/arasan_nand.c: In function 'anfc_read_buf':
>> drivers/mtd/nand/arasan_nand.c:348:3: error: implicit declaration of function 'readsl' [-Werror=implicit-function-declaration]
      readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
      ^~~~~~
   drivers/mtd/nand/arasan_nand.c: In function 'anfc_write_buf':
>> drivers/mtd/nand/arasan_nand.c:402:3: error: implicit declaration of function 'writesl' [-Werror=implicit-function-declaration]
      writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
      ^~~~~~~
   cc1: some warnings being treated as errors

vim +/readsl +348 drivers/mtd/nand/arasan_nand.c

   342			anfc_wait_for_event(nfc);
   343			buf_rd_cnt++;
   344	
   345			if (buf_rd_cnt == pktcount)
   346				anfc_enable_intrs(nfc, XFER_COMPLETE);
   347	
 > 348			readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
   349			bufptr += (pktsize / 4);
   350	
   351			if (buf_rd_cnt < pktcount)
   352				anfc_enable_intrs(nfc, (READ_READY | eccintr));
   353		}
   354	
   355		anfc_wait_for_event(nfc);
   356	}
   357	
   358	static void anfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
   359	{
   360		u32 pktcount, pktsize;
   361		unsigned int buf_wr_cnt = 0;
   362		u32 *bufptr = (u32 *)buf;
   363		struct nand_chip *chip = mtd_to_nand(mtd);
   364		struct anfc_nand_chip *achip = to_anfc_nand(chip);
   365		struct anfc *nfc = to_anfc(chip->controller);
   366		dma_addr_t paddr;
   367	
   368		if (nfc->iswriteoob) {
   369			pktsize = len;
   370			pktcount = 1;
   371		} else {
   372			pktsize = achip->pktsize;
   373			pktcount = mtd->writesize / pktsize;
   374		}
   375	
   376		anfc_setpktszcnt(nfc, pktsize, pktcount);
   377	
   378		if (nfc->dma) {
   379			paddr = dma_map_single(nfc->dev, (void *)buf, len,
   380					       DMA_TO_DEVICE);
   381			if (dma_mapping_error(nfc->dev, paddr)) {
   382				dev_err(nfc->dev, "Write buffer mapping error");
   383				return;
   384			}
   385			lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
   386			anfc_enable_intrs(nfc, XFER_COMPLETE);
   387			writel(PROG_PGPROG, nfc->base + PROG_OFST);
   388			anfc_wait_for_event(nfc);
   389			dma_unmap_single(nfc->dev, paddr, len, DMA_TO_DEVICE);
   390			return;
   391		}
   392	
   393		anfc_enable_intrs(nfc, WRITE_READY);
   394		writel(PROG_PGPROG, nfc->base + PROG_OFST);
   395	
   396		while (buf_wr_cnt < pktcount) {
   397			anfc_wait_for_event(nfc);
   398			buf_wr_cnt++;
   399			if (buf_wr_cnt == pktcount)
   400				anfc_enable_intrs(nfc, XFER_COMPLETE);
   401	
 > 402			writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
   403			bufptr += (pktsize / 4);
   404	
   405			if (buf_wr_cnt < pktcount)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Punnaiah Choudary Kalluri Dec. 5, 2016, 1:55 p.m. UTC | #3
Hi Marek,

  Thanks for the review and comments.

> -----Original Message-----
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: Monday, December 05, 2016 10:10 AM
> To: Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> dwmw2@infradead.org; computersforpeace@gmail.com;
> boris.brezillon@free-electrons.com; richard@nod.at;
> cyrille.pitchen@atmel.com; robh+dt@kernel.org; mark.rutland@arm.com
> Cc: linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> kalluripunnaiahchoudary@gmail.com; kpc528@gmail.com; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [PATCH v6 2/2] mtd: nand: Add support for Arasan Nand Flash
> Controller
> 
> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > Added the basic driver for Arasan Nand Flash Controller used in
> > Zynq UltraScale+ MPSoC. It supports only Hw Ecc and upto 24bit
> > correction.
> 
> Ummm, NAND, ECC, can you fix the acronyms to be in caps ?
>
 
Ok

> > Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> > ---
> > Chnages in v6:
> > - Addressed most of the Brian and Boris comments
> > - Separated the nandchip from the nand controller
> > - Removed the ecc lookup table from driver
> > - Now use framework nand waitfunction and readoob
> > - Fixed the compiler warning
> > - Adapted the new frameowrk changes related to ecc and ooblayout
> > - Disabled the clocks after the nand_reelase
> > - Now using only one completion object
> > - Boris suggessions like adapting cmd_ctrl and rework on read/write byte
> >   are not implemented and i will patch them later
> > - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr
> will
> >   implement later once the basic driver is mainlined.
> > Changes in v5:
> > - Renamed the driver filei as arasan_nand.c
> > - Fixed all comments relaqted coding style
> > - Fixed comments related to propagating the errors
> > - Modified the anfc_write_page_hwecc as per the write_page
> >   prototype
> > Changes in v4:
> > - Added support for onfi timing mode configuration
> > - Added clock supppport
> > - Added support for multiple chipselects
> > Changes in v3:
> > - Removed unused variables
> > - Avoided busy loop and used jifies based implementation
> > - Fixed compiler warnings "right shift count >= width of type"
> > - Removed unneeded codei and improved error reporting
> > - Added onfi version check to ensure reading the valid address cycles
> > Changes in v2:
> > - Added missing of.h to avoid kbuild system report erro
> > ---
> >  drivers/mtd/nand/Kconfig       |   8 +
> >  drivers/mtd/nand/Makefile      |   1 +
> >  drivers/mtd/nand/arasan_nand.c | 974
> +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 983 insertions(+)
> >  create mode 100644 drivers/mtd/nand/arasan_nand.c
> >
> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > index 7b7a887..e831f4e 100644
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -569,4 +569,12 @@ config MTD_NAND_MTK
> >  	  Enables support for NAND controller on MTK SoCs.
> >  	  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> >
> > +config MTD_NAND_ARASAN
> > +	tristate "Support for Arasan Nand Flash controller"
> > +	depends on HAS_IOMEM
> > +	depends on HAS_DMA
> > +	help
> > +	  Enables the driver for the Arasan Nand Flash controller on
> > +	  Zynq UltraScale+ MPSoC.
> > +
> >  endif # MTD_NAND
> > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> > index cafde6f..44b8b00 100644
> > --- a/drivers/mtd/nand/Makefile
> > +++ b/drivers/mtd/nand/Makefile
> > @@ -58,5 +58,6 @@ obj-$(CONFIG_MTD_NAND_HISI504)	        +=
> hisi504_nand.o
> >  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
> >  obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
> >  obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_nand.o mtk_ecc.o
> > +obj-$(CONFIG_MTD_NAND_ARASAN)		+= arasan_nand.o
> 
> Keep the list at least reasonably sorted.

Ok

> 
> >  nand-objs := nand_base.o nand_bbt.o nand_timings.o
> > diff --git a/drivers/mtd/nand/arasan_nand.c
> b/drivers/mtd/nand/arasan_nand.c
> > new file mode 100644
> > index 0000000..6b0670e
> > --- /dev/null
> > +++ b/drivers/mtd/nand/arasan_nand.c
> > @@ -0,0 +1,974 @@
> > +/*
> > + * Arasan Nand Flash Controller Driver
> 
> NAND
> 
> > + * Copyright (C) 2014 - 2015 Xilinx, Inc.
> 
> It's 2016 now ...

Sorry :). Yes, I will update.

> 
> > + * This program is free software; you can redistribute it and/or modify it
> under
> > + * the terms of the GNU General Public License version 2 as published by
> the
> > + * Free Software Foundation; either version 2 of the License, or (at your
> > + * option) any later version.
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io-64-nonatomic-lo-hi.h>
> > +#include <linux/module.h>
> > +#include <linux/mtd/mtd.h>
> > +#include <linux/mtd/nand.h>
> > +#include <linux/mtd/partitions.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define DRIVER_NAME			"arasan_nand"
> > +#define EVNT_TIMEOUT			1000
> 
> Rename to EVENT_TIMEOUT_<units> to make this less cryptic

Ok.

> 
> > +#define STATUS_TIMEOUT			2000
> 
> DTTO

Thanks. Just realized that it is unused. I will remove this.

> 
> > +#define PKT_OFST			0x00
> > +#define MEM_ADDR1_OFST			0x04
> > +#define MEM_ADDR2_OFST			0x08
> > +#define CMD_OFST			0x0C
> > +#define PROG_OFST			0x10
> > +#define INTR_STS_EN_OFST		0x14
> > +#define INTR_SIG_EN_OFST		0x18
> > +#define INTR_STS_OFST			0x1C
> > +#define READY_STS_OFST			0x20
> > +#define DMA_ADDR1_OFST			0x24
> > +#define FLASH_STS_OFST			0x28
> > +#define DATA_PORT_OFST			0x30
> > +#define ECC_OFST			0x34
> > +#define ECC_ERR_CNT_OFST		0x38
> > +#define ECC_SPR_CMD_OFST		0x3C
> > +#define ECC_ERR_CNT_1BIT_OFST		0x40
> > +#define ECC_ERR_CNT_2BIT_OFST		0x44
> > +#define DMA_ADDR0_OFST			0x50
> > +#define DATA_INTERFACE_REG		0x6C
> 
> Why are some things suffixed with _OFST and some with _REG ? Consistency
> please. Using ARASAN_ prefix, ie. #define ARASAN_FOO 0xbar to define
> regs would be nice.
> 

Ok. I will maintain the consistency. Since all these definitions for arasan controller
and I feel adding the prefix is no value and some places it will go beyond 80 col limit.
Different reviewers have different opinions here. I am following the above convention.  

> > +#define PKT_CNT_SHIFT			12
> > +
> > +#define ECC_ENABLE			BIT(31)
> > +#define DMA_EN_MASK			GENMASK(27, 26)
> > +#define DMA_ENABLE			0x2
> > +#define DMA_EN_SHIFT			26
> > +#define REG_PAGE_SIZE_MASK		GENMASK(25, 23)
> > +#define REG_PAGE_SIZE_SHIFT		23
> > +#define REG_PAGE_SIZE_512		0
> > +#define REG_PAGE_SIZE_1K		5
> > +#define REG_PAGE_SIZE_2K		1
> > +#define REG_PAGE_SIZE_4K		2
> > +#define REG_PAGE_SIZE_8K		3
> > +#define REG_PAGE_SIZE_16K		4
> > +#define CMD2_SHIFT			8
> > +#define ADDR_CYCLES_SHIFT		28
> > +
> > +#define XFER_COMPLETE			BIT(2)
> > +#define READ_READY			BIT(1)
> > +#define WRITE_READY			BIT(0)
> > +#define MBIT_ERROR			BIT(3)
> > +#define ERR_INTRPT			BIT(4)
> > +
> > +#define PROG_PGRD			BIT(0)
> > +#define PROG_ERASE			BIT(2)
> > +#define PROG_STATUS			BIT(3)
> > +#define PROG_PGPROG			BIT(4)
> > +#define PROG_RDID			BIT(6)
> > +#define PROG_RDPARAM			BIT(7)
> > +#define PROG_RST			BIT(8)
> > +#define PROG_GET_FEATURE		BIT(9)
> > +#define PROG_SET_FEATURE		BIT(10)
> > +
> > +#define ONFI_STATUS_FAIL		BIT(0)
> > +#define ONFI_STATUS_READY		BIT(6)
> > +
> > +#define PG_ADDR_SHIFT			16
> > +#define BCH_MODE_SHIFT			25
> > +#define BCH_EN_SHIFT			27
> > +#define ECC_SIZE_SHIFT			16
> > +
> > +#define MEM_ADDR_MASK			GENMASK(7, 0)
> > +#define BCH_MODE_MASK			GENMASK(27, 25)
> > +
> > +#define CS_MASK				GENMASK(31, 30)
> > +#define CS_SHIFT			30
> > +
> > +#define PAGE_ERR_CNT_MASK		GENMASK(16, 8)
> > +#define PKT_ERR_CNT_MASK		GENMASK(7, 0)
> > +
> > +#define NVDDR_MODE			BIT(9)
> > +#define NVDDR_TIMING_MODE_SHIFT		3
> > +
> > +#define ONFI_ID_LEN			8
> > +#define TEMP_BUF_SIZE			512
> > +#define NVDDR_MODE_PACKET_SIZE		8
> > +#define SDR_MODE_PACKET_SIZE		4
> > +
> > +#define ONFI_DATA_INTERFACE_NVDDR      (1 << 4)
> 
> BIT() ?

Sure.

> 
> 
> [...]
> 
> > +struct anfc {
> > +	struct nand_hw_control controller;
> > +	struct list_head chips;
> > +	struct device *dev;
> > +	void __iomem *base;
> > +	int curr_cmd;
> > +	struct clk *clk_sys;
> > +	struct clk *clk_flash;
> > +	bool dma;
> > +	bool err;
> > +	bool iswriteoob;
> > +	u8 buf[TEMP_BUF_SIZE];
> > +	int irq;
> > +	u32 bufshift;
> > +	int csnum;
> > +	struct completion evnt;
> 
> event ?

Ok

> 
> > +};
> 
> [...]
> 
> > +static void anfc_prepare_cmd(struct anfc *nfc, u8 cmd1, u8 cmd2, u8
> dmamode,
> > +			     u32 pagesize, u8 addrcycles)
> > +{
> > +	u32 regval;
> > +
> > +	regval = cmd1 | (cmd2 << CMD2_SHIFT);
> > +	if (dmamode && nfc->dma)
> > +		regval |= DMA_ENABLE << DMA_EN_SHIFT;
> > +	if (addrcycles)
> > +		regval |= addrcycles << ADDR_CYCLES_SHIFT;
> > +	if (pagesize)
> > +		regval |= anfc_page(pagesize) << REG_PAGE_SIZE_SHIFT;
> 
> Drop the if (foo), if it's zero, the regval would be OR'd with zero.

Ok.
> 
> > +	writel(regval, nfc->base + CMD_OFST);
> > +}
> > +
> > +static int anfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> > +			  int page)
> > +{
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +
> > +	nfc->iswriteoob = true;
> > +	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page);
> > +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> > +	nfc->iswriteoob = false;
> > +
> > +	return 0;
> > +}
> > +
> > +static void anfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> > +{
> > +	u32 pktcount, pktsize, eccintr = 0;
> > +	unsigned int buf_rd_cnt = 0;
> > +	u32 *bufptr = (u32 *)buf;
> > +	struct nand_chip *chip = mtd_to_nand(mtd);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +	dma_addr_t paddr;
> > +
> > +	if (nfc->curr_cmd == NAND_CMD_READ0) {
> > +		pktsize = achip->pktsize;
> > +		pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
> > +		if (!achip->bch)
> > +			eccintr = MBIT_ERROR;
> > +	} else {
> > +		pktsize = len;
> > +		pktcount = 1;
> > +	}
> > +
> > +	anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > +	if (nfc->dma) {
> > +		paddr = dma_map_single(nfc->dev, buf, len,
> DMA_FROM_DEVICE);
> > +		if (dma_mapping_error(nfc->dev, paddr)) {
> > +			dev_err(nfc->dev, "Read buffer mapping error");
> > +			return;
> > +		}
> > +		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> > +		anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
> > +		writel(PROG_PGRD, nfc->base + PROG_OFST);
> > +		anfc_wait_for_event(nfc);
> > +		dma_unmap_single(nfc->dev, paddr, len,
> DMA_FROM_DEVICE);
> 
> Split this function into anfc_read_buf() and then anfc_read_buf_pio()
> and anfc_read_buf_dma() to avoid this ugliness. Also, does this handle
> any errors in any way ? Looks like it ignores all errors, so please fix.
> 

Ok. Regarding errors, it handles the errors except checking for contiguous
Address region  for the given address. I will add this in next version.

> > +		return;
> > +	}
> > +
> > +	anfc_enable_intrs(nfc, (READ_READY | eccintr));
> > +	writel(PROG_PGRD, nfc->base + PROG_OFST);
> > +
> > +	while (buf_rd_cnt < pktcount) {
> > +		anfc_wait_for_event(nfc);
> > +		buf_rd_cnt++;
> > +
> > +		if (buf_rd_cnt == pktcount)
> > +			anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +
> > +		readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> > +		bufptr += (pktsize / 4);
> > +
> > +		if (buf_rd_cnt < pktcount)
> > +			anfc_enable_intrs(nfc, (READ_READY | eccintr));
> > +	}
> > +
> > +	anfc_wait_for_event(nfc);
> > +}
> > +
> > +static void anfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int
> len)
> > +{
> > +	u32 pktcount, pktsize;
> > +	unsigned int buf_wr_cnt = 0;
> > +	u32 *bufptr = (u32 *)buf;
> > +	struct nand_chip *chip = mtd_to_nand(mtd);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +	dma_addr_t paddr;
> > +
> > +	if (nfc->iswriteoob) {
> > +		pktsize = len;
> > +		pktcount = 1;
> > +	} else {
> > +		pktsize = achip->pktsize;
> > +		pktcount = mtd->writesize / pktsize;
> > +	}
> 
> This looks like a copy of the read path. Can these two functions be
> parametrized and merged ?
> 
Ok.

> > +	anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > +	if (nfc->dma) {
> > +		paddr = dma_map_single(nfc->dev, (void *)buf, len,
> > +				       DMA_TO_DEVICE);
> > +		if (dma_mapping_error(nfc->dev, paddr)) {
> > +			dev_err(nfc->dev, "Write buffer mapping error");
> > +			return;
> > +		}
> > +		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> > +		anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +		writel(PROG_PGPROG, nfc->base + PROG_OFST);
> > +		anfc_wait_for_event(nfc);
> > +		dma_unmap_single(nfc->dev, paddr, len, DMA_TO_DEVICE);
> > +		return;
> > +	}
> > +
> > +	anfc_enable_intrs(nfc, WRITE_READY);
> > +	writel(PROG_PGPROG, nfc->base + PROG_OFST);
> > +
> > +	while (buf_wr_cnt < pktcount) {
> > +		anfc_wait_for_event(nfc);
> > +		buf_wr_cnt++;
> > +		if (buf_wr_cnt == pktcount)
> > +			anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +
> > +		writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> > +		bufptr += (pktsize / 4);
> > +
> > +		if (buf_wr_cnt < pktcount)
> > +			anfc_enable_intrs(nfc, WRITE_READY);
> > +	}
> > +
> > +	anfc_wait_for_event(nfc);
> > +}
> 
> 
> [...]
> 
> > +static void anfc_writefifo(struct anfc *nfc, u32 prog, u32 size, u8 *buf)
> > +{
> > +	u32 *bufptr = (u32 *)buf;
> > +
> > +	anfc_enable_intrs(nfc, WRITE_READY);
> > +
> > +	writel(prog, nfc->base + PROG_OFST);
> > +	anfc_wait_for_event(nfc);
> > +
> > +	anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +	writesl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
> 
> use ioread32_rep and iowrite32_rep , otherwise this won't compile on x86
> with COMPILE_TEST.
> 

Ok. I have just got the kbuild notification for x86 system. I will fix this.


> > +	anfc_wait_for_event(nfc);
> > +}
> > +
> > +static void anfc_readfifo(struct anfc *nfc, u32 prog, u32 size)
> > +{
> > +	u32 *bufptr = (u32 *)nfc->buf;
> > +
> > +	anfc_enable_intrs(nfc, READ_READY);
> > +
> > +	writel(prog, nfc->base + PROG_OFST);
> > +	anfc_wait_for_event(nfc);
> > +
> > +	anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +	readsl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
> 
> See above
> 
> > +	anfc_wait_for_event(nfc);
> > +}
> 
> 
> [...]
> 
> > +static void anfc_select_chip(struct mtd_info *mtd, int num)
> > +{
> > +	u32 val;
> > +	struct nand_chip *chip = mtd_to_nand(mtd);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +
> > +	if (num == -1)
> > +		return;
> > +
> > +	val = readl(nfc->base + MEM_ADDR2_OFST);
> > +	val = (val & ~(CS_MASK)) | (achip->csnum << CS_SHIFT);
> > +	val = (val & ~(BCH_MODE_MASK)) | (achip->bchmode <<
> BCH_MODE_SHIFT);
> 
> Just rewrite this as a series of val &= ~(foo | bar); val |= baz | quux;
> for clarity sake.
>

 
Ok.

> > +	writel(val, nfc->base + MEM_ADDR2_OFST);
> > +	nfc->csnum = achip->csnum;
> > +	writel(achip->eccval, nfc->base + ECC_OFST);
> > +	writel(achip->inftimeval, nfc->base + DATA_INTERFACE_REG);
> > +}
> > +
> > +static irqreturn_t anfc_irq_handler(int irq, void *ptr)
> > +{
> > +	struct anfc *nfc = ptr;
> > +	u32 regval = 0, status;
> > +
> > +	status = readl(nfc->base + INTR_STS_OFST);
> > +	if (status & XFER_COMPLETE) {
> > +		complete(&nfc->evnt);
> > +		regval |= XFER_COMPLETE;
> 
> Can the complete() be invoked multiple times ? That seems a bit odd.
> 

I will check and fix this.

> > +	}
> > +
> > +	if (status & READ_READY) {
> > +		complete(&nfc->evnt);
> > +		regval |= READ_READY;
> > +	}
> > +
> > +	if (status & WRITE_READY) {
> > +		complete(&nfc->evnt);
> > +		regval |= WRITE_READY;
> > +	}
> > +
> > +	if (status & MBIT_ERROR) {
> > +		nfc->err = true;
> > +		complete(&nfc->evnt);
> > +		regval |= MBIT_ERROR;
> > +	}
> > +
> > +	if (regval) {
> > +		writel(regval, nfc->base + INTR_STS_OFST);
> > +		writel(0, nfc->base + INTR_STS_EN_OFST);
> > +		writel(0, nfc->base + INTR_SIG_EN_OFST);
> > +
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	return IRQ_NONE;
> > +}
> > +
> > +static int anfc_onfi_set_features(struct mtd_info *mtd, struct nand_chip
> *chip,
> > +				int addr, uint8_t *subfeature_param)
> > +{
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	int status;
> > +
> > +	if (!chip->onfi_version || !(le16_to_cpu(chip-
> >onfi_params.opt_cmd)
> > +		& ONFI_OPT_CMD_SET_GET_FEATURES))
> 
> Split this into two conditions to improve readability.
> 

Ok.
> > +		return -EINVAL;
> > +
> > +	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
> > +	anfc_writefifo(nfc, PROG_SET_FEATURE, achip->spktsize,
> > +			subfeature_param);
> > +
> > +	status = chip->waitfunc(mtd, chip);
> > +	if (status & NAND_STATUS_FAIL)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static int anfc_init_timing_mode(struct anfc *nfc,
> > +				 struct anfc_nand_chip *achip)
> > +{
> > +	int mode, err;
> > +	unsigned int feature[2];
> > +	u32 inftimeval;
> > +	struct nand_chip *chip = &achip->chip;
> > +	struct mtd_info *mtd = nand_to_mtd(chip);
> > +
> > +	memset(feature, 0, NVDDR_MODE_PACKET_SIZE);
> > +	/* Get nvddr timing modes */
> > +	mode = onfi_get_sync_timing_mode(chip) & 0xff;
> > +	if (!mode) {
> > +		mode = fls(onfi_get_async_timing_mode(chip)) - 1;
> > +		inftimeval = mode;
> > +	} else {
> > +		mode = fls(mode) - 1;
> > +		inftimeval = NVDDR_MODE | (mode <<
> NVDDR_TIMING_MODE_SHIFT);
> > +		mode |= ONFI_DATA_INTERFACE_NVDDR;
> > +	}
> > +
> > +	feature[0] = mode;
> > +	chip->select_chip(mtd, achip->csnum);
> > +	err = chip->onfi_set_features(mtd, chip,
> ONFI_FEATURE_ADDR_TIMING_MODE,
> > +				      (uint8_t *)feature);
> > +	chip->select_chip(mtd, -1);
> > +	if (err)
> > +		return err;
> > +
> > +	achip->inftimeval = inftimeval;
> > +
> > +	if (mode & ONFI_DATA_INTERFACE_NVDDR)
> > +		achip->spktsize = NVDDR_MODE_PACKET_SIZE;
> > +
> > +	return 0;
> > +}
> 
> [...]
> 
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Xilinx, Inc");
> 
> There should be a contact with email address here.
> 
I think there is an alias for Xilinx, Inc in maintainers list.
If not I will update it.

Regards,
Punnaiah

> > +MODULE_DESCRIPTION("Arasan NAND Flash Controller Driver");
> >
> 
> 
> --
> Best regards,
> Marek Vasut
diff mbox

Patch

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7b7a887..e831f4e 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -569,4 +569,12 @@  config MTD_NAND_MTK
 	  Enables support for NAND controller on MTK SoCs.
 	  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
 
+config MTD_NAND_ARASAN
+	tristate "Support for Arasan Nand Flash controller"
+	depends on HAS_IOMEM
+	depends on HAS_DMA
+	help
+	  Enables the driver for the Arasan Nand Flash controller on
+	  Zynq UltraScale+ MPSoC.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index cafde6f..44b8b00 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -58,5 +58,6 @@  obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_nand.o mtk_ecc.o
+obj-$(CONFIG_MTD_NAND_ARASAN)		+= arasan_nand.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/arasan_nand.c b/drivers/mtd/nand/arasan_nand.c
new file mode 100644
index 0000000..6b0670e
--- /dev/null
+++ b/drivers/mtd/nand/arasan_nand.c
@@ -0,0 +1,974 @@ 
+/*
+ * Arasan Nand Flash Controller Driver
+ *
+ * Copyright (C) 2014 - 2015 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define DRIVER_NAME			"arasan_nand"
+#define EVNT_TIMEOUT			1000
+#define STATUS_TIMEOUT			2000
+
+#define PKT_OFST			0x00
+#define MEM_ADDR1_OFST			0x04
+#define MEM_ADDR2_OFST			0x08
+#define CMD_OFST			0x0C
+#define PROG_OFST			0x10
+#define INTR_STS_EN_OFST		0x14
+#define INTR_SIG_EN_OFST		0x18
+#define INTR_STS_OFST			0x1C
+#define READY_STS_OFST			0x20
+#define DMA_ADDR1_OFST			0x24
+#define FLASH_STS_OFST			0x28
+#define DATA_PORT_OFST			0x30
+#define ECC_OFST			0x34
+#define ECC_ERR_CNT_OFST		0x38
+#define ECC_SPR_CMD_OFST		0x3C
+#define ECC_ERR_CNT_1BIT_OFST		0x40
+#define ECC_ERR_CNT_2BIT_OFST		0x44
+#define DMA_ADDR0_OFST			0x50
+#define DATA_INTERFACE_REG		0x6C
+
+#define PKT_CNT_SHIFT			12
+
+#define ECC_ENABLE			BIT(31)
+#define DMA_EN_MASK			GENMASK(27, 26)
+#define DMA_ENABLE			0x2
+#define DMA_EN_SHIFT			26
+#define REG_PAGE_SIZE_MASK		GENMASK(25, 23)
+#define REG_PAGE_SIZE_SHIFT		23
+#define REG_PAGE_SIZE_512		0
+#define REG_PAGE_SIZE_1K		5
+#define REG_PAGE_SIZE_2K		1
+#define REG_PAGE_SIZE_4K		2
+#define REG_PAGE_SIZE_8K		3
+#define REG_PAGE_SIZE_16K		4
+#define CMD2_SHIFT			8
+#define ADDR_CYCLES_SHIFT		28
+
+#define XFER_COMPLETE			BIT(2)
+#define READ_READY			BIT(1)
+#define WRITE_READY			BIT(0)
+#define MBIT_ERROR			BIT(3)
+#define ERR_INTRPT			BIT(4)
+
+#define PROG_PGRD			BIT(0)
+#define PROG_ERASE			BIT(2)
+#define PROG_STATUS			BIT(3)
+#define PROG_PGPROG			BIT(4)
+#define PROG_RDID			BIT(6)
+#define PROG_RDPARAM			BIT(7)
+#define PROG_RST			BIT(8)
+#define PROG_GET_FEATURE		BIT(9)
+#define PROG_SET_FEATURE		BIT(10)
+
+#define ONFI_STATUS_FAIL		BIT(0)
+#define ONFI_STATUS_READY		BIT(6)
+
+#define PG_ADDR_SHIFT			16
+#define BCH_MODE_SHIFT			25
+#define BCH_EN_SHIFT			27
+#define ECC_SIZE_SHIFT			16
+
+#define MEM_ADDR_MASK			GENMASK(7, 0)
+#define BCH_MODE_MASK			GENMASK(27, 25)
+
+#define CS_MASK				GENMASK(31, 30)
+#define CS_SHIFT			30
+
+#define PAGE_ERR_CNT_MASK		GENMASK(16, 8)
+#define PKT_ERR_CNT_MASK		GENMASK(7, 0)
+
+#define NVDDR_MODE			BIT(9)
+#define NVDDR_TIMING_MODE_SHIFT		3
+
+#define ONFI_ID_LEN			8
+#define TEMP_BUF_SIZE			512
+#define NVDDR_MODE_PACKET_SIZE		8
+#define SDR_MODE_PACKET_SIZE		4
+
+#define ONFI_DATA_INTERFACE_NVDDR      (1 << 4)
+
+/**
+ * struct anfc_nand_chip - Defines the nand chip related information
+ * @node:		used to store NAND chips into a list.
+ * @chip:		NAND chip information structure.
+ * @bch:		Bch / Hamming mode enable/disable.
+ * @bchmode:		Bch mode.
+ * @eccval:		Ecc config value.
+ * @raddr_cycles:	Row address cycle information.
+ * @caddr_cycles:	Column address cycle information.
+ * @pktsize:		Packet size for read / write operation.
+ * @csnum:		chipselect number to be used.
+ * @spktsize:		Packet size in ddr mode for status operation.
+ * @inftimeval		Data interface and timing mode information
+ */
+struct anfc_nand_chip {
+	struct list_head node;
+	struct nand_chip chip;
+	bool bch;
+	u32 bchmode;
+	u32 eccval;
+	u16 raddr_cycles;
+	u16 caddr_cycles;
+	u32 pktsize;
+	int csnum;
+	u32 spktsize;
+	u32 inftimeval;
+};
+
+/**
+ * struct anfc - Defines the Arasan NAND flash driver instance
+ * @controller:		base controller structure.
+ * @chips:		list of all nand chips attached to the ctrler.
+ * @dev:		Pointer to the device structure.
+ * @base:		Virtual address of the NAND flash device.
+ * @curr_cmd:		Current command issued.
+ * @clk_sys:		Pointer to the system clock.
+ * @clk_flash:		Pointer to the flash clock.
+ * @dma:		Dma enable/disable.
+ * @err:		Error identifier.
+ * @iswriteoob:		Identifies if oob write operation is required.
+ * @buf:		Buffer used for read/write byte operations.
+ * @irq:		irq number
+ * @bufshift:		Variable used for indexing buffer operation
+ * @csnum:		Chip select number currently inuse.
+ * @evnt:		Completion event for nand status events.
+ */
+struct anfc {
+	struct nand_hw_control controller;
+	struct list_head chips;
+	struct device *dev;
+	void __iomem *base;
+	int curr_cmd;
+	struct clk *clk_sys;
+	struct clk *clk_flash;
+	bool dma;
+	bool err;
+	bool iswriteoob;
+	u8 buf[TEMP_BUF_SIZE];
+	int irq;
+	u32 bufshift;
+	int csnum;
+	struct completion evnt;
+};
+
+static int anfc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				    struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *nand = mtd_to_nand(mtd);
+
+	if (section)
+		return -ERANGE;
+
+	oobregion->length = nand->ecc.total;
+	oobregion->offset = mtd->oobsize - oobregion->length;
+
+	return 0;
+}
+
+static int anfc_ooblayout_free(struct mtd_info *mtd, int section,
+				     struct mtd_oob_region *oobregion)
+{
+	struct nand_chip *nand = mtd_to_nand(mtd);
+
+	if (section)
+		return -ERANGE;
+
+	oobregion->offset = 2;
+	oobregion->length = mtd->oobsize - nand->ecc.total - 2;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops anfc_ooblayout_ops = {
+	.ecc = anfc_ooblayout_ecc,
+	.free = anfc_ooblayout_free,
+};
+
+static inline struct anfc_nand_chip *to_anfc_nand(struct nand_chip *nand)
+{
+	return container_of(nand, struct anfc_nand_chip, chip);
+}
+
+static inline struct anfc *to_anfc(struct nand_hw_control *ctrl)
+{
+	return container_of(ctrl, struct anfc, controller);
+}
+
+static u8 anfc_page(u32 pagesize)
+{
+	switch (pagesize) {
+	case 512:
+		return REG_PAGE_SIZE_512;
+	case 1024:
+		return REG_PAGE_SIZE_1K;
+	case 2048:
+		return REG_PAGE_SIZE_2K;
+	case 4096:
+		return REG_PAGE_SIZE_4K;
+	case 8192:
+		return REG_PAGE_SIZE_8K;
+	case 16384:
+		return REG_PAGE_SIZE_16K;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static inline void anfc_enable_intrs(struct anfc *nfc, u32 val)
+{
+	writel(val, nfc->base + INTR_STS_EN_OFST);
+	writel(val, nfc->base + INTR_SIG_EN_OFST);
+}
+
+static inline int anfc_wait_for_event(struct anfc *nfc)
+{
+	return wait_for_completion_timeout(&nfc->evnt,
+					msecs_to_jiffies(EVNT_TIMEOUT));
+}
+
+static inline void anfc_setpktszcnt(struct anfc *nfc, u32 pktsize,
+				    u32 pktcount)
+{
+	writel(pktsize | (pktcount << PKT_CNT_SHIFT), nfc->base + PKT_OFST);
+}
+
+static inline void anfc_set_eccsparecmd(struct anfc *nfc,
+				struct anfc_nand_chip *achip, u8 cmd1, u8 cmd2)
+{
+	writel(cmd1 | (cmd2 << CMD2_SHIFT) |
+	       (achip->caddr_cycles << ADDR_CYCLES_SHIFT),
+	       nfc->base + ECC_SPR_CMD_OFST);
+}
+
+static void anfc_setpagecoladdr(struct anfc *nfc, u32 page, u16 col)
+{
+	u32 val;
+
+	writel(col | (page << PG_ADDR_SHIFT), nfc->base + MEM_ADDR1_OFST);
+
+	val = readl(nfc->base + MEM_ADDR2_OFST);
+	val = (val & ~MEM_ADDR_MASK) |
+	      ((page >> PG_ADDR_SHIFT) & MEM_ADDR_MASK);
+	writel(val, nfc->base + MEM_ADDR2_OFST);
+}
+
+static void anfc_prepare_cmd(struct anfc *nfc, u8 cmd1, u8 cmd2, u8 dmamode,
+			     u32 pagesize, u8 addrcycles)
+{
+	u32 regval;
+
+	regval = cmd1 | (cmd2 << CMD2_SHIFT);
+	if (dmamode && nfc->dma)
+		regval |= DMA_ENABLE << DMA_EN_SHIFT;
+	if (addrcycles)
+		regval |= addrcycles << ADDR_CYCLES_SHIFT;
+	if (pagesize)
+		regval |= anfc_page(pagesize) << REG_PAGE_SIZE_SHIFT;
+	writel(regval, nfc->base + CMD_OFST);
+}
+
+static int anfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			  int page)
+{
+	struct anfc *nfc = to_anfc(chip->controller);
+
+	nfc->iswriteoob = true;
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page);
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
+	nfc->iswriteoob = false;
+
+	return 0;
+}
+
+static void anfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	u32 pktcount, pktsize, eccintr = 0;
+	unsigned int buf_rd_cnt = 0;
+	u32 *bufptr = (u32 *)buf;
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+	struct anfc *nfc = to_anfc(chip->controller);
+	dma_addr_t paddr;
+
+	if (nfc->curr_cmd == NAND_CMD_READ0) {
+		pktsize = achip->pktsize;
+		pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
+		if (!achip->bch)
+			eccintr = MBIT_ERROR;
+	} else {
+		pktsize = len;
+		pktcount = 1;
+	}
+
+	anfc_setpktszcnt(nfc, pktsize, pktcount);
+
+	if (nfc->dma) {
+		paddr = dma_map_single(nfc->dev, buf, len, DMA_FROM_DEVICE);
+		if (dma_mapping_error(nfc->dev, paddr)) {
+			dev_err(nfc->dev, "Read buffer mapping error");
+			return;
+		}
+		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
+		anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
+		writel(PROG_PGRD, nfc->base + PROG_OFST);
+		anfc_wait_for_event(nfc);
+		dma_unmap_single(nfc->dev, paddr, len, DMA_FROM_DEVICE);
+		return;
+	}
+
+	anfc_enable_intrs(nfc, (READ_READY | eccintr));
+	writel(PROG_PGRD, nfc->base + PROG_OFST);
+
+	while (buf_rd_cnt < pktcount) {
+		anfc_wait_for_event(nfc);
+		buf_rd_cnt++;
+
+		if (buf_rd_cnt == pktcount)
+			anfc_enable_intrs(nfc, XFER_COMPLETE);
+
+		readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
+		bufptr += (pktsize / 4);
+
+		if (buf_rd_cnt < pktcount)
+			anfc_enable_intrs(nfc, (READ_READY | eccintr));
+	}
+
+	anfc_wait_for_event(nfc);
+}
+
+static void anfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	u32 pktcount, pktsize;
+	unsigned int buf_wr_cnt = 0;
+	u32 *bufptr = (u32 *)buf;
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+	struct anfc *nfc = to_anfc(chip->controller);
+	dma_addr_t paddr;
+
+	if (nfc->iswriteoob) {
+		pktsize = len;
+		pktcount = 1;
+	} else {
+		pktsize = achip->pktsize;
+		pktcount = mtd->writesize / pktsize;
+	}
+
+	anfc_setpktszcnt(nfc, pktsize, pktcount);
+
+	if (nfc->dma) {
+		paddr = dma_map_single(nfc->dev, (void *)buf, len,
+				       DMA_TO_DEVICE);
+		if (dma_mapping_error(nfc->dev, paddr)) {
+			dev_err(nfc->dev, "Write buffer mapping error");
+			return;
+		}
+		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
+		anfc_enable_intrs(nfc, XFER_COMPLETE);
+		writel(PROG_PGPROG, nfc->base + PROG_OFST);
+		anfc_wait_for_event(nfc);
+		dma_unmap_single(nfc->dev, paddr, len, DMA_TO_DEVICE);
+		return;
+	}
+
+	anfc_enable_intrs(nfc, WRITE_READY);
+	writel(PROG_PGPROG, nfc->base + PROG_OFST);
+
+	while (buf_wr_cnt < pktcount) {
+		anfc_wait_for_event(nfc);
+		buf_wr_cnt++;
+		if (buf_wr_cnt == pktcount)
+			anfc_enable_intrs(nfc, XFER_COMPLETE);
+
+		writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
+		bufptr += (pktsize / 4);
+
+		if (buf_wr_cnt < pktcount)
+			anfc_enable_intrs(nfc, WRITE_READY);
+	}
+
+	anfc_wait_for_event(nfc);
+}
+
+static int anfc_read_page_hwecc(struct mtd_info *mtd,
+				struct nand_chip *chip, uint8_t *buf,
+				int oob_required, int page)
+{
+	u32 val;
+	struct anfc *nfc = to_anfc(chip->controller);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+
+	anfc_set_eccsparecmd(nfc, achip, NAND_CMD_RNDOUT, NAND_CMD_RNDOUTSTART);
+
+	val = readl(nfc->base + CMD_OFST);
+	val = val | ECC_ENABLE;
+	writel(val, nfc->base + CMD_OFST);
+
+	chip->read_buf(mtd, buf, mtd->writesize);
+
+	val = readl(nfc->base + ECC_ERR_CNT_OFST);
+	if (achip->bch) {
+		mtd->ecc_stats.corrected += val & PAGE_ERR_CNT_MASK;
+	} else {
+		val = readl(nfc->base + ECC_ERR_CNT_1BIT_OFST);
+		mtd->ecc_stats.corrected += val;
+		val = readl(nfc->base + ECC_ERR_CNT_2BIT_OFST);
+		mtd->ecc_stats.failed += val;
+		/* Clear ecc error count register 1Bit, 2Bit */
+		writel(0x0, nfc->base + ECC_ERR_CNT_1BIT_OFST);
+		writel(0x0, nfc->base + ECC_ERR_CNT_2BIT_OFST);
+	}
+	nfc->err = false;
+
+	if (oob_required)
+		chip->ecc.read_oob(mtd, chip, page);
+
+	return 0;
+}
+
+static int anfc_write_page_hwecc(struct mtd_info *mtd,
+				 struct nand_chip *chip, const uint8_t *buf,
+				 int oob_required, int page)
+{
+	u32 val;
+	int ret;
+	struct anfc *nfc = to_anfc(chip->controller);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+	uint8_t *ecc_calc = chip->buffers->ecccalc;
+
+	anfc_set_eccsparecmd(nfc, achip, NAND_CMD_RNDIN, 0);
+
+	val = readl(nfc->base + CMD_OFST);
+	val = val | ECC_ENABLE;
+	writel(val, nfc->base + CMD_OFST);
+
+	chip->write_buf(mtd, buf, mtd->writesize);
+
+	if (oob_required) {
+		chip->waitfunc(mtd, chip);
+		chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
+		chip->read_buf(mtd, ecc_calc, mtd->oobsize);
+		ret = mtd_ooblayout_set_eccbytes(mtd, ecc_calc, chip->oob_poi,
+						 0, chip->ecc.total);
+		if (ret)
+			return ret;
+		chip->ecc.write_oob(mtd, chip, page);
+	}
+
+	return 0;
+}
+
+static u8 anfc_read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct anfc *nfc = to_anfc(chip->controller);
+
+	return nfc->buf[nfc->bufshift++];
+}
+
+static void anfc_writefifo(struct anfc *nfc, u32 prog, u32 size, u8 *buf)
+{
+	u32 *bufptr = (u32 *)buf;
+
+	anfc_enable_intrs(nfc, WRITE_READY);
+
+	writel(prog, nfc->base + PROG_OFST);
+	anfc_wait_for_event(nfc);
+
+	anfc_enable_intrs(nfc, XFER_COMPLETE);
+	writesl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
+	anfc_wait_for_event(nfc);
+}
+
+static void anfc_readfifo(struct anfc *nfc, u32 prog, u32 size)
+{
+	u32 *bufptr = (u32 *)nfc->buf;
+
+	anfc_enable_intrs(nfc, READ_READY);
+
+	writel(prog, nfc->base + PROG_OFST);
+	anfc_wait_for_event(nfc);
+
+	anfc_enable_intrs(nfc, XFER_COMPLETE);
+	readsl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
+	anfc_wait_for_event(nfc);
+}
+
+static int anfc_ecc_init(struct mtd_info *mtd,
+			 struct nand_ecc_ctrl *ecc)
+{
+	u32 ecc_addr;
+	unsigned int bchmode, steps;
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+
+	ecc->mode = NAND_ECC_HW;
+	ecc->read_page = anfc_read_page_hwecc;
+	ecc->write_page = anfc_write_page_hwecc;
+	ecc->write_oob = anfc_write_oob;
+	mtd_set_ooblayout(mtd, &anfc_ooblayout_ops);
+
+	steps = mtd->writesize / chip->ecc_step_ds;
+
+	switch (chip->ecc_strength_ds) {
+	case 12:
+		bchmode = 0x1;
+		break;
+	case 8:
+		bchmode = 0x2;
+		break;
+	case 4:
+		bchmode = 0x3;
+		break;
+	case 24:
+		bchmode = 0x4;
+		break;
+	default:
+		bchmode = 0x0;
+	}
+
+	if (!bchmode)
+		ecc->total = 3 * steps;
+	else
+		ecc->total =
+		     DIV_ROUND_UP(fls(8 * chip->ecc_step_ds) *
+			 chip->ecc_strength_ds * steps, 8);
+
+	ecc->strength = chip->ecc_strength_ds;
+	ecc->size = chip->ecc_step_ds;
+	ecc->bytes = ecc->total / steps;
+	ecc->steps = steps;
+	achip->bchmode = bchmode;
+	achip->bch = achip->bchmode;
+	ecc_addr = mtd->writesize + (mtd->oobsize - ecc->total);
+
+	achip->eccval = ecc_addr | (ecc->total << ECC_SIZE_SHIFT) |
+			(achip->bch << BCH_EN_SHIFT);
+
+	if (chip->ecc_step_ds >= 1024)
+		achip->pktsize = 1024;
+	else
+		achip->pktsize = 512;
+
+	return 0;
+}
+
+static void anfc_cmd_function(struct mtd_info *mtd,
+			      unsigned int cmd, int column, int page_addr)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+	struct anfc *nfc = to_anfc(chip->controller);
+	bool wait = false, read = false;
+	u32 addrcycles, prog;
+	u32 *bufptr = (u32 *)nfc->buf;
+
+	nfc->bufshift = 0;
+	nfc->curr_cmd = cmd;
+
+	if (page_addr == -1)
+		page_addr = 0;
+	if (column == -1)
+		column = 0;
+
+	switch (cmd) {
+	case NAND_CMD_RESET:
+		anfc_prepare_cmd(nfc, cmd, 0, 0, 0, 0);
+		prog = PROG_RST;
+		wait = true;
+		break;
+	case NAND_CMD_SEQIN:
+		addrcycles = achip->raddr_cycles + achip->caddr_cycles;
+		anfc_prepare_cmd(nfc, cmd, NAND_CMD_PAGEPROG, 1,
+				 mtd->writesize, addrcycles);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		break;
+	case NAND_CMD_READOOB:
+		column += mtd->writesize;
+	case NAND_CMD_READ0:
+	case NAND_CMD_READ1:
+		addrcycles = achip->raddr_cycles + achip->caddr_cycles;
+		anfc_prepare_cmd(nfc, NAND_CMD_READ0, NAND_CMD_READSTART, 1,
+				 mtd->writesize, addrcycles);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		break;
+	case NAND_CMD_RNDOUT:
+		anfc_prepare_cmd(nfc, cmd, NAND_CMD_RNDOUTSTART, 1,
+				 mtd->writesize, 2);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		break;
+	case NAND_CMD_PARAM:
+		anfc_prepare_cmd(nfc, cmd, 0, 0, 0, 1);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		anfc_setpktszcnt(nfc, sizeof(struct nand_onfi_params), 1);
+		anfc_readfifo(nfc, PROG_RDPARAM,
+				sizeof(struct nand_onfi_params));
+		break;
+	case NAND_CMD_READID:
+		anfc_prepare_cmd(nfc, cmd, 0, 0, 0, 1);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		anfc_setpktszcnt(nfc, ONFI_ID_LEN, 1);
+		anfc_readfifo(nfc, PROG_RDID, ONFI_ID_LEN);
+		break;
+	case NAND_CMD_ERASE1:
+		addrcycles = achip->raddr_cycles;
+		prog = PROG_ERASE;
+		anfc_prepare_cmd(nfc, cmd, NAND_CMD_ERASE2, 0, 0, addrcycles);
+		column = page_addr & 0xffff;
+		page_addr = (page_addr >> PG_ADDR_SHIFT) & 0xffff;
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		wait = true;
+		break;
+	case NAND_CMD_STATUS:
+		anfc_prepare_cmd(nfc, cmd, 0, 0, 0, 0);
+		anfc_setpktszcnt(nfc, achip->spktsize/4, 1);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		prog = PROG_STATUS;
+		wait = read = true;
+		break;
+	case NAND_CMD_GET_FEATURES:
+		anfc_prepare_cmd(nfc, cmd, 0, 0, 0, 1);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		anfc_setpktszcnt(nfc, achip->spktsize, 1);
+		anfc_readfifo(nfc, PROG_GET_FEATURE, 4);
+		break;
+	case NAND_CMD_SET_FEATURES:
+		anfc_prepare_cmd(nfc, cmd, 0, 0, 0, 1);
+		anfc_setpagecoladdr(nfc, page_addr, column);
+		anfc_setpktszcnt(nfc, achip->spktsize, 1);
+		break;
+	default:
+		return;
+	}
+
+	if (wait) {
+		anfc_enable_intrs(nfc, XFER_COMPLETE);
+		writel(prog, nfc->base + PROG_OFST);
+		anfc_wait_for_event(nfc);
+	}
+
+	if (read)
+		bufptr[0] = readl(nfc->base + FLASH_STS_OFST);
+}
+
+static void anfc_select_chip(struct mtd_info *mtd, int num)
+{
+	u32 val;
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+	struct anfc *nfc = to_anfc(chip->controller);
+
+	if (num == -1)
+		return;
+
+	val = readl(nfc->base + MEM_ADDR2_OFST);
+	val = (val & ~(CS_MASK)) | (achip->csnum << CS_SHIFT);
+	val = (val & ~(BCH_MODE_MASK)) | (achip->bchmode << BCH_MODE_SHIFT);
+	writel(val, nfc->base + MEM_ADDR2_OFST);
+	nfc->csnum = achip->csnum;
+	writel(achip->eccval, nfc->base + ECC_OFST);
+	writel(achip->inftimeval, nfc->base + DATA_INTERFACE_REG);
+}
+
+static irqreturn_t anfc_irq_handler(int irq, void *ptr)
+{
+	struct anfc *nfc = ptr;
+	u32 regval = 0, status;
+
+	status = readl(nfc->base + INTR_STS_OFST);
+	if (status & XFER_COMPLETE) {
+		complete(&nfc->evnt);
+		regval |= XFER_COMPLETE;
+	}
+
+	if (status & READ_READY) {
+		complete(&nfc->evnt);
+		regval |= READ_READY;
+	}
+
+	if (status & WRITE_READY) {
+		complete(&nfc->evnt);
+		regval |= WRITE_READY;
+	}
+
+	if (status & MBIT_ERROR) {
+		nfc->err = true;
+		complete(&nfc->evnt);
+		regval |= MBIT_ERROR;
+	}
+
+	if (regval) {
+		writel(regval, nfc->base + INTR_STS_OFST);
+		writel(0, nfc->base + INTR_STS_EN_OFST);
+		writel(0, nfc->base + INTR_SIG_EN_OFST);
+
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static int anfc_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
+				int addr, uint8_t *subfeature_param)
+{
+	struct anfc *nfc = to_anfc(chip->controller);
+	struct anfc_nand_chip *achip = to_anfc_nand(chip);
+	int status;
+
+	if (!chip->onfi_version || !(le16_to_cpu(chip->onfi_params.opt_cmd)
+		& ONFI_OPT_CMD_SET_GET_FEATURES))
+		return -EINVAL;
+
+	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
+	anfc_writefifo(nfc, PROG_SET_FEATURE, achip->spktsize,
+			subfeature_param);
+
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
+	return 0;
+}
+
+static int anfc_init_timing_mode(struct anfc *nfc,
+				 struct anfc_nand_chip *achip)
+{
+	int mode, err;
+	unsigned int feature[2];
+	u32 inftimeval;
+	struct nand_chip *chip = &achip->chip;
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	memset(feature, 0, NVDDR_MODE_PACKET_SIZE);
+	/* Get nvddr timing modes */
+	mode = onfi_get_sync_timing_mode(chip) & 0xff;
+	if (!mode) {
+		mode = fls(onfi_get_async_timing_mode(chip)) - 1;
+		inftimeval = mode;
+	} else {
+		mode = fls(mode) - 1;
+		inftimeval = NVDDR_MODE | (mode << NVDDR_TIMING_MODE_SHIFT);
+		mode |= ONFI_DATA_INTERFACE_NVDDR;
+	}
+
+	feature[0] = mode;
+	chip->select_chip(mtd, achip->csnum);
+	err = chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_TIMING_MODE,
+				      (uint8_t *)feature);
+	chip->select_chip(mtd, -1);
+	if (err)
+		return err;
+
+	achip->inftimeval = inftimeval;
+
+	if (mode & ONFI_DATA_INTERFACE_NVDDR)
+		achip->spktsize = NVDDR_MODE_PACKET_SIZE;
+
+	return 0;
+}
+
+static int anfc_nand_chip_init(struct anfc *nfc,
+				struct anfc_nand_chip *anand_chip,
+				struct device_node *np)
+{
+	struct nand_chip *chip = &anand_chip->chip;
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int ret;
+
+	ret = of_property_read_u32(np, "reg", &anand_chip->csnum);
+	if (ret) {
+		dev_err(nfc->dev, "can't get chip-select\n");
+		return -ENXIO;
+	}
+
+	mtd->name = devm_kasprintf(nfc->dev, GFP_KERNEL, "arasan_nand.%d",
+				   anand_chip->csnum);
+	mtd->dev.parent = nfc->dev;
+
+	chip->cmdfunc = anfc_cmd_function;
+	chip->chip_delay = 30;
+	chip->controller = &nfc->controller;
+	chip->read_buf = anfc_read_buf;
+	chip->write_buf = anfc_write_buf;
+	chip->read_byte = anfc_read_byte;
+	chip->options = NAND_BUSWIDTH_AUTO | NAND_NO_SUBPAGE_WRITE;
+	chip->bbt_options = NAND_BBT_USE_FLASH;
+	chip->select_chip = anfc_select_chip;
+	chip->onfi_set_features = anfc_onfi_set_features;
+	nand_set_flash_node(chip, np);
+
+	anand_chip->spktsize = SDR_MODE_PACKET_SIZE;
+	ret = nand_scan_ident(mtd, 1, NULL);
+	if (ret) {
+		dev_err(nfc->dev, "nand_scan_ident for NAND failed\n");
+		return ret;
+	}
+	if (chip->onfi_version) {
+		anand_chip->raddr_cycles = chip->onfi_params.addr_cycles & 0xf;
+		anand_chip->caddr_cycles =
+				(chip->onfi_params.addr_cycles >> 4) & 0xf;
+	} else {
+		/* For non-ONFI devices, configuring the address cyles as 5 */
+		anand_chip->raddr_cycles = 3;
+		anand_chip->caddr_cycles = 2;
+	}
+
+	ret = anfc_init_timing_mode(nfc, anand_chip);
+	if (ret) {
+		dev_err(nfc->dev, "timing mode init failed\n");
+		return ret;
+	}
+
+	ret = anfc_ecc_init(mtd, &chip->ecc);
+	if (ret)
+		return ret;
+
+	ret = nand_scan_tail(mtd);
+	if (ret) {
+		dev_err(nfc->dev, "nand_scan_tail for NAND failed\n");
+		return ret;
+	}
+
+	return mtd_device_register(mtd, NULL, 0);
+}
+
+static int anfc_probe(struct platform_device *pdev)
+{
+	struct anfc *nfc;
+	struct anfc_nand_chip *anand_chip;
+	struct device_node *np = pdev->dev.of_node, *child;
+	struct resource *res;
+	int err;
+
+	nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
+	if (!nfc)
+		return -ENOMEM;
+
+	init_waitqueue_head(&nfc->controller.wq);
+	INIT_LIST_HEAD(&nfc->chips);
+	init_completion(&nfc->evnt);
+	nfc->dev = &pdev->dev;
+	platform_set_drvdata(pdev, nfc);
+	nfc->csnum = -1;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	nfc->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(nfc->base))
+		return PTR_ERR(nfc->base);
+	nfc->dma = of_property_read_bool(pdev->dev.of_node,
+					 "arasan,has-mdma");
+	nfc->irq = platform_get_irq(pdev, 0);
+	if (nfc->irq < 0) {
+		dev_err(&pdev->dev, "platform_get_irq failed\n");
+		return -ENXIO;
+	}
+	err = devm_request_irq(&pdev->dev, nfc->irq, anfc_irq_handler,
+			       0, "arasannfc", nfc);
+	if (err)
+		return err;
+	nfc->clk_sys = devm_clk_get(&pdev->dev, "clk_sys");
+	if (IS_ERR(nfc->clk_sys)) {
+		dev_err(&pdev->dev, "sys clock not found.\n");
+		return PTR_ERR(nfc->clk_sys);
+	}
+
+	nfc->clk_flash = devm_clk_get(&pdev->dev, "clk_flash");
+	if (IS_ERR(nfc->clk_flash)) {
+		dev_err(&pdev->dev, "flash clock not found.\n");
+		return PTR_ERR(nfc->clk_flash);
+	}
+
+	err = clk_prepare_enable(nfc->clk_sys);
+	if (err) {
+		dev_err(&pdev->dev, "Unable to enable sys clock.\n");
+		return err;
+	}
+
+	err = clk_prepare_enable(nfc->clk_flash);
+	if (err) {
+		dev_err(&pdev->dev, "Unable to enable flash clock.\n");
+		goto clk_dis_sys;
+	}
+
+	for_each_available_child_of_node(np, child) {
+		anand_chip = devm_kzalloc(&pdev->dev, sizeof(*anand_chip),
+					  GFP_KERNEL);
+		if (!anand_chip) {
+			of_node_put(child);
+			err = -ENOMEM;
+			goto nandchip_clean_up;
+		}
+
+		err = anfc_nand_chip_init(nfc, anand_chip, child);
+		if (err) {
+			devm_kfree(&pdev->dev, anand_chip);
+			continue;
+		}
+
+		list_add_tail(&anand_chip->node, &nfc->chips);
+	}
+
+	return 0;
+
+nandchip_clean_up:
+	list_for_each_entry(anand_chip, &nfc->chips, node)
+		nand_release(nand_to_mtd(&anand_chip->chip));
+	clk_disable_unprepare(nfc->clk_flash);
+clk_dis_sys:
+	clk_disable_unprepare(nfc->clk_sys);
+
+	return err;
+}
+
+static int anfc_remove(struct platform_device *pdev)
+{
+	struct anfc *nfc = platform_get_drvdata(pdev);
+	struct anfc_nand_chip *anand_chip;
+
+	list_for_each_entry(anand_chip, &nfc->chips, node)
+		nand_release(nand_to_mtd(&anand_chip->chip));
+
+	clk_disable_unprepare(nfc->clk_sys);
+	clk_disable_unprepare(nfc->clk_flash);
+
+	return 0;
+}
+
+static const struct of_device_id anfc_ids[] = {
+	{ .compatible = "arasan,nfc-v3p10" },
+	{  }
+};
+MODULE_DEVICE_TABLE(of, anfc_ids);
+
+static struct platform_driver anfc_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = anfc_ids,
+	},
+	.probe = anfc_probe,
+	.remove = anfc_remove,
+};
+module_platform_driver(anfc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Xilinx, Inc");
+MODULE_DESCRIPTION("Arasan NAND Flash Controller Driver");