Hi Lucas, Thanks for picking that up! I did some short benchmarks on Colibri T20 V1.2, L4T. Write/read speeds I measured on the YAFFS2 based file system: # dd if=/dev/zero of=test bs=50M count=1 conv=fdatasync 1+0 records in 1+0 records out 52428800 bytes (52 MB) copied, 9.88293 s, 5.3 MB/s echo 3 > /proc/sys/vm/drop_caches # dd if=test of=/dev/zero bs=50M count=1 1+0 records in 1+0 records out 52428800 bytes (52 MB) copied, 5.97056 s, 8.8 MB/s So your values look quite realistic then! Some comments below... On 2015-01-04 21:39, Lucas Stach wrote: > Add support for the NAND flash controller found on NVIDIA > Tegra 2/3 SoCs. This is a largely reworked version of the driver > started by Thierry. > > Signed-off-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx> > Signed-off-by: Lucas Stach <dev@xxxxxxxxxx> > --- > I've tested this driver with the in-kernel mtd-tests and some > realworld workloads on a Colibri T20 module. > --- > .../bindings/mtd/nvidia,tegra20-nand.txt | 30 + > MAINTAINERS | 6 + > drivers/mtd/nand/Kconfig | 6 + > drivers/mtd/nand/Makefile | 1 + > drivers/mtd/nand/tegra_nand.c | 794 +++++++++++++++++++++ > 5 files changed, 837 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt > create mode 100644 drivers/mtd/nand/tegra_nand.c > > diff --git > a/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt > b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt > new file mode 100644 > index 0000000..088223c > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt > @@ -0,0 +1,30 @@ > +NVIDIA Tegra NAND Flash controller > + > +Required properties: > +- compatible: Must be one of: > + - "nvidia,tegra20-nand" > + - "nvidia,tegra30-nand" > +- reg: MMIO address range > +- interrupts: interrupt output of the NFC controller > +- clocks: Must contain an entry for each entry in clock-names. > + See ../clocks/clock-bindings.txt for details. > +- clock-names: Must include the following entries: > + - nand > +- resets: Must contain an entry for each entry in reset-names. > + See ../reset/reset.txt for details. > +- reset-names: Must include the following entries: > + - nand > + > +Optional properties: > +- nvidia,wp-gpios: GPIO used to disable write protection of the flash > + > + Example: > + nand@70008000 { > + compatible = "nvidia,tegra20-nand"; > + reg = <0x70008000 0x100>; > + interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&tegra_car TEGRA20_CLK_NDFLASH>; > + clock-names = "nand"; > + resets = <&tegra_car 13>; > + reset-names = "nand"; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index ddb9ac8..972e31d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9459,6 +9459,12 @@ M: Laxman Dewangan <ldewangan@xxxxxxxxxx> > S: Supported > F: drivers/input/keyboard/tegra-kbc.c > > +TEGRA NAND DRIVER > +M: Lucas Stach <dev@xxxxxxxxxx> > +S: Maintained > +F: Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt > +F: drivers/mtd/nand/tegra_nand.c > + > TEGRA PWM DRIVER > M: Thierry Reding <thierry.reding@xxxxxxxxx> > S: Supported > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index 7d0150d..1eafd4e 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -524,4 +524,10 @@ config MTD_NAND_SUNXI > help > Enables support for NAND Flash chips on Allwinner SoCs. > > +config MTD_NAND_TEGRA > + tristate "Support for NAND on NVIDIA Tegra" > + depends on ARCH_TEGRA || COMPILE_TEST > + help > + Enables support for NAND flash on NVIDIA Tegra SoC based boards. > + > endif # MTD_NAND > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index bd38f21..58399ce 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -51,5 +51,6 @@ obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ > obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o > obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ > obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o > +obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o > > nand-objs := nand_base.o nand_bbt.o nand_timings.o > diff --git a/drivers/mtd/nand/tegra_nand.c b/drivers/mtd/nand/tegra_nand.c > new file mode 100644 > index 0000000..b919a6e > --- /dev/null > +++ b/drivers/mtd/nand/tegra_nand.c > @@ -0,0 +1,794 @@ > +/* > + * Copyright (C) 2014-2015 Lucas Stach <dev@xxxxxxxxxx> > + * Copyright (C) 2012 Avionic Design GmbH > + * > + * 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. > + */ > + > +#include <linux/clk.h> > +#include <linux/completion.h> > +#include <linux/delay.h> > +#include <linux/dma-mapping.h> > +#include <linux/err.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/mtd/nand.h> > +#include <linux/mtd/partitions.h> > +#include <linux/of_gpio.h> > +#include <linux/of_mtd.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/reset.h> > + > +#define CMD 0x00 > +#define CMD_GO (1 << 31) > +#define CMD_CLE (1 << 30) > +#define CMD_ALE (1 << 29) > +#define CMD_PIO (1 << 28) > +#define CMD_TX (1 << 27) > +#define CMD_RX (1 << 26) > +#define CMD_SEC_CMD (1 << 25) > +#define CMD_AFT_DAT (1 << 24) > +#define CMD_TRANS_SIZE(x) (((x) & 0xf) << 20) > +#define CMD_A_VALID (1 << 19) > +#define CMD_B_VALID (1 << 18) > +#define CMD_RD_STATUS_CHK (1 << 17) > +#define CMD_RBSY_CHK (1 << 16) > +#define CMD_CE(x) (1 << (8 + ((x) & 0x7))) > +#define CMD_CLE_SIZE(x) (((x) & 0x3) << 4) > +#define CMD_ALE_SIZE(x) (((x) & 0xf) << 0) > + > +#define STATUS 0x04 > + > +#define ISR 0x08 > +#define ISR_UND (1 << 7) > +#define ISR_OVR (1 << 6) > +#define ISR_CMD_DONE (1 << 5) > +#define ISR_ECC_ERR (1 << 4) > + > +#define IER 0x0c > +#define IER_ERR_TRIG_VAL(x) (((x) & 0xf) << 16) > +#define IER_UND (1 << 7) > +#define IER_OVR (1 << 6) > +#define IER_CMD_DONE (1 << 5) > +#define IER_ECC_ERR (1 << 4) > +#define IER_GIE (1 << 0) > + > +#define CFG 0x10 > +#define CFG_HW_ECC (1 << 31) > +#define CFG_ECC_SEL (1 << 30) > +#define CFG_ERR_COR (1 << 29) > +#define CFG_PIPE_EN (1 << 28) > +#define CFG_TVAL_4 (0 << 24) > +#define CFG_TVAL_6 (1 << 24) > +#define CFG_TVAL_8 (2 << 24) > +#define CFG_SKIP_SPARE (1 << 23) > +#define CFG_BUS_WIDTH_8 (0 << 21) > +#define CFG_BUS_WIDTH_16 (1 << 21) > +#define CFG_COM_BSY (1 << 20) > +#define CFG_PS_256 (0 << 16) > +#define CFG_PS_512 (1 << 16) > +#define CFG_PS_1024 (2 << 16) > +#define CFG_PS_2048 (3 << 16) > +#define CFG_PS_4096 (4 << 16) > +#define CFG_SKIP_SPARE_SIZE_4 (0 << 14) > +#define CFG_SKIP_SPARE_SIZE_8 (1 << 14) > +#define CFG_SKIP_SPARE_SIZE_12 (2 << 14) > +#define CFG_SKIP_SPARE_SIZE_16 (3 << 14) > +#define CFG_TAG_BYTE_SIZE(x) ((x) & 0xff) > + > +#define TIMING_1 0x14 > +#define TIMING_TRP_RESP(x) (((x) & 0xf) << 28) > +#define TIMING_TWB(x) (((x) & 0xf) << 24) > +#define TIMING_TCR_TAR_TRR(x) (((x) & 0xf) << 20) > +#define TIMING_TWHR(x) (((x) & 0xf) << 16) > +#define TIMING_TCS(x) (((x) & 0xc) << 14) In Tegra 2 TRM v02p, this is 15:14. You shift the masked value by 14, hence the mask should be 0x3... > +#define TIMING_TWH(x) (((x) & 0x3) << 12) > +#define TIMING_TWP(x) (((x) & 0xf) << 8) > +#define TIMING_TRH(x) (((x) & 0xf) << 4) > +#define TIMING_TRP(x) (((x) & 0xf) << 0) > + > +#define RESP 0x18 > + > +#define TIMING_2 0x1c > +#define TIMING_TADL(x) ((x) & 0xf) > + > +#define CMD_1 0x20 > +#define CMD_2 0x24 > +#define ADDR_1 0x28 > +#define ADDR_2 0x2c > + > +#define DMA_CTRL 0x30 > +#define DMA_CTRL_GO (1 << 31) > +#define DMA_CTRL_IN (0 << 30) > +#define DMA_CTRL_OUT (1 << 30) > +#define DMA_CTRL_PERF_EN (1 << 29) > +#define DMA_CTRL_IE_DONE (1 << 28) > +#define DMA_CTRL_REUSE (1 << 27) > +#define DMA_CTRL_BURST_1 (2 << 24) > +#define DMA_CTRL_BURST_4 (3 << 24) > +#define DMA_CTRL_BURST_8 (4 << 24) > +#define DMA_CTRL_BURST_16 (5 << 24) > +#define DMA_CTRL_IS_DONE (1 << 20) > +#define DMA_CTRL_EN_A (1 << 2) > +#define DMA_CTRL_EN_B (1 << 1) > + > +#define DMA_CFG_A 0x34 > +#define DMA_CFG_B 0x38 > + > +#define FIFO_CTRL 0x3c > +#define FIFO_CTRL_CLR_ALL (1 << 3) > + > +#define DATA_PTR 0x40 > +#define TAG_PTR 0x44 > +#define ECC_PTR 0x48 > + > +#define HWSTATUS_CMD 0x50 > +#define HWSTATUS_MASK 0x54 > +#define HWSTATUS_RDSTATUS_MASK(x) (((x) & 0xff) << 24) > +#define HWSTATUS_RDSTATUS_VALUE(x) (((x) & 0xff) << 16) > +#define HWSTATUS_RBSY_MASK(x) (((x) & 0xff) << 8) > +#define HWSTATUS_RBSY_VALUE(x) (((x) & 0xff) << 0) > + > +#define DEC_RESULT 0xd0 > +#define DEC_RESULT_CORRFAIL (1 << 8) > + > +#define DEC_STATUS_BUF 0xd4 > +#define DEC_STATUS_BUF_FAIL_SEC_FLAG(x) ((x) & (0xff << 24)) > +#define DEC_STATUS_BUF_CORR_SEC_FLAG(x) ((x) & (0xff << 16)) > +#define DEC_STATUS_BUF_MAX_CORR_CNT(x) (((x) & 0xf00) >> 8) > + > +struct tegra_nand { > + void __iomem *regs; > + int irq; > + struct clk *clk; > + struct reset_control *rst; > + int wp_gpio; > + int buswidth; > + > + struct nand_chip chip; > + struct mtd_info mtd; > + struct device *dev; > + > + struct completion command_complete; > + struct completion dma_complete; > + > + dma_addr_t data_dma; > + void *data_buf; > + dma_addr_t oob_dma; > + void *oob_buf; > + > + int cur_chip; > +}; > + > +static inline struct tegra_nand *to_tegra_nand(struct mtd_info *mtd) > +{ > + return container_of(mtd, struct tegra_nand, mtd); > +} > + > +static struct nand_ecclayout tegra_nand_oob_16 = { > + .eccbytes = 4, > + .eccpos = { 3, 4, 5, 6 }, > + .oobfree = { > + { .offset = 8, . length = 8 } > + } > +}; > + > +static struct nand_ecclayout tegra_nand_oob_64 = { > + .eccbytes = 36, > + .eccpos = { > + 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, > + 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, > + 35, 36, 37, 38, 39 This list contains 37 bytes, is this intended? > + }, > + .oobfree = { > + { .offset = 40, .length = 20 } > + } > +}; > + > +static struct nand_ecclayout tegra_nand_oob_128 = { > + .eccbytes = 72, > + .eccpos = { > + 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, > + 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, > + 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, > + 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, > + 67, 68, 69, 70, 71, 72, 73, 74, 75 > + }, > + .oobfree = { > + { .offset = 76, .length = 52 } > + } > +}; > + > +static struct nand_ecclayout tegra_nand_oob_224 = { > + .eccbytes = 144, > + .eccpos = { > + 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, > + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, > + 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, > + 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, > + 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, > + 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, > + 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, > + 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, > + 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, > + 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, > + 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, > + 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, > + 147 > + }, > + .oobfree = { > + { .offset = 148, .length = 76 } > + } > +}; > + > +static irqreturn_t tegra_nand_irq(int irq, void *data) > +{ > + struct tegra_nand *nand = data; > + irqreturn_t ret = IRQ_HANDLED; > + u32 isr, dma; > + > + isr = readl(nand->regs + ISR); > + dma = readl(nand->regs + DMA_CTRL); > + > + if (!isr && !(dma & DMA_CTRL_IS_DONE)) { > + ret = IRQ_NONE; > + goto out; The out label doesn't do anything more than just return. Why not just return IRQ_NONE here and return IRQ_HANDLED at the end, saves the local variable and helps readability... Why is this needed anyway, is the IRQ shared with other peripherals? In the L4T driver, there is a warning message about spurious interrupts, does this works around this interrupts? > + } > + > + if (isr & ISR_CMD_DONE) > + complete(&nand->command_complete); > + > + if (isr & ISR_UND) > + dev_dbg(nand->dev, " FIFO underrun\n"); Two spaces? > + > + if (isr & ISR_OVR) > + dev_dbg(nand->dev, " FIFO overrun\n"); > + > + /* handle DMA interrupts */ > + if (dma & DMA_CTRL_IS_DONE) { > + writel(dma, nand->regs + DMA_CTRL); > + complete(&nand->dma_complete); > + } > + > + /* clear interrupts */ > + writel(isr, nand->regs + ISR); > + > +out: > + return ret; > +} > + > +static void tegra_nand_command(struct mtd_info *mtd, unsigned int command, > + int column, int page_addr) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + u32 value; > + > + switch (command) { > + case NAND_CMD_READOOB: > + column += mtd->writesize; > + /* fall-through */ > + > + case NAND_CMD_READ0: > + writel(NAND_CMD_READ0, nand->regs + CMD_1); > + writel(NAND_CMD_READSTART, nand->regs + CMD_2); > + > + value = (page_addr << 16) | (column & 0xffff); > + writel(value, nand->regs + ADDR_1); > + > + value = page_addr >> 16; > + writel(value, nand->regs + ADDR_2); > + > + value = CMD_CLE | CMD_ALE | CMD_ALE_SIZE(4) | CMD_SEC_CMD | > + CMD_RBSY_CHK | CMD_CE(nand->cur_chip) | CMD_GO; > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_SEQIN: > + writel(NAND_CMD_SEQIN, nand->regs + CMD_1); > + > + value = (page_addr << 16) | (column & 0xffff); > + writel(value, nand->regs + ADDR_1); > + > + value = page_addr >> 16; > + writel(value, nand->regs + ADDR_2); > + > + value = CMD_CLE | CMD_ALE | CMD_ALE_SIZE(4) | > + CMD_CE(nand->cur_chip) | CMD_GO; > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_PAGEPROG: > + writel(NAND_CMD_PAGEPROG, nand->regs + CMD_1); > + > + value = CMD_CLE | CMD_CE(nand->cur_chip) | CMD_GO; > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_READID: > + writel(NAND_CMD_READID, nand->regs + CMD_1); > + writel(column & 0xff, nand->regs + ADDR_1); > + > + value = CMD_GO | CMD_CLE | CMD_ALE | CMD_CE(nand->cur_chip); > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_ERASE1: > + writel(NAND_CMD_ERASE1, nand->regs + CMD_1); > + writel(NAND_CMD_ERASE2, nand->regs + CMD_2); > + writel(page_addr, nand->regs + ADDR_1); > + > + value = CMD_GO | CMD_CLE | CMD_ALE | CMD_ALE_SIZE(2) | > + CMD_SEC_CMD | CMD_RBSY_CHK | CMD_CE(nand->cur_chip); > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_ERASE2: > + return; > + > + case NAND_CMD_STATUS: > + writel(NAND_CMD_STATUS, nand->regs + CMD_1); > + > + value = CMD_GO | CMD_CLE | CMD_CE(nand->cur_chip); > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_PARAM: > + writel(NAND_CMD_PARAM, nand->regs + CMD_1); > + writel(column & 0xff, nand->regs + ADDR_1); > + value = CMD_GO | CMD_CLE | CMD_ALE | CMD_CE(nand->cur_chip); > + writel(value, nand->regs + CMD); > + break; > + > + case NAND_CMD_RESET: > + writel(NAND_CMD_RESET, nand->regs + CMD_1); > + > + value = CMD_GO | CMD_CLE | CMD_CE(nand->cur_chip); > + writel(value, nand->regs + CMD); > + break; > + > + default: > + dev_warn(nand->dev, "unsupported command: %x\n", command); > + return; > + } > + > + wait_for_completion(&nand->command_complete); > +} > + > +static void tegra_nand_select_chip(struct mtd_info *mtd, int chip) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + > + nand->cur_chip = chip; > +} > + > +static uint8_t tegra_nand_read_byte(struct mtd_info *mtd) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + u32 value; > + > + value = CMD_TRANS_SIZE(0) | CMD_CE(nand->cur_chip) | > + CMD_PIO | CMD_RX | CMD_A_VALID | CMD_GO; > + > + writel(value, nand->regs + CMD); > + wait_for_completion(&nand->command_complete); > + > + return readl(nand->regs + RESP) & 0xff; > +} > + > +static void tegra_nand_read_buf(struct mtd_info *mtd, uint8_t *buffer, > + int length) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + size_t i; > + > + for (i = 0; i < length; i += 4) { > + u32 value; > + size_t n = min_t(size_t, length - i, 4); > + > + value = CMD_GO | CMD_PIO | CMD_RX | CMD_A_VALID | > + CMD_CE(nand->cur_chip) | CMD_TRANS_SIZE(n - 1); > + > + writel(value, nand->regs + CMD); > + wait_for_completion(&nand->command_complete); > + > + value = readl(nand->regs + RESP); > + memcpy(buffer + i, &value, n); > + } > +} > + > +static void tegra_nand_write_buf(struct mtd_info *mtd, const uint8_t *buffer, > + int length) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + size_t i; > + > + for (i = 0; i < length; i += 4) { > + u32 value; > + size_t n = min_t(size_t, length - i, 4); > + > + memcpy(&value, buffer + i, n); > + writel(value, nand->regs + RESP); > + > + value = CMD_GO | CMD_PIO | CMD_TX | CMD_A_VALID | > + CMD_CE(nand->cur_chip) | CMD_TRANS_SIZE(n - 1); > + > + writel(value, nand->regs + CMD); > + wait_for_completion(&nand->command_complete); > + } > +} > + > +static int tegra_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip, > + uint8_t *buf, int oob_required, int page) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + u32 value; > + > + writel(mtd->writesize - 1, nand->regs + DMA_CFG_A); > + writel(nand->data_dma, nand->regs + DATA_PTR); > + > + if (oob_required) { > + writel(mtd->oobsize - 1, nand->regs + DMA_CFG_B); > + writel(nand->oob_dma, nand->regs + TAG_PTR); > + } else { > + writel(0, nand->regs + DMA_CFG_B); > + writel(0, nand->regs + TAG_PTR); > + } > + > + value = DMA_CTRL_GO | DMA_CTRL_IN | DMA_CTRL_PERF_EN | > + DMA_CTRL_REUSE | DMA_CTRL_IE_DONE | DMA_CTRL_IS_DONE | > + DMA_CTRL_BURST_8 | DMA_CTRL_EN_A; > + > + if (oob_required) > + value |= DMA_CTRL_EN_B; > + > + writel(value, nand->regs + DMA_CTRL); > + > + value = CMD_GO | CMD_RX | CMD_TRANS_SIZE(8) | > + CMD_A_VALID | CMD_CE(nand->cur_chip); > + if (oob_required) > + value |= CMD_B_VALID; > + writel(value, nand->regs + CMD); > + > + wait_for_completion(&nand->command_complete); > + wait_for_completion(&nand->dma_complete); > + > + if (oob_required) > + memcpy(chip->oob_poi, nand->oob_buf, mtd->oobsize); > + memcpy(buf, nand->data_buf, mtd->writesize); > + > + value = readl(nand->regs + DEC_RESULT); > + if (value & DEC_RESULT_CORRFAIL) { > + value = readl(nand->regs + DEC_STATUS_BUF); > + > + if (DEC_STATUS_BUF_FAIL_SEC_FLAG(value)) > + return -1; > + > + if (DEC_STATUS_BUF_CORR_SEC_FLAG(value)) > + return DEC_STATUS_BUF_MAX_CORR_CNT(value); > + } > + > + return 0; > +} > + > +static int tegra_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip, > + const uint8_t *buf, int oob_required) > +{ > + struct tegra_nand *nand = to_tegra_nand(mtd); > + unsigned long value; > + int ret = 0; > + > + memcpy(nand->data_buf, buf, mtd->writesize); > + > + writel(mtd->writesize - 1, nand->regs + DMA_CFG_A); > + writel(nand->data_dma, nand->regs + DATA_PTR); > + > + writel(0, nand->regs + DMA_CFG_B); > + writel(0, nand->regs + TAG_PTR); > + > + value = DMA_CTRL_GO | DMA_CTRL_OUT | DMA_CTRL_PERF_EN | > + DMA_CTRL_IE_DONE | DMA_CTRL_IS_DONE | > + DMA_CTRL_BURST_8 | DMA_CTRL_EN_A; > + writel(value, nand->regs + DMA_CTRL); > + > + value = CMD_GO | CMD_TX | CMD_A_VALID | CMD_TRANS_SIZE(8) | > + CMD_CE(nand->cur_chip); > + writel(value, nand->regs + CMD); > + > + wait_for_completion(&nand->command_complete); > + wait_for_completion(&nand->dma_complete); > + > + return ret; ret is never accessed in this function hence you can omit it. > +} > + > +static void tegra_nand_setup_timing(struct tegra_nand *nand, int mode) > +{ > + unsigned long rate = clk_get_rate(nand->clk) / 1000000; > + unsigned long period = 1000000 / rate; Hm, period of a clock in ns... Sounds like a common use case. I searched for a macro/helper, but did not found anything. Well then. > + const struct nand_sdr_timings *timings; > + u32 val, reg = 0; > + > + timings = onfi_async_timing_mode_to_sdr_timings(mode); > + > + val = max3(timings->tAR_min, timings->tRR_min, > + timings->tRC_min) / period; > + if (val > 2) > + val -= 2; According to my TRM this is: Generated timing = (n+3) * NAND_CLK_PERIOD ns. Shouldn't this look like if (val >= 3) val -= 3; then? I see that all the calculations of the timings below are different then in TRM. I think we need to take the whole offset into account, or do I miss something here? > + reg |= TIMING_TCR_TAR_TRR(val); > + > + val = max(max(timings->tCS_min, timings->tCH_min), > + max(timings->tALS_min, timings->tALH_min)) / period; > + if (val > 1) > + val -= 1; > + reg |= TIMING_TCS(val); See mask error in macro definition. > + > + val = max(timings->tRP_min, timings->tREA_max) + 6000; > + reg |= TIMING_TRP(val / 1000); > + reg |= TIMING_TRP_RESP(val / period); > + > + reg |= TIMING_TWB(timings->tWB_max / period); > + reg |= TIMING_TWHR(timings->tWHR_min / period); > + reg |= TIMING_TWH(timings->tWH_min / 1000); > + reg |= TIMING_TWP(timings->tWP_min / 1000); > + reg |= TIMING_TRH(timings->tRHW_min / 1000); Why 1000 for those three values? In my TRM, those values are in NAND_CLK_PERIOD too. > + > + writel(reg, nand->regs + TIMING_1); > + > + val = timings->tADL_min / period; > + if (val > 2) > + val -= 2; > + reg = TIMING_TADL(val); > + > + writel(reg, nand->regs + TIMING_2); > +} > + > +static void tegra_nand_setup_chiptiming(struct tegra_nand *nand) > +{ > + struct nand_chip *chip = &nand->chip; > + int mode; > + > + mode = onfi_get_async_timing_mode(chip); > + if (mode == ONFI_TIMING_MODE_UNKNOWN) > + mode = chip->onfi_timing_mode_default; > + else > + mode = fls(mode); > + > + tegra_nand_setup_timing(nand, mode); > +} > + > +static int tegra_nand_parse_dt(struct device_node *node, > + struct tegra_nand *nand) > +{ > + enum of_gpio_flags flags; > + > + nand->wp_gpio = of_get_named_gpio_flags(node, "nvidia,wp-gpios", 0, > + &flags); There is a non flags variant, this should work fine for this case I guess. > + if (nand->wp_gpio < 0) > + nand->wp_gpio = 0; > + > + nand->buswidth = of_get_nand_bus_width(node); > + if (nand->buswidth < 0) > + return nand->buswidth; > + > + return 0; > +} > + > +static const char * const part_probes[] = { > + "cmdlinepart", "ofpart", NULL }; > + > +static int tegra_nand_probe(struct platform_device *pdev) > +{ > + struct tegra_nand *nand; > + struct nand_chip *chip; > + struct mtd_info *mtd; > + struct resource *res; > + unsigned long value; > + int err = 0; > + > + nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL); > + if (!nand) > + return -ENOMEM; > + > + nand->dev = &pdev->dev; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + nand->regs = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(nand->regs)) > + return PTR_ERR(nand->regs); > + > + nand->irq = platform_get_irq(pdev, 0); Double space... > + err = devm_request_irq(&pdev->dev, nand->irq, tegra_nand_irq, 0, > + dev_name(&pdev->dev), nand); > + if (err) > + return err; > + > + nand->rst = devm_reset_control_get(&pdev->dev, "nand"); > + if (IS_ERR(nand->rst)) > + return PTR_ERR(nand->rst); > + > + nand->clk = devm_clk_get(&pdev->dev, "nand"); > + if (IS_ERR(nand->clk)) > + return PTR_ERR(nand->clk); > + > + err = tegra_nand_parse_dt(pdev->dev.of_node, nand); > + if (err) > + return err; > + > + err = clk_prepare_enable(nand->clk); > + if (err) > + return err; > + > + reset_control_assert(nand->rst); > + udelay(2); > + reset_control_deassert(nand->rst); > + > + if (gpio_is_valid(nand->wp_gpio)) { > + err = devm_gpio_request_one(&pdev->dev, nand->wp_gpio, > + GPIOF_OUT_INIT_HIGH, "tegra-nand-wp"); > + if (err) > + return err; > + } > + > + value = HWSTATUS_RDSTATUS_MASK(1) | HWSTATUS_RDSTATUS_VALUE(0) | > + HWSTATUS_RBSY_MASK(NAND_STATUS_READY) | > + HWSTATUS_RBSY_VALUE(NAND_STATUS_READY); > + writel(NAND_CMD_STATUS, nand->regs + HWSTATUS_CMD); > + writel(value, nand->regs + HWSTATUS_MASK); > + > + init_completion(&nand->command_complete); > + init_completion(&nand->dma_complete); > + > + mtd = &nand->mtd; > + mtd->name = dev_name(&pdev->dev); > + mtd->owner = THIS_MODULE; > + mtd->priv = &nand->chip; > + > + mtd->type = MTD_NANDFLASH; > + mtd->flags = MTD_CAP_NANDFLASH; > + > + /* clear interrupts */ > + value = readl(nand->regs + ISR); > + writel(value, nand->regs + ISR); > + > + writel(DMA_CTRL_IS_DONE, nand->regs + DMA_CTRL); > + > + /* enable interrupts */ > + value = IER_UND | IER_OVR | IER_CMD_DONE | IER_ECC_ERR | IER_GIE; > + writel(value, nand->regs + IER); > + > + chip = &nand->chip; > + chip->cmdfunc = tegra_nand_command; > + chip->select_chip = tegra_nand_select_chip; > + chip->read_byte = tegra_nand_read_byte; > + chip->read_buf = tegra_nand_read_buf; > + chip->write_buf = tegra_nand_write_buf; > + > + tegra_nand_setup_timing(nand, 0); > + > + err = nand_scan_ident(mtd, 1, NULL); > + if (err) > + return err; > + > + nand->data_buf = dmam_alloc_coherent(&pdev->dev, mtd->writesize, > + &nand->data_dma, GFP_KERNEL); > + if (!nand->data_buf) > + return -ENOMEM; > + > + nand->oob_buf = dmam_alloc_coherent(&pdev->dev, mtd->oobsize, > + &nand->oob_dma, GFP_KERNEL); > + if (!nand->oob_buf) > + return -ENOMEM; > + > + chip->ecc.mode = NAND_ECC_HW; > + chip->ecc.size = 512; > + chip->ecc.bytes = mtd->oobsize; > + chip->ecc.read_page = tegra_nand_read_page; > + chip->ecc.write_page = tegra_nand_write_page; > + > + value = CFG_HW_ECC | CFG_ECC_SEL | CFG_ERR_COR | CFG_PIPE_EN | > + CFG_TVAL_8 | CFG_SKIP_SPARE | CFG_SKIP_SPARE_SIZE_4; > + > + switch (mtd->oobsize) { > + case 16: > + chip->ecc.layout = &tegra_nand_oob_16; > + chip->ecc.strength = 1; > + value |= CFG_TAG_BYTE_SIZE(4); > + break; > + case 64: > + chip->ecc.layout = &tegra_nand_oob_64; > + chip->ecc.strength = 8; > + value |= CFG_TAG_BYTE_SIZE(36); > + break; > + case 128: > + chip->ecc.layout = &tegra_nand_oob_128; > + chip->ecc.strength = 8; > + value |= CFG_TAG_BYTE_SIZE(72); > + break; > + case 224: > + chip->ecc.layout = &tegra_nand_oob_224; > + chip->ecc.strength = 8; > + value |= CFG_TAG_BYTE_SIZE(144); > + break; > + default: > + dev_err(&pdev->dev, "unhandled OOB size %d\n", mtd->oobsize); > + return -ENODEV; > + } > + > + switch (mtd->writesize) { > + case 256: > + value |= CFG_PS_256; > + break; > + case 512: > + value |= CFG_PS_512; > + break; > + case 1024: > + value |= CFG_PS_1024; > + break; > + case 2048: > + value |= CFG_PS_2048; > + break; > + case 4096: > + value |= CFG_PS_4096; > + break; > + default: > + dev_err(&pdev->dev, "unhandled writesize %d\n", mtd->writesize); > + return -ENODEV; > + } > + > + if (nand->buswidth == 16) > + value |= CFG_BUS_WIDTH_16; > + > + writel(value, nand->regs + CFG); > + > + tegra_nand_setup_chiptiming(nand); > + > + err = nand_scan_tail(mtd); > + if (err) > + return err; > + > + mtd_device_parse_register(mtd, NULL, > + &(struct mtd_part_parser_data) { > + .of_node = pdev->dev.of_node, > + }, > + NULL, 0); > + > + platform_set_drvdata(pdev, nand); > + > + return 0; > +} > + > +static int tegra_nand_remove(struct platform_device *pdev) > +{ > + struct tegra_nand *nand = platform_get_drvdata(pdev); > + > + nand_release(&nand->mtd); > + > + clk_disable_unprepare(nand->clk); > + > + return 0; > +} > + > +static const struct of_device_id tegra_nand_of_match[] = { > + { .compatible = "nvidia,tegra20-nand" }, > + { .compatible = "nvidia,tegra30-nand" }, > + { } > +}; > + > +static struct platform_driver tegra_nand_driver = { > + .driver = { > + .name = "tegra-nand", > + .of_match_table = tegra_nand_of_match, > + }, > + .probe = tegra_nand_probe, > + .remove = tegra_nand_remove, > +}; > +module_platform_driver(tegra_nand_driver); > + > +MODULE_DESCRIPTION("NVIDIA Tegra NAND driver"); > +MODULE_AUTHOR("Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx"); > +MODULE_AUTHOR("Lucas Stach <dev@xxxxxxxxxx"); > +MODULE_LICENSE("GPL v2"); > +MODULE_DEVICE_TABLE(of, tegra_nand_of_match); -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html