nand/denali: Adding Denali NAND driver support

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <[email protected]>

Cc Artem Bityutskiy <[email protected]>

Cc David Woodhouse <[email protected]>

Cc Brian Norris <[email protected]>

Cc Scott Wood <[email protected]>

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h

drivers/mtd/nand/Makefile +1 -0

index 02b149c..24e8218 100644
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
+obj-$(CONFIG_NAND_DENALI) += denali_nand.o
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o
 obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o

drivers/mtd/nand/denali_nand.c new file +0 -0

new file mode 100644
index 0000000..55246c9

drivers/mtd/nand/denali_nand.c new file +79 -0

+ * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/errno.h>
+#include <asm/io.h>
+
+#include "denali_nand.h"
+
+/* We define a module parameter that allows the user to override
+ * the hardware and decide what timing mode should be used.
+ */
+#define NAND_DEFAULT_TIMINGS	-1
+
+static struct denali_nand_info denali;
+static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
+
+/* We define a macro here that combines all interrupts this driver uses into
+ * a single constant value, for convenience. */
+#define DENALI_IRQ_ALL	(INTR_STATUS__DMA_CMD_COMP | \
+			INTR_STATUS__ECC_TRANSACTION_DONE | \
+			INTR_STATUS__ECC_ERR | \
+			INTR_STATUS__PROGRAM_FAIL | \
+			INTR_STATUS__LOAD_COMP | \
+			INTR_STATUS__PROGRAM_COMP | \
+			INTR_STATUS__TIME_OUT | \
+			INTR_STATUS__ERASE_FAIL | \
+			INTR_STATUS__RST_COMP | \
+			INTR_STATUS__ERASE_COMP | \
+			INTR_STATUS__ECC_UNCOR_ERR | \
+			INTR_STATUS__INT_ACT | \
+			INTR_STATUS__LOCKED_BLK)
+
+/* indicates whether or not the internal value for the flash bank is
+ * valid or not */
+#define CHIP_SELECT_INVALID	-1
+
+#define SUPPORT_8BITECC		1
+
+/* This macro divides two integers and rounds fractional values up
+ * to the nearest integer value. */
+#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
+
+/* These constants are defined by the driver to enable common driver
+ * configuration options. */
+#define SPARE_ACCESS		0x41
+#define MAIN_ACCESS		0x42
+#define MAIN_SPARE_ACCESS	0x43
+
+#define DENALI_UNLOCK_START	0x10
+#define DENALI_UNLOCK_END	0x11
+#define DENALI_LOCK		0x21
+#define DENALI_LOCK_TIGHT	0x31
+#define DENALI_BUFFER_LOAD	0x60
+#define DENALI_BUFFER_WRITE	0x62
+
+#define DENALI_READ	0
+#define DENALI_WRITE	0x100
+
+/* types of device accesses. We can issue commands and get status */
+#define COMMAND_CYCLE	0
+#define ADDR_CYCLE	1
+#define STATUS_CYCLE	2
+
+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */
+static inline void clear_interrupt(uint32_t irq_mask)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
+}
+

drivers/mtd/nand/denali_nand.c new file +17 -0

+
+	do {
+		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+		if (intr_status & irq_mask) {
+			denali.irq_status &= ~irq_mask;
+			/* our interrupt was detected */
+			break;
+		}
+		udelay(1);
+		comp_res--;
+	} while (comp_res != 0);
+
+	if (comp_res == 0) {
+		/* timeout */
+		printf("Denali timeout with interrupt status %08x\n",
+			read_interrupt_status());
+		intr_status = 0;

drivers/mtd/nand/denali_nand.c new file +20 -0

+static void index_addr(uint32_t address, uint32_t data)
+{
+	__raw_writel(address, denali.flash_mem);
+	__raw_writel(data, denali.flash_mem + 0x10);
+}
+
+/* Perform an indexed read of the device */
+static void index_addr_read_data(uint32_t address, uint32_t *pdata)
+{
+	__raw_writel(address, denali.flash_mem);
+	*pdata = __raw_readl(denali.flash_mem + 0x10);
+}
+
+/* We need to buffer some data for some of the NAND core routines.
+ * The operations manage buffering that data. */
+static void reset_buf(void)
+{
+	denali.buf.head = denali.buf.tail = 0;
+}
+

drivers/mtd/nand/denali_nand.c new file +8 -0

+	uint32_t irq_mask = INTR_STATUS__RST_COMP |
+			    INTR_STATUS__TIME_OUT;
+
+	clear_interrupts();
+
+	__raw_writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
+
+	irq_status = wait_for_irq(irq_mask);

drivers/mtd/nand/denali_nand.c new file +131 -0

+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+		denali.flash_reg + INTR_STATUS(i));
+
+	for (i = 0 ; i < denali.max_banks; i++) {
+		__raw_writel(1 << i, denali.flash_reg + DEVICE_RESET);
+		while (!(__raw_readl(denali.flash_reg +	INTR_STATUS(i)) &
+			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
+			if (__raw_readl(denali.flash_reg + INTR_STATUS(i)) &
+				INTR_STATUS__TIME_OUT)
+				debug(KERN_DEBUG "NAND Reset operation "
+					"timed out on bank %d\n", i);
+	}
+
+	for (i = 0; i < denali.max_banks; i++)
+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+			denali.flash_reg + INTR_STATUS(i));
+
+	return PASS;
+}
+
+/* this routine calculates the ONFI timing values for a given mode and
+ * programs the clocking register accordingly. The mode is determined by
+ * the get_onfi_nand_para routine.
+ */
+static void nand_onfi_timing_set(uint16_t mode)
+{
+	uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
+	uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
+	uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
+	uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
+	uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
+	uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
+	uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
+	uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
+	uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
+	uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
+
+	uint16_t TclsRising = 1;
+	uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
+	uint16_t dv_window = 0;
+	uint16_t en_lo, en_hi;
+	uint16_t acc_clks;
+	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
+
+	en_lo = CEIL_DIV(Trp[mode], CLK_X);
+	en_hi = CEIL_DIV(Treh[mode], CLK_X);
+#if ONFI_BLOOM_TIME
+	if ((en_hi * CLK_X) < (Treh[mode] + 2))
+		en_hi++;
+#endif
+
+	if ((en_lo + en_hi) * CLK_X < Trc[mode])
+		en_lo += CEIL_DIV((Trc[mode] - (en_lo + en_hi) * CLK_X), CLK_X);
+
+	if ((en_lo + en_hi) < CLK_MULTI)
+		en_lo += CLK_MULTI - en_lo - en_hi;
+
+	while (dv_window < 8) {
+		data_invalid_rhoh = en_lo * CLK_X + Trhoh[mode];
+
+		data_invalid_rloh = (en_lo + en_hi) * CLK_X + Trloh[mode];
+
+		data_invalid =
+		    data_invalid_rhoh <
+		    data_invalid_rloh ? data_invalid_rhoh : data_invalid_rloh;
+
+		dv_window = data_invalid - Trea[mode];
+
+		if (dv_window < 8)
+			en_lo++;
+	}
+
+	acc_clks = CEIL_DIV(Trea[mode], CLK_X);
+
+	while (((acc_clks * CLK_X) - Trea[mode]) < 3)
+		acc_clks++;
+
+	if ((data_invalid - acc_clks * CLK_X) < 2)
+		debug(KERN_WARNING "%s, Line %d: Warning!\n",
+			__FILE__, __LINE__);
+
+	addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
+	re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
+	re_2_re = CEIL_DIV(Trhz[mode], CLK_X);
+	we_2_re = CEIL_DIV(Twhr[mode], CLK_X);
+	cs_cnt = CEIL_DIV((Tcs[mode] - Trp[mode]), CLK_X);
+	if (!TclsRising)
+		cs_cnt = CEIL_DIV(Tcs[mode], CLK_X);
+	if (cs_cnt == 0)
+		cs_cnt = 1;
+
+	if (Tcea[mode]) {
+		while (((cs_cnt * CLK_X) + Trea[mode]) < Tcea[mode])
+			cs_cnt++;
+	}
+
+#if MODE5_WORKAROUND
+	if (mode == 5)
+		acc_clks = 5;
+#endif
+
+	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
+	if ((__raw_readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
+		(__raw_readl(denali.flash_reg + DEVICE_ID) == 0x88))
+		acc_clks = 6;
+
+	__raw_writel(acc_clks, denali.flash_reg + ACC_CLKS);
+	__raw_writel(re_2_we, denali.flash_reg + RE_2_WE);
+	__raw_writel(re_2_re, denali.flash_reg + RE_2_RE);
+	__raw_writel(we_2_re, denali.flash_reg + WE_2_RE);
+	__raw_writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
+	__raw_writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
+	__raw_writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
+	__raw_writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
+}
+
+/* queries the NAND device to see what ONFI modes it supports. */
+static uint16_t get_onfi_nand_para(void)
+{
+	int i;
+	/* we needn't to do a reset here because driver has already
+	 * reset all the banks before
+	 * */
+	if (!(__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
+		ONFI_TIMING_MODE__VALUE))
+		return FAIL;
+
+	for (i = 5; i > 0; i--) {
+		if (__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &

drivers/mtd/nand/denali_nand.c new file +7 -0

+/* lld_nand.h */
+/*
+ * NAND Flash Controller Device Driver
+ * Copyright (c) 2009, Intel Corporation and its suppliers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,

drivers/mtd/nand/denali_nand.c line 2

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +2 -0

@@ -0,0 +1,1166 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>

What about 2014?

Hello Michal,

These files were imported from Linux Kernel.
(drivers/mtd/nand/denali.[ch])

I guess Chin does not want to change the code unless it is really necessary.
(And I like this way.
We can easily find which parts were adjusted by diffing.)

But, good catch!
I think your feedback is highly appreciated for Linux folks. Can you post your feedback to Linux Kernel?

Quoted 34 lines · Chin Liang See

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <[email protected]>

Cc Artem Bityutskiy <[email protected]>

Cc David Woodhouse <[email protected]>

Cc Brian Norris <[email protected]>

Cc Scott Wood <[email protected]>

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h

drivers/mtd/nand/Makefile +1 -0

index 02b149c..24e8218 100644
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
+obj-$(CONFIG_NAND_DENALI) += denali_nand.o
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o
 obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o

drivers/mtd/nand/denali_nand.c new file +0 -0

new file mode 100644

drivers/mtd/nand/denali_nand.c line 2

Quoted 6 lines

index 0000000..55246c9

drivers/mtd/nand/denali_nand.c new file +2 -0

@@ -0,0 +1,1166 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>

What about 2014?

Quoted 85 lines

drivers/mtd/nand/denali_nand.c new file +83 -0

+ * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/errno.h>
+#include <asm/io.h>
+
+#include "denali_nand.h"
+
+/* We define a module parameter that allows the user to override
+ * the hardware and decide what timing mode should be used.
+ */
+#define NAND_DEFAULT_TIMINGS	-1
+
+static struct denali_nand_info denali;
+static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
+
+/* We define a macro here that combines all interrupts this driver uses into
+ * a single constant value, for convenience. */
+#define DENALI_IRQ_ALL	(INTR_STATUS__DMA_CMD_COMP | \
+			INTR_STATUS__ECC_TRANSACTION_DONE | \
+			INTR_STATUS__ECC_ERR | \
+			INTR_STATUS__PROGRAM_FAIL | \
+			INTR_STATUS__LOAD_COMP | \
+			INTR_STATUS__PROGRAM_COMP | \
+			INTR_STATUS__TIME_OUT | \
+			INTR_STATUS__ERASE_FAIL | \
+			INTR_STATUS__RST_COMP | \
+			INTR_STATUS__ERASE_COMP | \
+			INTR_STATUS__ECC_UNCOR_ERR | \
+			INTR_STATUS__INT_ACT | \
+			INTR_STATUS__LOCKED_BLK)
+
+/* indicates whether or not the internal value for the flash bank is
+ * valid or not */
+#define CHIP_SELECT_INVALID	-1
+
+#define SUPPORT_8BITECC		1
+
+/* This macro divides two integers and rounds fractional values up
+ * to the nearest integer value. */
+#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
+
+/* These constants are defined by the driver to enable common driver
+ * configuration options. */
+#define SPARE_ACCESS		0x41
+#define MAIN_ACCESS		0x42
+#define MAIN_SPARE_ACCESS	0x43
+
+#define DENALI_UNLOCK_START	0x10
+#define DENALI_UNLOCK_END	0x11
+#define DENALI_LOCK		0x21
+#define DENALI_LOCK_TIGHT	0x31
+#define DENALI_BUFFER_LOAD	0x60
+#define DENALI_BUFFER_WRITE	0x62
+
+#define DENALI_READ	0
+#define DENALI_WRITE	0x100
+
+/* types of device accesses. We can issue commands and get status */
+#define COMMAND_CYCLE	0
+#define ADDR_CYCLE	1
+#define STATUS_CYCLE	2
+
+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */
+static inline void clear_interrupt(uint32_t irq_mask)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
+}
+
+static uint32_t read_interrupt_status(void)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);

drivers/mtd/nand/denali_nand.c line 91

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+	return __raw_readl(denali.flash_reg + intr_status_reg);
+}
+
+static void clear_interrupts(void)
+{
+	uint32_t status = 0x0;

just 0

drivers/mtd/nand/denali_nand.c line 94

Quoted 4 lines

drivers/mtd/nand/denali_nand.c new file +3 -0

+	status = read_interrupt_status();
+	clear_interrupt(status);
+	denali.irq_status = 0x0;

just 0

Quoted 4 lines

drivers/mtd/nand/denali_nand.c new file +3 -0

+}
+
+static void denali_irq_enable(uint32_t int_mask)

drivers/mtd/nand/denali_nand.c line 107

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+{
+	int i;
+	for (i = 0; i < denali.max_banks; ++i)
+		__raw_writel(int_mask, denali.flash_reg + INTR_EN(i));
+}
+
+static uint32_t wait_for_irq(uint32_t irq_mask)
+{
+	unsigned long comp_res = 1000;
+	uint32_t intr_status = 0;

do you need to init this?

Quoted 22 lines

drivers/mtd/nand/denali_nand.c new file +21 -0

+
+	do {
+		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+		if (intr_status & irq_mask) {
+			denali.irq_status &= ~irq_mask;
+			/* our interrupt was detected */
+			break;
+		}
+		udelay(1);
+		comp_res--;
+	} while (comp_res != 0);
+
+	if (comp_res == 0) {
+		/* timeout */
+		printf("Denali timeout with interrupt status %08x\n",
+			read_interrupt_status());
+		intr_status = 0;
+	}
+	return intr_status;
+}
+

drivers/mtd/nand/denali_nand.c line 134

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+/* Certain operations for the denali NAND controller use
+ * an indexed mode to read/write data. The operation is
+ * performed by writing the address value of the command
+ * to the device memory followed by the data. This function
+ * abstracts this common operation.
+*/

weird comment coding style check globally.

Quoted 21 lines

drivers/mtd/nand/denali_nand.c new file +20 -0

+static void index_addr(uint32_t address, uint32_t data)
+{
+	__raw_writel(address, denali.flash_mem);
+	__raw_writel(data, denali.flash_mem + 0x10);
+}
+
+/* Perform an indexed read of the device */
+static void index_addr_read_data(uint32_t address, uint32_t *pdata)
+{
+	__raw_writel(address, denali.flash_mem);
+	*pdata = __raw_readl(denali.flash_mem + 0x10);
+}
+
+/* We need to buffer some data for some of the NAND core routines.
+ * The operations manage buffering that data. */
+static void reset_buf(void)
+{
+	denali.buf.head = denali.buf.tail = 0;
+}
+

drivers/mtd/nand/denali_nand.c line 164

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+static void write_byte_to_buf(uint8_t byte)
+{
+	BUG_ON(denali.buf.tail >= sizeof(denali.buf.buf));
+	denali.buf.buf[denali.buf.tail++] = byte;
+}
+
+/* resets a specific device connected to the core */
+static void reset_bank(void)
+{
+	uint32_t irq_status = 0;

ditto.

Quoted 8 lines

drivers/mtd/nand/denali_nand.c new file +8 -0

+	uint32_t irq_mask = INTR_STATUS__RST_COMP |
+			    INTR_STATUS__TIME_OUT;
+
+	clear_interrupts();
+
+	__raw_writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
+
+	irq_status = wait_for_irq(irq_mask);

drivers/mtd/nand/denali_nand.c line 182

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+	if (irq_status & INTR_STATUS__TIME_OUT)
+		debug(KERN_ERR "reset bank failed.\n");
+}
+
+/* Reset the flash controller */
+static uint16_t denali_nand_reset(void)
+{
+	uint32_t i;
+
+	for (i = 0 ; i < denali.max_banks; i++)

fix all checkpatch warnings. This is also broken coding style.

total: 0 errors, 15 warnings, 24 checks, 1674 lines checked

Quoted 138 lines

drivers/mtd/nand/denali_nand.c new file +135 -0

+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+		denali.flash_reg + INTR_STATUS(i));
+
+	for (i = 0 ; i < denali.max_banks; i++) {
+		__raw_writel(1 << i, denali.flash_reg + DEVICE_RESET);
+		while (!(__raw_readl(denali.flash_reg +	INTR_STATUS(i)) &
+			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
+			if (__raw_readl(denali.flash_reg + INTR_STATUS(i)) &
+				INTR_STATUS__TIME_OUT)
+				debug(KERN_DEBUG "NAND Reset operation "
+					"timed out on bank %d\n", i);
+	}
+
+	for (i = 0; i < denali.max_banks; i++)
+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+			denali.flash_reg + INTR_STATUS(i));
+
+	return PASS;
+}
+
+/* this routine calculates the ONFI timing values for a given mode and
+ * programs the clocking register accordingly. The mode is determined by
+ * the get_onfi_nand_para routine.
+ */
+static void nand_onfi_timing_set(uint16_t mode)
+{
+	uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
+	uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
+	uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
+	uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
+	uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
+	uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
+	uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
+	uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
+	uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
+	uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
+
+	uint16_t TclsRising = 1;
+	uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
+	uint16_t dv_window = 0;
+	uint16_t en_lo, en_hi;
+	uint16_t acc_clks;
+	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
+
+	en_lo = CEIL_DIV(Trp[mode], CLK_X);
+	en_hi = CEIL_DIV(Treh[mode], CLK_X);
+#if ONFI_BLOOM_TIME
+	if ((en_hi * CLK_X) < (Treh[mode] + 2))
+		en_hi++;
+#endif
+
+	if ((en_lo + en_hi) * CLK_X < Trc[mode])
+		en_lo += CEIL_DIV((Trc[mode] - (en_lo + en_hi) * CLK_X), CLK_X);
+
+	if ((en_lo + en_hi) < CLK_MULTI)
+		en_lo += CLK_MULTI - en_lo - en_hi;
+
+	while (dv_window < 8) {
+		data_invalid_rhoh = en_lo * CLK_X + Trhoh[mode];
+
+		data_invalid_rloh = (en_lo + en_hi) * CLK_X + Trloh[mode];
+
+		data_invalid =
+		    data_invalid_rhoh <
+		    data_invalid_rloh ? data_invalid_rhoh : data_invalid_rloh;
+
+		dv_window = data_invalid - Trea[mode];
+
+		if (dv_window < 8)
+			en_lo++;
+	}
+
+	acc_clks = CEIL_DIV(Trea[mode], CLK_X);
+
+	while (((acc_clks * CLK_X) - Trea[mode]) < 3)
+		acc_clks++;
+
+	if ((data_invalid - acc_clks * CLK_X) < 2)
+		debug(KERN_WARNING "%s, Line %d: Warning!\n",
+			__FILE__, __LINE__);
+
+	addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
+	re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
+	re_2_re = CEIL_DIV(Trhz[mode], CLK_X);
+	we_2_re = CEIL_DIV(Twhr[mode], CLK_X);
+	cs_cnt = CEIL_DIV((Tcs[mode] - Trp[mode]), CLK_X);
+	if (!TclsRising)
+		cs_cnt = CEIL_DIV(Tcs[mode], CLK_X);
+	if (cs_cnt == 0)
+		cs_cnt = 1;
+
+	if (Tcea[mode]) {
+		while (((cs_cnt * CLK_X) + Trea[mode]) < Tcea[mode])
+			cs_cnt++;
+	}
+
+#if MODE5_WORKAROUND
+	if (mode == 5)
+		acc_clks = 5;
+#endif
+
+	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
+	if ((__raw_readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
+		(__raw_readl(denali.flash_reg + DEVICE_ID) == 0x88))
+		acc_clks = 6;
+
+	__raw_writel(acc_clks, denali.flash_reg + ACC_CLKS);
+	__raw_writel(re_2_we, denali.flash_reg + RE_2_WE);
+	__raw_writel(re_2_re, denali.flash_reg + RE_2_RE);
+	__raw_writel(we_2_re, denali.flash_reg + WE_2_RE);
+	__raw_writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
+	__raw_writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
+	__raw_writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
+	__raw_writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
+}
+
+/* queries the NAND device to see what ONFI modes it supports. */
+static uint16_t get_onfi_nand_para(void)
+{
+	int i;
+	/* we needn't to do a reset here because driver has already
+	 * reset all the banks before
+	 * */
+	if (!(__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
+		ONFI_TIMING_MODE__VALUE))
+		return FAIL;
+
+	for (i = 5; i > 0; i--) {
+		if (__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
+			(0x01 << i))
+			break;
+	}
+

drivers/mtd/nand/denali_nand.c line 323

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+	nand_onfi_timing_set(i);
+
+	/* By now, all the ONFI devices we know support the page cache */
+	/* rw feature. So here we enable the pipeline_rw_ahead feature */
+	/* __raw_writel(1, denali.flash_reg + CACHE_WRITE_ENABLE); */
+	/* __raw_writel(1, denali.flash_reg + CACHE_READ_ENABLE);  */

Dead code?

<snip>

Quoted 12 lines

drivers/mtd/nand/denali_nand.c new file +12 -0

+/* lld_nand.h */
+/*
+ * NAND Flash Controller Device Driver
+ * Copyright (c) 2009, Intel Corporation and its suppliers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for

Quoted 5 lines

+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

Isn't this pretty weird if we are using SPDX?

Thanks,
Michal

Quoted 12 lines

Quoted 12 lines

drivers/mtd/nand/denali_nand.h new file +12 -0

+/* lld_nand.h */
+/*
+ * NAND Flash Controller Device Driver
+ * Copyright (c) 2009, Intel Corporation and its suppliers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for

Quoted 8 lines

Quoted 5 lines

+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

Isn't this pretty weird if we are using SPDX?

I believe the Linux driver owner wish to maintain this header when he/she copy into NAND driver. Anyhow, the SPDX license already stated at top of the header file.

Quoted 21 lines

Quoted 12 lines

Quoted 12 lines

drivers/mtd/nand/denali_nand.h new file +12 -0

+/* lld_nand.h */
+/*
+ * NAND Flash Controller Device Driver
+ * Copyright (c) 2009, Intel Corporation and its suppliers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for

Quoted 8 lines

Quoted 5 lines

+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

Isn't this pretty weird if we are using SPDX?

Quoted 3 lines

I believe the Linux driver owner wish to maintain this header when he/she copy into NAND driver. Anyhow, the SPDX license already stated at top of the header file.

1. Using address in license is just wrong and it should be removed because they can move to different location.

2. u-boot adopted SPDX and all these fragments were removed.

Wolfgang/Tom: IMHO this should be also changed/fixed.

Thanks,
Michal

Quoted 30 lines · Michal Simek

Quoted 21 lines

Quoted 12 lines

Quoted 12 lines

drivers/mtd/nand/denali_nand.h new file +12 -0

+/* lld_nand.h */
+/*
+ * NAND Flash Controller Device Driver
+ * Copyright (c) 2009, Intel Corporation and its suppliers.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for

Quoted 8 lines

Quoted 5 lines

+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

Isn't this pretty weird if we are using SPDX?

Quoted 3 lines

I believe the Linux driver owner wish to maintain this header when he/she copy into NAND driver. Anyhow, the SPDX license already stated at top of the header file.

1. Using address in license is just wrong and it should be removed because they can move to different location.

2. u-boot adopted SPDX and all these fragments were removed.

Quoted 1 line

Wolfgang/Tom: IMHO this should be also changed/fixed.

Just check the latest denali.h in Linux and this license header still there. But removing the license header put by original author might not a good thing to do. Unless we can get some approval from Wolfgang or Jason (who is submitter of this file to Linux).

drivers/mtd/nand/denali_nand.c new file +10 -0

+
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
+	nand->ecc.read_oob = denali_read_oob;
+	nand->ecc.write_oob = denali_write_oob;
+	nand->ecc.read_page = denali_read_page;
+	nand->ecc.read_page_raw = denali_read_page_raw;
+	nand->ecc.write_page = denali_write_page;
+	nand->ecc.write_page_raw = denali_write_page_raw;
+#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST

Hello Chin,

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
+	nand->ecc.read_oob = denali_read_oob;
+	nand->ecc.write_oob = denali_write_oob;
+	nand->ecc.read_page = denali_read_page;
+	nand->ecc.read_page_raw = denali_read_page_raw;
+	nand->ecc.write_page = denali_write_page;
+	nand->ecc.write_page_raw = denali_write_page_raw;
+#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST

drivers/mtd/nand/denali_nand.c line 1150

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+	/* 15bit ECC */
+	nand->ecc.bytes = 26;
+	nand->ecc.layout = &nand_15bit_oob;
+#else	/* 8bit ECC */
+	nand->ecc.bytes = 14;
+	nand->ecc.layout = &nand_8bit_oob;
+#endif
+	nand->ecc.calculate = denali_ecc_calculate;
+	nand->ecc.correct  = denali_ecc_correct;
+	nand->ecc.hwctl  = denali_ecc_hwctl;

You set nand->ecc.mode = NAND_ECC_HW,
but it looks like you don't set nand->ecc.strength.

So, I think initialization will fail in nand_scan_tail() function.

Here,

                if (mtd->writesize >= chip->ecc.size) {
                        if (!chip->ecc.strength) {
                                pr_warn("Driver must set ecc.strength when using hardware ECC\n");
                                BUG();
                        }
                        break;

Where do you set nand->ecc.strength?

Best Regards
Masahiro Yamada

Hi Masahiro,

Quoted 40 lines · Masahiro Yamada

Hello Chin,

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
+	nand->ecc.read_oob = denali_read_oob;
+	nand->ecc.write_oob = denali_write_oob;
+	nand->ecc.read_page = denali_read_page;
+	nand->ecc.read_page_raw = denali_read_page_raw;
+	nand->ecc.write_page = denali_write_page;
+	nand->ecc.write_page_raw = denali_write_page_raw;
+#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST

drivers/mtd/nand/denali_nand.c line 1150

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+	/* 15bit ECC */
+	nand->ecc.bytes = 26;
+	nand->ecc.layout = &nand_15bit_oob;
+#else	/* 8bit ECC */
+	nand->ecc.bytes = 14;
+	nand->ecc.layout = &nand_8bit_oob;
+#endif
+	nand->ecc.calculate = denali_ecc_calculate;
+	nand->ecc.correct  = denali_ecc_correct;
+	nand->ecc.hwctl  = denali_ecc_hwctl;

You set nand->ecc.mode = NAND_ECC_HW,
but it looks like you don't set nand->ecc.strength.

So, I think initialization will fail in nand_scan_tail() function.

Here,

                if (mtd->writesize >= chip->ecc.size) {
                        if (!chip->ecc.strength) {
                                pr_warn("Driver must set ecc.strength when using hardware ECC\n");
                                BUG();
                        }
                        break;

Quoted 1 line

Where do you set nand->ecc.strength?

I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are using the NAND_ECC_HW (without the syndrome). Wonder you hit error during run?

Quoted 43 lines · Chin Liang See

Hi Masahiro,

Quoted 40 lines · Masahiro Yamada

Hello Chin,

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE;
+	nand->ecc.read_oob = denali_read_oob;
+	nand->ecc.write_oob = denali_write_oob;
+	nand->ecc.read_page = denali_read_page;
+	nand->ecc.read_page_raw = denali_read_page_raw;
+	nand->ecc.write_page = denali_write_page;
+	nand->ecc.write_page_raw = denali_write_page_raw;
+#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST

drivers/mtd/nand/denali_nand.c line 1150

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+	/* 15bit ECC */
+	nand->ecc.bytes = 26;
+	nand->ecc.layout = &nand_15bit_oob;
+#else	/* 8bit ECC */
+	nand->ecc.bytes = 14;
+	nand->ecc.layout = &nand_8bit_oob;
+#endif
+	nand->ecc.calculate = denali_ecc_calculate;
+	nand->ecc.correct  = denali_ecc_correct;
+	nand->ecc.hwctl  = denali_ecc_hwctl;

You set nand->ecc.mode = NAND_ECC_HW,
but it looks like you don't set nand->ecc.strength.

So, I think initialization will fail in nand_scan_tail() function.

Here,

                if (mtd->writesize >= chip->ecc.size) {
                        if (!chip->ecc.strength) {
                                pr_warn("Driver must set ecc.strength when using hardware ECC\n");
                                BUG();
                        }
                        break;

Quoted 5 lines

Quoted 1 line

Where do you set nand->ecc.strength?

I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are using the NAND_ECC_HW (without the syndrome). Wonder you hit error during run?

No, it must always be set for hardware ECC. Note the lack of a break; before case NAND_ECC_HW_SYNDROME.

-Scott

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Where do you set nand->ecc.strength?

I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are using the NAND_ECC_HW (without the syndrome). Wonder you hit error during run?

No, it must always be set for hardware ECC. Note the lack of a break; before case NAND_ECC_HW_SYNDROME.

Good catch, thanks Scott!

Hi Masahiro,

I rechecked my documentation and the value is 8.
The data sector size is 512 bytes while ECC sector size is 14 bytes. With that, the controller able to auto correct up to 8 bits. This is how a page will look like

512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 470 bytes data | 2 byte for bad block marker | 42 bytes data | 14 bytes ECC | unused

FYI, my documentation is located at
http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=doc/README.SOCFPGA;hb=refs/heads/socfpga_v2013.01.01

#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
#define ECC_15BITS 26

static struct nand_ecclayout nand_15bit_oob = {
	.eccbytes = ECC_15BITS,
};

#else
#define ECC_8BITS 14
static struct nand_ecclayout nand_8bit_oob = {
.eccbytes = ECC_8BITS,
};
#endif /* CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST */

If you want to run under SPL, there are some patches for that. Let me know if you need that. While for U-Boot, they are working fine. Probably

Hello Chin,

Quoted 10 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Where do you set nand->ecc.strength?

I believe this is only applicable for NAND_ECC_HW_SYNDROME mode. We are using the NAND_ECC_HW (without the syndrome). Wonder you hit error during run?

No, it must always be set for hardware ECC. Note the lack of a break; before case NAND_ECC_HW_SYNDROME.

Good catch, thanks Scott!

ecc.strengh must be set for NAND_ECC_HW.

Otherwise, error message will be displayed and reboot.

NAND: NAND: Denali NAND controller

   BUG: failure at drivers/mtd/nand/nand_base.c:3224/nand_scan_tail()!
   BUG!
   resetting ...

You said this driver was tested against 3 different devices.

In that case, I can't understand how your board passed through nand_scan_tail().

Anyway, I modifed denali_nand_init() locally
to set nand->ecc.strength.

Quoted 9 lines

Hi Masahiro,

I rechecked my documentation and the value is 8.
The data sector size is 512 bytes while ECC sector size is 14 bytes. With that, the controller able to auto correct up to 8 bits. This is how a page will look like

Quoted 6 lines

512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 512 bytes data | 14 bytes ECC | 470 bytes data | 2 byte for bad block marker | 42 bytes data | 14 bytes ECC | unused

FYI, my documentation is located at
http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=doc/README.SOCFPGA;hb=refs/heads/socfpga_v2013.01.01

For SOCFPAG, Denali controller IP is configured with 512 byte ECC sector size. OK.

I refer to "Denali NAND Flash Memory Controller User's Guide".

Accoding to it, Denali's IP has 2 choice for ECC sector size:
512 byte or 1024 byte.

Panasonic's UniPhier SoCs adopt 1024 byte ECC sector size. (CONFIG_NAND_DENALI_ECC_SIZE = nand->ecc.size = 1024 for us.) ECC strength (nand->ecc.strength) is selectable from 8bit/16bit/24bit. (They correspond to nand->ecc.bytes = 14, 28, 42, respectively)

So, In our SoC s
1024 byte data | {14 or 28 or 42 byte ECC} | 1024 byte data | {14 or 28 or 42 byte ECC} ...

Quoted 7 lines

#ifdef CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST
#define ECC_15BITS 26

static struct nand_ecclayout nand_15bit_oob = {
	.eccbytes = ECC_15BITS,
};

Quoted 6 lines

#else
#define ECC_8BITS 14
static struct nand_ecclayout nand_8bit_oob = {
.eccbytes = ECC_8BITS,
};
#endif /* CONFIG_SYS_NAND_15BIT_HW_ECC_OOBFIRST */

I'm afraid this part works only for 512 byte ECC sector.

drivers/mtd/nand/denali_nand.c new file +8 -0

@@ -0,0 +1,1166 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>
+ * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>

drivers/mtd/nand/denali_nand.c new file +51 -0

+static struct denali_nand_info denali;
+static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
+
+/* We define a macro here that combines all interrupts this driver uses into
+ * a single constant value, for convenience. */
+#define DENALI_IRQ_ALL	(INTR_STATUS__DMA_CMD_COMP | \
+			INTR_STATUS__ECC_TRANSACTION_DONE | \
+			INTR_STATUS__ECC_ERR | \
+			INTR_STATUS__PROGRAM_FAIL | \
+			INTR_STATUS__LOAD_COMP | \
+			INTR_STATUS__PROGRAM_COMP | \
+			INTR_STATUS__TIME_OUT | \
+			INTR_STATUS__ERASE_FAIL | \
+			INTR_STATUS__RST_COMP | \
+			INTR_STATUS__ERASE_COMP | \
+			INTR_STATUS__ECC_UNCOR_ERR | \
+			INTR_STATUS__INT_ACT | \
+			INTR_STATUS__LOCKED_BLK)
+
+/* indicates whether or not the internal value for the flash bank is
+ * valid or not */
+#define CHIP_SELECT_INVALID	-1
+
+#define SUPPORT_8BITECC		1
+
+/* This macro divides two integers and rounds fractional values up
+ * to the nearest integer value. */
+#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
+
+/* These constants are defined by the driver to enable common driver
+ * configuration options. */
+#define SPARE_ACCESS		0x41
+#define MAIN_ACCESS		0x42
+#define MAIN_SPARE_ACCESS	0x43
+
+#define DENALI_UNLOCK_START	0x10
+#define DENALI_UNLOCK_END	0x11
+#define DENALI_LOCK		0x21
+#define DENALI_LOCK_TIGHT	0x31
+#define DENALI_BUFFER_LOAD	0x60
+#define DENALI_BUFFER_WRITE	0x62
+
+#define DENALI_READ	0
+#define DENALI_WRITE	0x100
+
+/* types of device accesses. We can issue commands and get status */
+#define COMMAND_CYCLE	0
+#define ADDR_CYCLE	1
+#define STATUS_CYCLE	2
+
+/* this is a helper macro that allows us to

drivers/mtd/nand/denali_nand.c new file +27 -0

+static uint32_t read_interrupt_status(void)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	return __raw_readl(denali.flash_reg + intr_status_reg);
+}
+
+static void clear_interrupts(void)
+{
+	uint32_t status = 0x0;
+	status = read_interrupt_status();
+	clear_interrupt(status);
+	denali.irq_status = 0x0;
+}
+
+static void denali_irq_enable(uint32_t int_mask)
+{
+	int i;
+	for (i = 0; i < denali.max_banks; ++i)
+		__raw_writel(int_mask, denali.flash_reg + INTR_EN(i));
+}
+
+static uint32_t wait_for_irq(uint32_t irq_mask)
+{
+	unsigned long comp_res = 1000;
+	uint32_t intr_status = 0;
+

drivers/mtd/nand/denali_nand.c new file +161 -0

+
+/* Perform an indexed read of the device */
+static void index_addr_read_data(uint32_t address, uint32_t *pdata)
+{
+	__raw_writel(address, denali.flash_mem);
+	*pdata = __raw_readl(denali.flash_mem + 0x10);
+}
+
+/* We need to buffer some data for some of the NAND core routines.
+ * The operations manage buffering that data. */
+static void reset_buf(void)
+{
+	denali.buf.head = denali.buf.tail = 0;
+}
+
+static void write_byte_to_buf(uint8_t byte)
+{
+	BUG_ON(denali.buf.tail >= sizeof(denali.buf.buf));
+	denali.buf.buf[denali.buf.tail++] = byte;
+}
+
+/* resets a specific device connected to the core */
+static void reset_bank(void)
+{
+	uint32_t irq_status = 0;
+	uint32_t irq_mask = INTR_STATUS__RST_COMP |
+			    INTR_STATUS__TIME_OUT;
+
+	clear_interrupts();
+
+	__raw_writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
+
+	irq_status = wait_for_irq(irq_mask);
+	if (irq_status & INTR_STATUS__TIME_OUT)
+		debug(KERN_ERR "reset bank failed.\n");
+}
+
+/* Reset the flash controller */
+static uint16_t denali_nand_reset(void)
+{
+	uint32_t i;
+
+	for (i = 0 ; i < denali.max_banks; i++)
+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+		denali.flash_reg + INTR_STATUS(i));
+
+	for (i = 0 ; i < denali.max_banks; i++) {
+		__raw_writel(1 << i, denali.flash_reg + DEVICE_RESET);
+		while (!(__raw_readl(denali.flash_reg +	INTR_STATUS(i)) &
+			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
+			if (__raw_readl(denali.flash_reg + INTR_STATUS(i)) &
+				INTR_STATUS__TIME_OUT)
+				debug(KERN_DEBUG "NAND Reset operation "
+					"timed out on bank %d\n", i);
+	}
+
+	for (i = 0; i < denali.max_banks; i++)
+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+			denali.flash_reg + INTR_STATUS(i));
+
+	return PASS;
+}
+
+/* this routine calculates the ONFI timing values for a given mode and
+ * programs the clocking register accordingly. The mode is determined by
+ * the get_onfi_nand_para routine.
+ */
+static void nand_onfi_timing_set(uint16_t mode)
+{
+	uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
+	uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
+	uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
+	uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
+	uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
+	uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
+	uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
+	uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
+	uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
+	uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
+
+	uint16_t TclsRising = 1;
+	uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
+	uint16_t dv_window = 0;
+	uint16_t en_lo, en_hi;
+	uint16_t acc_clks;
+	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
+
+	en_lo = CEIL_DIV(Trp[mode], CLK_X);
+	en_hi = CEIL_DIV(Treh[mode], CLK_X);
+#if ONFI_BLOOM_TIME
+	if ((en_hi * CLK_X) < (Treh[mode] + 2))
+		en_hi++;
+#endif
+
+	if ((en_lo + en_hi) * CLK_X < Trc[mode])
+		en_lo += CEIL_DIV((Trc[mode] - (en_lo + en_hi) * CLK_X), CLK_X);
+
+	if ((en_lo + en_hi) < CLK_MULTI)
+		en_lo += CLK_MULTI - en_lo - en_hi;
+
+	while (dv_window < 8) {
+		data_invalid_rhoh = en_lo * CLK_X + Trhoh[mode];
+
+		data_invalid_rloh = (en_lo + en_hi) * CLK_X + Trloh[mode];
+
+		data_invalid =
+		    data_invalid_rhoh <
+		    data_invalid_rloh ? data_invalid_rhoh : data_invalid_rloh;
+
+		dv_window = data_invalid - Trea[mode];
+
+		if (dv_window < 8)
+			en_lo++;
+	}
+
+	acc_clks = CEIL_DIV(Trea[mode], CLK_X);
+
+	while (((acc_clks * CLK_X) - Trea[mode]) < 3)
+		acc_clks++;
+
+	if ((data_invalid - acc_clks * CLK_X) < 2)
+		debug(KERN_WARNING "%s, Line %d: Warning!\n",
+			__FILE__, __LINE__);
+
+	addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
+	re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
+	re_2_re = CEIL_DIV(Trhz[mode], CLK_X);
+	we_2_re = CEIL_DIV(Twhr[mode], CLK_X);
+	cs_cnt = CEIL_DIV((Tcs[mode] - Trp[mode]), CLK_X);
+	if (!TclsRising)
+		cs_cnt = CEIL_DIV(Tcs[mode], CLK_X);
+	if (cs_cnt == 0)
+		cs_cnt = 1;
+
+	if (Tcea[mode]) {
+		while (((cs_cnt * CLK_X) + Trea[mode]) < Tcea[mode])
+			cs_cnt++;
+	}
+
+#if MODE5_WORKAROUND
+	if (mode == 5)
+		acc_clks = 5;
+#endif
+
+	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
+	if ((__raw_readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
+		(__raw_readl(denali.flash_reg + DEVICE_ID) == 0x88))
+		acc_clks = 6;
+
+	__raw_writel(acc_clks, denali.flash_reg + ACC_CLKS);
+	__raw_writel(re_2_we, denali.flash_reg + RE_2_WE);
+	__raw_writel(re_2_re, denali.flash_reg + RE_2_RE);
+	__raw_writel(we_2_re, denali.flash_reg + WE_2_RE);
+	__raw_writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
+	__raw_writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
+	__raw_writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
+	__raw_writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
+}
+

drivers/mtd/nand/denali_nand.c new file +8 -0

+/* determines how many NAND chips are connected to the controller. Note for
+ * Intel CE4100 devices we don't support more than one device.
+ */
+static void find_valid_banks(void)
+{
+	uint32_t id[denali.max_banks];
+	int i;
+

drivers/mtd/nand/denali_nand.c new file +12 -0

+static void detect_partition_feature(void)
+{
+	/* For MRST platform, denali.fwblks represent the
+	 * number of blocks firmware is taken,
+	 * FW is in protect partition and MTD driver has no
+	 * permission to access it. So let driver know how many
+	 * blocks it can't touch.
+	 * */
+	if (__raw_readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
+		if ((__raw_readl(denali.flash_reg + PERM_SRC_ID(1)) &
+			PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
+			denali.fwblks =

drivers/mtd/nand/denali_nand.c new file +23 -0

+/* raw include ECC value and all the spare area */
+static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				uint8_t *buf, int oob_required, int page)
+{
+	uint32_t irq_status, irq_mask = INTR_STATUS__DMA_CMD_COMP;
+
+	debug("denali_read_page_raw at page %08x\n", page);
+	if (denali.page != page) {
+		debug("Missing NAND_CMD_READ0 command\n");
+		return -EIO;
+	}
+
+	if (oob_required)
+		/* switch to main + spare access */
+		denali_mode_main_spare_access();
+	else
+		/* switch to main access only */
+		denali_mode_main_access();
+
+	/* setting up the DMA where ecc_enable is false */
+	irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
+						oob_required);
+

drivers/mtd/nand/denali_nand.c new file +13 -0

+static void denali_erase(struct mtd_info *mtd, int page)
+{
+	uint32_t cmd = 0x0, irq_status = 0;
+
+	debug("denali_erase at page %08x\n", page);
+
+	/* clear interrupts */
+	clear_interrupts();
+
+	/* setup page read request for access type */
+	cmd = MODE_10 | BANK(denali.flash_bank) | page;
+	index_addr((uint32_t)cmd, 0x1);
+

drivers/mtd/nand/denali_nand.c new file +12 -0

+/* stubs for ECC functions not used by the NAND core */
+static int denali_ecc_calculate(struct mtd_info *mtd, const uint8_t *data,
+				uint8_t *ecc_code)
+{
+	debug("Should not be called as ECC handled by hardware\n");
+	BUG();
+	return -EIO;
+}
+
+static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
+				uint8_t *read_ecc, uint8_t *calc_ecc)
+{

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */

drivers/mtd/nand/denali_nand.c line 80

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+static inline void clear_interrupt(uint32_t irq_mask)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
+}

Why are you using raw I/O accessors? The Linux driver doesn't do this.

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+
+static uint32_t wait_for_irq(uint32_t irq_mask)
+{
+	unsigned long comp_res = 1000;
+	uint32_t intr_status = 0;
+

drivers/mtd/nand/denali_nand.c line 118

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+	do {
+		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+		if (intr_status & irq_mask) {
+			denali.irq_status &= ~irq_mask;
+			/* our interrupt was detected */
+			break;
+		}
+		udelay(1);
+		comp_res--;
+	} while (comp_res != 0);

This looks like a much shorter timeout than Linux uses (1000us versus 1000ms). Though FWIW the Linux timeout code looks buggy.

Also, comp_res is a very odd name for a timeout variable.

Hello Scott, Chin,

Quoted 5 lines

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */

Quoted 8 lines

drivers/mtd/nand/denali_nand.c line 80

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+static inline void clear_interrupt(uint32_t irq_mask)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
+}

Why are you using raw I/O accessors? The Linux driver doesn't do this.

Add ioread32/iowrite32 to arch/arm/include/asm/io.h and use them?

There is another problem.
I think there is a cache coherency problem in this driver code. DMA is used in this driver but D-cache is never flushed.

When D-cache is on (CONFIG_SYS_DCACHE_OFF is not defined), ARM processor writes to/reads from the buffer through D-cache. On the other hand, Denali NAND controller always wites to/reads from the buffer on physical memory.
So, this driver writes/reads wrong data.

I had to add flush_dcache_range() function call
in denali_setup_dma_sequence().

Hello Scott, Chin,

Quoted 5 lines

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */

Quoted 8 lines

drivers/mtd/nand/denali_nand.c line 80

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+static inline void clear_interrupt(uint32_t irq_mask)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
+}

Why are you using raw I/O accessors? The Linux driver doesn't do this.

Add ioread32/iowrite32 to arch/arm/include/asm/io.h and use them?

Quoted 22 lines

Quoted 8 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+
+static uint32_t wait_for_irq(uint32_t irq_mask)
+{
+	unsigned long comp_res = 1000;
+	uint32_t intr_status = 0;
+

drivers/mtd/nand/denali_nand.c line 118

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +10 -0

+	do {
+		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+		if (intr_status & irq_mask) {
+			denali.irq_status &= ~irq_mask;
+			/* our interrupt was detected */
+			break;
+		}
+		udelay(1);
+		comp_res--;
+	} while (comp_res != 0);

This looks like a much shorter timeout than Linux uses (1000us versus 1000ms). Though FWIW the Linux timeout code looks buggy.

Quoted 1 line

Also, comp_res is a very odd name for a timeout variable.

Right. I think wait time is too short.
When I tested this code on my board, timeout error always occurred on page write command.

I had to fix it to run write command.

    s/udelay(1)/udelay(1000)/

There is another problem.
I think there is a cache coherency problem in this driver code. DMA is used in this driver but D-cache is never flushed.

When D-cache is on (CONFIG_SYS_DCACHE_OFF is not defined), ARM processor writes to/reads from the buffer through D-cache. On the other hand, Denali NAND controller always wites to/reads from the buffer on physical memory.
So, this driver writes/reads wrong data.

I had to add flush_dcache_range() function call
in denali_setup_dma_sequence().

  @@ -689,6 +689,8 @@ static void denali_setup_dma_sequence(int op)
          uint32_t mode;
          uint32_t addr = (uint32_t)denali.buf.dma_buf;

+ flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
+

          mode = MODE_10 | BANK(denali.flash_bank);
   
          /* DMA is a four step process */

Best Regards
Masahiro Yamada

Quoted 31 lines · Chin Liang See

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <[email protected]>

Cc Artem Bityutskiy <[email protected]>

Cc David Woodhouse <[email protected]>

Cc Brian Norris <[email protected]>

Cc Scott Wood <[email protected]>

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h

drivers/mtd/nand/Makefile +1 -0

index 02b149c..24e8218 100644
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
+obj-$(CONFIG_NAND_DENALI) += denali_nand.o
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o

drivers/mtd/nand/denali_nand.c

Quoted 6 lines

drivers/mtd/nand/Makefile +0 -0

 obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o

drivers/mtd/nand/denali_nand.c new file +0 -0

new file mode 100644
index 0000000..55246c9

It's "denali.c" in Linux -- why "denali_nand.c" here?

Quoted 14 lines

drivers/mtd/nand/denali_nand.c new file +12 -0

@@ -0,0 +1,1166 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>
+ * Copyright (C) 2009-2010, Intel Corporation and its suppliers.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/errno.h>
+#include <asm/io.h>
+

drivers/mtd/nand/denali_nand.c line 18

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+#include "denali_nand.h"
+
+/* We define a module parameter that allows the user to override
+ * the hardware and decide what timing mode should be used.
+ */
+#define NAND_DEFAULT_TIMINGS	-1

A module parameter? In U-Boot?

Sharing code with Linux is fine, but try to edit out the stuff that's irrelevant in U-Boot.

Quoted 55 lines

drivers/mtd/nand/denali_nand.c new file +55 -0

+static struct denali_nand_info denali;
+static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
+
+/* We define a macro here that combines all interrupts this driver uses into
+ * a single constant value, for convenience. */
+#define DENALI_IRQ_ALL	(INTR_STATUS__DMA_CMD_COMP | \
+			INTR_STATUS__ECC_TRANSACTION_DONE | \
+			INTR_STATUS__ECC_ERR | \
+			INTR_STATUS__PROGRAM_FAIL | \
+			INTR_STATUS__LOAD_COMP | \
+			INTR_STATUS__PROGRAM_COMP | \
+			INTR_STATUS__TIME_OUT | \
+			INTR_STATUS__ERASE_FAIL | \
+			INTR_STATUS__RST_COMP | \
+			INTR_STATUS__ERASE_COMP | \
+			INTR_STATUS__ECC_UNCOR_ERR | \
+			INTR_STATUS__INT_ACT | \
+			INTR_STATUS__LOCKED_BLK)
+
+/* indicates whether or not the internal value for the flash bank is
+ * valid or not */
+#define CHIP_SELECT_INVALID	-1
+
+#define SUPPORT_8BITECC		1
+
+/* This macro divides two integers and rounds fractional values up
+ * to the nearest integer value. */
+#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
+
+/* These constants are defined by the driver to enable common driver
+ * configuration options. */
+#define SPARE_ACCESS		0x41
+#define MAIN_ACCESS		0x42
+#define MAIN_SPARE_ACCESS	0x43
+
+#define DENALI_UNLOCK_START	0x10
+#define DENALI_UNLOCK_END	0x11
+#define DENALI_LOCK		0x21
+#define DENALI_LOCK_TIGHT	0x31
+#define DENALI_BUFFER_LOAD	0x60
+#define DENALI_BUFFER_WRITE	0x62
+
+#define DENALI_READ	0
+#define DENALI_WRITE	0x100
+
+/* types of device accesses. We can issue commands and get status */
+#define COMMAND_CYCLE	0
+#define ADDR_CYCLE	1
+#define STATUS_CYCLE	2
+
+/* this is a helper macro that allows us to
+ * format the bank into the proper bits for the controller */
+#define BANK(x) ((x) << 24)
+
+/* Interrupts are cleared by writing a 1 to the appropriate status bit */

drivers/mtd/nand/denali_nand.c line 80

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+static inline void clear_interrupt(uint32_t irq_mask)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	__raw_writel(irq_mask, denali.flash_reg + intr_status_reg);
+}

Why are you using raw I/O accessors? The Linux driver doesn't do this.

Quoted 33 lines

drivers/mtd/nand/denali_nand.c new file +32 -0

+static uint32_t read_interrupt_status(void)
+{
+	uint32_t intr_status_reg = 0;
+	intr_status_reg = INTR_STATUS(denali.flash_bank);
+	return __raw_readl(denali.flash_reg + intr_status_reg);
+}
+
+static void clear_interrupts(void)
+{
+	uint32_t status = 0x0;
+	status = read_interrupt_status();
+	clear_interrupt(status);
+	denali.irq_status = 0x0;
+}
+
+static void denali_irq_enable(uint32_t int_mask)
+{
+	int i;
+	for (i = 0; i < denali.max_banks; ++i)
+		__raw_writel(int_mask, denali.flash_reg + INTR_EN(i));
+}
+
+static uint32_t wait_for_irq(uint32_t irq_mask)
+{
+	unsigned long comp_res = 1000;
+	uint32_t intr_status = 0;
+
+	do {
+		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+		if (intr_status & irq_mask) {
+			denali.irq_status &= ~irq_mask;
+			/* our interrupt was detected */

drivers/mtd/nand/denali_nand.c line 118

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+			break;
+		}
+		udelay(1);
+		comp_res--;
+	} while (comp_res != 0);

This looks like a much shorter timeout than Linux uses (1000us versus 1000ms). Though FWIW the Linux timeout code looks buggy.

Also, comp_res is a very odd name for a timeout variable.

drivers/mtd/nand/denali_nand.c line 139

Quoted 12 lines

drivers/mtd/nand/denali_nand.c new file +11 -0

+/* Certain operations for the denali NAND controller use
+ * an indexed mode to read/write data. The operation is
+ * performed by writing the address value of the command
+ * to the device memory followed by the data. This function
+ * abstracts this common operation.
+*/
+static void index_addr(uint32_t address, uint32_t data)
+{
+	__raw_writel(address, denali.flash_mem);
+	__raw_writel(data, denali.flash_mem + 0x10);
+}

What is 0x10? No magic numbers, please.

Quoted 166 lines

drivers/mtd/nand/denali_nand.c new file +166 -0

+
+/* Perform an indexed read of the device */
+static void index_addr_read_data(uint32_t address, uint32_t *pdata)
+{
+	__raw_writel(address, denali.flash_mem);
+	*pdata = __raw_readl(denali.flash_mem + 0x10);
+}
+
+/* We need to buffer some data for some of the NAND core routines.
+ * The operations manage buffering that data. */
+static void reset_buf(void)
+{
+	denali.buf.head = denali.buf.tail = 0;
+}
+
+static void write_byte_to_buf(uint8_t byte)
+{
+	BUG_ON(denali.buf.tail >= sizeof(denali.buf.buf));
+	denali.buf.buf[denali.buf.tail++] = byte;
+}
+
+/* resets a specific device connected to the core */
+static void reset_bank(void)
+{
+	uint32_t irq_status = 0;
+	uint32_t irq_mask = INTR_STATUS__RST_COMP |
+			    INTR_STATUS__TIME_OUT;
+
+	clear_interrupts();
+
+	__raw_writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
+
+	irq_status = wait_for_irq(irq_mask);
+	if (irq_status & INTR_STATUS__TIME_OUT)
+		debug(KERN_ERR "reset bank failed.\n");
+}
+
+/* Reset the flash controller */
+static uint16_t denali_nand_reset(void)
+{
+	uint32_t i;
+
+	for (i = 0 ; i < denali.max_banks; i++)
+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+		denali.flash_reg + INTR_STATUS(i));
+
+	for (i = 0 ; i < denali.max_banks; i++) {
+		__raw_writel(1 << i, denali.flash_reg + DEVICE_RESET);
+		while (!(__raw_readl(denali.flash_reg +	INTR_STATUS(i)) &
+			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
+			if (__raw_readl(denali.flash_reg + INTR_STATUS(i)) &
+				INTR_STATUS__TIME_OUT)
+				debug(KERN_DEBUG "NAND Reset operation "
+					"timed out on bank %d\n", i);
+	}
+
+	for (i = 0; i < denali.max_banks; i++)
+		__raw_writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
+			denali.flash_reg + INTR_STATUS(i));
+
+	return PASS;
+}
+
+/* this routine calculates the ONFI timing values for a given mode and
+ * programs the clocking register accordingly. The mode is determined by
+ * the get_onfi_nand_para routine.
+ */
+static void nand_onfi_timing_set(uint16_t mode)
+{
+	uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
+	uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
+	uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
+	uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
+	uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
+	uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
+	uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
+	uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
+	uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
+	uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
+	uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
+
+	uint16_t TclsRising = 1;
+	uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
+	uint16_t dv_window = 0;
+	uint16_t en_lo, en_hi;
+	uint16_t acc_clks;
+	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
+
+	en_lo = CEIL_DIV(Trp[mode], CLK_X);
+	en_hi = CEIL_DIV(Treh[mode], CLK_X);
+#if ONFI_BLOOM_TIME
+	if ((en_hi * CLK_X) < (Treh[mode] + 2))
+		en_hi++;
+#endif
+
+	if ((en_lo + en_hi) * CLK_X < Trc[mode])
+		en_lo += CEIL_DIV((Trc[mode] - (en_lo + en_hi) * CLK_X), CLK_X);
+
+	if ((en_lo + en_hi) < CLK_MULTI)
+		en_lo += CLK_MULTI - en_lo - en_hi;
+
+	while (dv_window < 8) {
+		data_invalid_rhoh = en_lo * CLK_X + Trhoh[mode];
+
+		data_invalid_rloh = (en_lo + en_hi) * CLK_X + Trloh[mode];
+
+		data_invalid =
+		    data_invalid_rhoh <
+		    data_invalid_rloh ? data_invalid_rhoh : data_invalid_rloh;
+
+		dv_window = data_invalid - Trea[mode];
+
+		if (dv_window < 8)
+			en_lo++;
+	}
+
+	acc_clks = CEIL_DIV(Trea[mode], CLK_X);
+
+	while (((acc_clks * CLK_X) - Trea[mode]) < 3)
+		acc_clks++;
+
+	if ((data_invalid - acc_clks * CLK_X) < 2)
+		debug(KERN_WARNING "%s, Line %d: Warning!\n",
+			__FILE__, __LINE__);
+
+	addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
+	re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
+	re_2_re = CEIL_DIV(Trhz[mode], CLK_X);
+	we_2_re = CEIL_DIV(Twhr[mode], CLK_X);
+	cs_cnt = CEIL_DIV((Tcs[mode] - Trp[mode]), CLK_X);
+	if (!TclsRising)
+		cs_cnt = CEIL_DIV(Tcs[mode], CLK_X);
+	if (cs_cnt == 0)
+		cs_cnt = 1;
+
+	if (Tcea[mode]) {
+		while (((cs_cnt * CLK_X) + Trea[mode]) < Tcea[mode])
+			cs_cnt++;
+	}
+
+#if MODE5_WORKAROUND
+	if (mode == 5)
+		acc_clks = 5;
+#endif
+
+	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
+	if ((__raw_readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
+		(__raw_readl(denali.flash_reg + DEVICE_ID) == 0x88))
+		acc_clks = 6;
+
+	__raw_writel(acc_clks, denali.flash_reg + ACC_CLKS);
+	__raw_writel(re_2_we, denali.flash_reg + RE_2_WE);
+	__raw_writel(re_2_re, denali.flash_reg + RE_2_RE);
+	__raw_writel(we_2_re, denali.flash_reg + WE_2_RE);
+	__raw_writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
+	__raw_writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
+	__raw_writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
+	__raw_writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
+}
+
+/* queries the NAND device to see what ONFI modes it supports. */
+static uint16_t get_onfi_nand_para(void)
+{
+	int i;
+	/* we needn't to do a reset here because driver has already

drivers/mtd/nand/denali_nand.c line 310

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+	 * reset all the banks before
+	 * */
+	if (!(__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
+		ONFI_TIMING_MODE__VALUE))
+		return FAIL;

Don't align continuation lines with an if/for body -- it makes it harder to see where one ends and the other begins.

Why PASS/FAIL rather than normal "0 on success, negative error code on error"? Why uint16_t?

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+	for (i = 5; i > 0; i--) {
+		if (__raw_readl(denali.flash_reg + ONFI_TIMING_MODE) &
+			(0x01 << i))
+			break;
+	}

drivers/mtd/nand/denali_nand.c line 323

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+	nand_onfi_timing_set(i);
+
+	/* By now, all the ONFI devices we know support the page cache */
+	/* rw feature. So here we enable the pipeline_rw_ahead feature */
+	/* __raw_writel(1, denali.flash_reg + CACHE_WRITE_ENABLE); */
+	/* __raw_writel(1, denali.flash_reg + CACHE_READ_ENABLE);  */

Don't add commented-out-code.

Quoted 13 lines

drivers/mtd/nand/denali_nand.c new file +12 -0

+/* determines how many NAND chips are connected to the controller. Note for
+ * Intel CE4100 devices we don't support more than one device.
+ */
+static void find_valid_banks(void)
+{
+	uint32_t id[denali.max_banks];
+	int i;
+
+	denali.total_used_banks = 1;
+	for (i = 0; i < denali.max_banks; i++) {
+		index_addr((uint32_t)(MODE_11 | (i << 24) | 0), 0x90);
+		index_addr((uint32_t)(MODE_11 | (i << 24) | 1), 0);

drivers/mtd/nand/denali_nand.c line 414

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+		index_addr_read_data((uint32_t)(MODE_11 | (i << 24) | 2),
+			&id[i]);
+
+		if (i == 0) {
+			if (!(id[i] & 0x0ff))
+				break; /* WTF? */

I'm not sure that this is the most helpful comment that could go here.

Quoted 16 lines

drivers/mtd/nand/denali_nand.c new file +16 -0

+static void detect_partition_feature(void)
+{
+	/* For MRST platform, denali.fwblks represent the
+	 * number of blocks firmware is taken,
+	 * FW is in protect partition and MTD driver has no
+	 * permission to access it. So let driver know how many
+	 * blocks it can't touch.
+	 * */
+	if (__raw_readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
+		if ((__raw_readl(denali.flash_reg + PERM_SRC_ID(1)) &
+			PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
+			denali.fwblks =
+			    ((__raw_readl(denali.flash_reg + MIN_MAX_BANK(1)) &
+			      MIN_MAX_BANK__MIN_VALUE) *
+			     denali.blksperchip)
+			    +

drivers/mtd/nand/denali_nand.c line 455

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+			    (__raw_readl(denali.flash_reg + MIN_BLK_ADDR(1)) &
+			    MIN_BLK_ADDR__VALUE);
+		} else
+			denali.fwblks = SPECTRA_START_BLOCK;
+	} else
+		denali.fwblks = SPECTRA_START_BLOCK;

If braces are needed on one side of if/else, use on both sides.

drivers/mtd/nand/denali_nand.c line 508

Quoted 8 lines

drivers/mtd/nand/denali_nand.c new file +7 -0

+static void denali_set_intr_modes(uint16_t INT_ENABLE)
+{
+	if (INT_ENABLE)
+		__raw_writel(1, denali.flash_reg + GLOBAL_INT_ENABLE);
+	else
+		__raw_writel(0, denali.flash_reg + GLOBAL_INT_ENABLE);
+}

CAPS are for macros, not function parameters.

Why not just require the caller to pass in 0/1 rather than 0/non-0 and write that directly to the register?

drivers/mtd/nand/denali_nand.c line 603

Quoted 4 lines

drivers/mtd/nand/denali_nand.c new file +3 -0

+/* helper function that simply writes a buffer to the flash */
+static int write_data_to_flash_mem(const uint8_t *buf,
+							int len)

There's no point in a continuation line if you indent so far that you're beyond where the previous line ended. If the above doesn't violate 80 columns, then you could just as well do it on one line.

drivers/mtd/nand/denali_nand.c line 614

Quoted 11 lines

drivers/mtd/nand/denali_nand.c new file +11 -0

+{
+	uint32_t i = 0, *buf32;
+
+	/* verify that the len is a multiple of 4. see comment in
+	 * read_data_from_flash_mem() */
+	BUG_ON((len % 4) != 0);
+
+	/* write the data to the flash memory */
+	buf32 = (uint32_t *)buf;
+	for (i = 0; i < len / 4; i++)
+		__raw_writel(*buf32++, denali.flash_mem + 0x10);

This violates C99 aliasing rules.

drivers/mtd/nand/denali_nand.c line 623

Quoted 10 lines

drivers/mtd/nand/denali_nand.c new file +9 -0

+	return i*4; /* intent is to return the number of bytes read */
+}
+
+static void denali_mode_main_access(void)
+{
+	uint32_t addr, cmd;
+	addr = BANK(denali.flash_bank) | denali.page;
+	cmd = MODE_10 | addr;
+	index_addr((uint32_t)cmd, MAIN_ACCESS);

Unnecessary cast.

Quoted 28 lines

drivers/mtd/nand/denali_nand.c new file +27 -0

+/* raw include ECC value and all the spare area */
+static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				uint8_t *buf, int oob_required, int page)
+{
+	uint32_t irq_status, irq_mask = INTR_STATUS__DMA_CMD_COMP;
+
+	debug("denali_read_page_raw at page %08x\n", page);
+	if (denali.page != page) {
+		debug("Missing NAND_CMD_READ0 command\n");
+		return -EIO;
+	}
+
+	if (oob_required)
+		/* switch to main + spare access */
+		denali_mode_main_spare_access();
+	else
+		/* switch to main access only */
+		denali_mode_main_access();
+
+	/* setting up the DMA where ecc_enable is false */
+	irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
+						oob_required);
+
+	/* if timeout happen, error out */
+	if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
+		debug("DMA timeout for denali_read_page_raw\n");
+		return -EIO;

drivers/mtd/nand/denali_nand.c line 863

Quoted 6 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+	}
+
+	/* splitting the content to destination buffer holder */
+	memcpy(chip->oob_poi, (const void *)(denali.buf.dma_buf +
+		mtd->writesize), mtd->oobsize);
+	memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);

Unnecessary casts.

Quoted 19 lines

drivers/mtd/nand/denali_nand.c new file +18 -0

+static void denali_erase(struct mtd_info *mtd, int page)
+{
+	uint32_t cmd = 0x0, irq_status = 0;
+
+	debug("denali_erase at page %08x\n", page);
+
+	/* clear interrupts */
+	clear_interrupts();
+
+	/* setup page read request for access type */
+	cmd = MODE_10 | BANK(denali.flash_bank) | page;
+	index_addr((uint32_t)cmd, 0x1);
+
+	/* wait for erase to complete or failure to occur */
+	irq_status = wait_for_irq(INTR_STATUS__ERASE_COMP |
+		INTR_STATUS__ERASE_FAIL);
+
+	if (irq_status & INTR_STATUS__ERASE_FAIL ||

drivers/mtd/nand/denali_nand.c line 981

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+		irq_status & INTR_STATUS__LOCKED_BLK)
+		denali.status = NAND_STATUS_FAIL;
+	else
+		denali.status = PASS;
+}

So PASS/FAIL are supposed to be in the same numberspace as NAND_STATUS_foo? Why use a separate FAIL elsewhere, then?

drivers/mtd/nand/denali_nand.c line 993

Quoted 12 lines

drivers/mtd/nand/denali_nand.c new file +11 -0

+static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
+			   int page)
+{
+	uint32_t addr;
+
+	switch (cmd) {
+	case NAND_CMD_PAGEPROG:
+		break;
+	case NAND_CMD_STATUS:
+		addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+		index_addr((uint32_t)addr | 0, cmd);

More unnecessary casts...

Quoted 18 lines

drivers/mtd/nand/denali_nand.c new file +17 -0

+/* stubs for ECC functions not used by the NAND core */
+static int denali_ecc_calculate(struct mtd_info *mtd, const uint8_t *data,
+				uint8_t *ecc_code)
+{
+	debug("Should not be called as ECC handled by hardware\n");
+	BUG();
+	return -EIO;
+}
+
+static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
+				uint8_t *read_ecc, uint8_t *calc_ecc)
+{
+	debug("Should not be called as ECC handled by hardware\n");
+	BUG();
+	return -EIO;
+}
+

drivers/mtd/nand/denali_nand.c line 1071

Quoted 5 lines

drivers/mtd/nand/denali_nand.c new file +5 -0

+static void denali_ecc_hwctl(struct mtd_info *mtd, int mode)
+{
+	debug("Should not be called as ECC handled by hardware\n");
+	BUG();
+}

These debug() messages seem superfluous given the BUG() will identify the line in the code. debug() is not for commenting code.

drivers/mtd/nand/denali_nand.c line 474

Quoted 7 lines

drivers/mtd/nand/denali_nand.c new file +6 -0

+/* ffsdefs.h */
+#define CLEAR 0                 /*use this to clear a field instead of "fail"*/
+#define SET   1                 /*use this to set a field instead of "pass"*/
+#define FAIL 1                  /*failed flag*/
+#define PASS 0                  /*success flag*/
+#define ERR -1                  /*error flag*/

What's the difference between FAIL and ERR?

-Scott

Quoted 32 lines · Scott Wood

Quoted 31 lines · Chin Liang See

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <[email protected]>

Cc Artem Bityutskiy <[email protected]>

Cc David Woodhouse <[email protected]>

Cc Brian Norris <[email protected]>

Cc Scott Wood <[email protected]>

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h

drivers/mtd/nand/Makefile +1 -0

index 02b149c..24e8218 100644
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
+obj-$(CONFIG_NAND_DENALI) += denali_nand.o
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o

Quoted 8 lines

drivers/mtd/nand/denali_nand.c

Quoted 6 lines

drivers/mtd/nand/Makefile +0 -0

 obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o

drivers/mtd/nand/denali_nand.c new file +0 -0

new file mode 100644
index 0000000..55246c9

It's "denali.c" in Linux -- why "denali_nand.c" here?

It seems all the existing U-Boot nand driver is using this naming standard where <platform>_nand.

Quoted 3 lines

Quoted 1 line

Why PASS/FAIL rather than normal "0 on success, negative error code on

error"? Why uint16_t?

Fixed by returning 0 when pass. Also changed uint16_t to uint32_t

Quoted 44 lines · Chin Liang See

Quoted 40 lines · Scott Wood

drivers/mtd/nand/denali_nand.c

Quoted 37 lines · Chin Liang See

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <[email protected]>

Cc Artem Bityutskiy <[email protected]>

Cc David Woodhouse <[email protected]>

Cc Brian Norris <[email protected]>

Cc Scott Wood <[email protected]>

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h

drivers/mtd/nand/Makefile +1 -0

index 02b149c..24e8218 100644
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
+obj-$(CONFIG_NAND_DENALI) += denali_nand.o
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o
 obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o

drivers/mtd/nand/denali_nand.c new file +0 -0

new file mode 100644
index 0000000..55246c9

It's "denali.c" in Linux -- why "denali_nand.c" here?

Quoted 2 lines

It seems all the existing U-Boot nand driver is using this naming standard where <platform>_nand.

Not all -- there's omap_gpmc.c, omap_elm.c, nomadik.c, ndfc.c, etc.

A lot of them have the _nand.c suffix in Linux, too. Personally, I think it's redundant.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Why PASS/FAIL rather than normal "0 on success, negative error code on

error"? Why uint16_t?

Fixed by returning 0 when pass. Also changed uint16_t to uint32_t

Why uint32_t and not int? Is that return value somewhere used in a context that expects a NAND hardware status?

-Scott

Quoted 8 lines · Scott Wood

Quoted 7 lines · Chin Liang See

Quoted 4 lines · Scott Wood

Quoted 1 line · Chin Liang See

Why PASS/FAIL rather than normal "0 on success, negative error code on

error"? Why uint16_t?

Quoted 5 lines

Quoted 1 line

Fixed by returning 0 when pass. Also changed uint16_t to uint32_t

Why uint32_t and not int? Is that return value somewhere used in a context that expects a NAND hardware status?

Nope, the return value is not used to compare against > 0 or < 0

Quoted 11 lines · Chin Liang See

Quoted 10 lines · Scott Wood

Quoted 8 lines · Chin Liang See

Quoted 4 lines · Scott Wood

Quoted 1 line · Chin Liang See

Why PASS/FAIL rather than normal "0 on success, negative error code on

error"? Why uint16_t?

Fixed by returning 0 when pass. Also changed uint16_t to uint32_t

Quoted 6 lines

Quoted 3 lines

Why uint32_t and not int? Is that return value somewhere used in a context that expects a NAND hardware status?

Nope, the return value is not used to compare against > 0 or < 0

Why not?

-Scott

Quoted 17 lines · Scott Wood

Quoted 16 lines · Chin Liang See

Quoted 13 lines · Scott Wood

Quoted 8 lines · Chin Liang See

Quoted 4 lines · Scott Wood

Quoted 1 line · Chin Liang See

Why PASS/FAIL rather than normal "0 on success, negative error code on

error"? Why uint16_t?

Fixed by returning 0 when pass. Also changed uint16_t to uint32_t

Why uint32_t and not int? Is that return value somewhere used in a context that expects a NAND hardware status?

Quoted 4 lines

Quoted 1 line

Nope, the return value is not used to compare against > 0 or < 0

Why not?

We just check whether 0 or not as success will return 0.

Quoted 23 lines · Chin Liang See

Quoted 21 lines · Scott Wood

Quoted 9 lines · Chin Liang See

Quoted 8 lines · Scott Wood

Quoted 7 lines · Chin Liang See

Quoted 4 lines · Scott Wood

Quoted 1 line · Chin Liang See

Why PASS/FAIL rather than normal "0 on success, negative error code on

error"? Why uint16_t?

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Fixed by returning 0 when pass. Also changed uint16_t to uint32_t

Why uint32_t and not int? Is that return value somewhere used in a context that expects a NAND hardware status?

Nope, the return value is not used to compare against > 0 or < 0

Why not?

Quoted 1 line

We just check whether 0 or not as success will return 0.

The standard error idiom in Linux and U-Boot is negative values for errors. That's why I asked if there was a reason for this, such as passing the value to something that expects status values as would be returned by hardware.

-Scott

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <clsee <at> altera.com>

Cc Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Cc David Woodhouse <David.Woodhouse <at> intel.com>

Cc Brian Norris <computersforpeace <at> gmail.com>

Cc Scott Wood <scottwood <at> freescale.com>

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166

 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h

Quoted 10 lines · Chin Liang See <clsee <at> altera.com> writes:

To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.

Signed-off-by Chin Liang See <clsee <at> altera.com>

Cc Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Cc David Woodhouse <David.Woodhouse <at> intel.com>

Cc Brian Norris <computersforpeace <at> gmail.com>

Cc Scott Wood <scottwood <at> freescale.com>

Quoted 6 lines

---
Changes for v2
- Enable this driver support for SOCFPGA

---
 drivers/mtd/nand/Makefile      |    1 +
 drivers/mtd/nand/denali_nand.c | 1166

++++++++++++++++++++++++++++++++++++++++

Quoted 5 lines

 drivers/mtd/nand/denali_nand.h |  501 +++++++++++++++++
 3 files changed, 1668 insertions(+)
 create mode 100644 drivers/mtd/nand/denali_nand.c
 create mode 100644 drivers/mtd/nand/denali_nand.h