All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
@ 2014-04-18 11:30 Masahiro Yamada
  2014-04-18 11:30 ` [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel Masahiro Yamada
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Masahiro Yamada @ 2014-04-18 11:30 UTC (permalink / raw)
  To: u-boot

This patch is a review feedback against Denali NAND controller driver.
http://patchwork.ozlabs.org/patch/333077/

This is not applicable to the mainline.

  ---

Hi Chin,

This patch fixes some issues.

[1] Fix denali_write_oob() handler.

As for v7, "nand markbad" did not work at all.

With this patch, it works.

[2] Make denali_read_oob()  10x faster.

One of the fatal issues of v7 is "nand bad" command is extremely slow.

This is the benchmark of v7

  => time nand bad

  Device 0 bad blocks:

  time: 11.300 seconds, 11300 ticks

It is really really painful to wait more than 10 seconds just for bad block
scanning to boot Linux.

In v7, denali_read_oob() calls denali_read_page_raw().
This causes the transfering main area data and memcpy of it,
which leads to significant performance regression.

Like Linux Kernel, dedicated denali_read_oob() must be impilemented.

With this patch, "nand bad" command gets much faster!

This is my benchmark:

  => time nand bad

  Device 0 bad blocks:

  time: 0.998 seconds, 998 ticks

[3] Remove false comment

 /* Writes OOB data to the device.
  * This code unused under normal U-Boot console as normally page write raw
  * to be used for write oob data with main data.
  */
  static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)

This comment is telling a lie.
write_oob_data() is called from "nand markbad" command.
It must be deleted.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Chin Liang See <clsee@altera.com>
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
---
 drivers/mtd/nand/denali.c | 136 +++++++++++++++++++++++++++++++---------------
 1 file changed, 91 insertions(+), 45 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 348e244..dcde3e6 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -527,46 +527,34 @@ static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
 static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
 					int access_type, int op)
 {
-	uint32_t addr = 0x0, cmd = 0x0, irq_status = 0,	 irq_mask = 0;
-	uint32_t page_count = 1;	/* always read a page */
-
-	if (op == DENALI_READ)
-		irq_mask = INTR_STATUS__LOAD_COMP;
-	else if (op == DENALI_WRITE)
-		irq_mask = INTR_STATUS__PROGRAM_COMP |
-			INTR_STATUS__PROGRAM_FAIL;
-	else
-		BUG();
+	uint32_t addr = 0x0, cmd = 0x0, irq_status;
+	static uint32_t page_count = 1;
+
+	setup_ecc_for_xfer(ecc_en, transfer_spare);
 
 	/* clear interrupts */
 	clear_interrupts();
 
-	/* setup ECC and transfer spare reg */
-	setup_ecc_for_xfer(ecc_en, transfer_spare);
-
 	addr = BANK(denali.flash_bank) | denali.page;
 
 	/* setup the acccess type */
 	cmd = MODE_10 | addr;
-	index_addr((uint32_t)cmd, access_type);
+	index_addr(cmd, access_type);
 
 	/* setup the pipeline command */
-	if (access_type == SPARE_ACCESS && op == DENALI_WRITE)
-		index_addr((uint32_t)cmd, DENALI_BUFFER_WRITE);
-	else if (access_type == SPARE_ACCESS && op == DENALI_READ)
-		index_addr((uint32_t)cmd, DENALI_BUFFER_LOAD);
-	else
-		index_addr((uint32_t)cmd, 0x2000 | op | page_count);
+	index_addr(cmd, 0x2000 | op | page_count);
 
-	/* wait for command to be accepted */
-	irq_status = wait_for_irq(irq_mask);
-	if ((irq_status & irq_mask) != irq_mask)
-		return -EIO;
+	cmd = MODE_01 | addr;
+	writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
 
-	if (access_type != SPARE_ACCESS) {
-		cmd = MODE_01 | addr;
-		writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
+	if (op == DENALI_READ) {
+		/* wait for command to be accepted */
+		irq_status = wait_for_irq(INTR_STATUS__LOAD_COMP);
+
+		if (irq_status == 0)
+			return -EIO;
 	}
+
 	return 0;
 }
 
@@ -586,6 +574,29 @@ static int write_data_to_flash_mem(const void *buf, int len)
 	return i * 4; /* intent is to return the number of bytes read */
 }
 
+/* helper function that simply reads a buffer from the flash */
+static int read_data_from_flash_mem(uint8_t *buf, int len)
+{
+	uint32_t i, *buf32;
+
+	/*
+	 * we assume that len will be a multiple of 4, if not
+	 * it would be nice to know about it ASAP rather than
+	 * have random failures...
+	 * This assumption is based on the fact that this
+	 * function is designed to be used to read flash pages,
+	 * which are typically multiples of 4...
+	 */
+
+	BUG_ON((len % 4) != 0);
+
+	/* transfer the data from the flash */
+	buf32 = (uint32_t *)buf;
+	for (i = 0; i < len / 4; i++)
+		*buf32++ = readl(denali.flash_mem + INDEX_DATA_REG);
+	return i * 4; /* intent is to return the number of bytes read */
+}
+
 static void denali_mode_main_access(void)
 {
 	uint32_t addr, cmd;
@@ -602,29 +613,64 @@ static void denali_mode_main_spare_access(void)
 	index_addr(cmd, MAIN_SPARE_ACCESS);
 }
 
-/* Writes OOB data to the device.
- * This code unused under normal U-Boot console as normally page write raw
- * to be used for write oob data with main data.
- */
+/* writes OOB data to the device */
 static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 {
-	uint32_t cmd;
+	uint32_t irq_status;
+	uint32_t irq_mask = INTR_STATUS__PROGRAM_COMP |
+						INTR_STATUS__PROGRAM_FAIL;
+	int status = 0;
 
 	denali.page = page;
-	debug("* write_oob_data *\n");
 
-	/* We need to write to buffer first through MAP00 command */
-	cmd = MODE_00 | BANK(denali.flash_bank);
-	writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
+	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
+							DENALI_WRITE) == 0) {
+		write_data_to_flash_mem(buf, mtd->oobsize);
 
-	/* send the data into flash buffer */
-	write_data_to_flash_mem(buf, mtd->oobsize);
+		/* wait for operation to complete */
+		irq_status = wait_for_irq(irq_mask);
 
-	/* activate the write through MAP10 commands */
-	if (denali_send_pipeline_cmd(false, false, SPARE_ACCESS, DENALI_WRITE))
-		return -EIO;
+		if (irq_status == 0) {
+			dev_err(denali->dev, "OOB write failed\n");
+			status = -EIO;
+		}
+	} else {
+		printf("unable to send pipeline command\n");
+		status = -EIO;
+	}
+	return status;
+}
 
-	return 0;
+/* reads OOB data from the device */
+static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
+{
+	uint32_t irq_mask = INTR_STATUS__LOAD_COMP,
+			 irq_status = 0, addr = 0x0, cmd = 0x0;
+
+	denali.page = page;
+
+	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
+							DENALI_READ) == 0) {
+		read_data_from_flash_mem(buf, mtd->oobsize);
+
+		/* wait for command to be accepted
+		 * can always use status0 bit as the mask is identical for each
+		 * bank. */
+		irq_status = wait_for_irq(irq_mask);
+
+		if (irq_status == 0)
+			printf("page on OOB timeout %d\n", denali.page);
+
+		/* We set the device back to MAIN_ACCESS here as I observed
+		 * instability with the controller if you do a block erase
+		 * and the last transaction was a SPARE_ACCESS. Block erase
+		 * is reliable (according to the MTD test infrastructure)
+		 * if you are in MAIN_ACCESS.
+		 */
+		addr = BANK(denali.flash_bank) | denali.page;
+		cmd = MODE_10 | addr;
+		index_addr(cmd, MAIN_ACCESS);
+	}
 }
 
 /* this function examines buffers to see if they contain data that
@@ -908,9 +954,9 @@ static uint8_t denali_read_byte(struct mtd_info *mtd)
 static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 				int page)
 {
-	debug("denali_read_oob at page %08x\n", page);
-	denali.page = page;
-	return denali_read_page_raw(mtd, chip, denali.buf.buf, 1, page);
+	read_oob_data(mtd, chip->oob_poi, page);
+
+	return 0;
 }
 
 static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel
  2014-04-18 11:30 [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Masahiro Yamada
@ 2014-04-18 11:30 ` Masahiro Yamada
  2014-04-23  7:02   ` Chin Liang See
  2014-04-21 22:21 ` [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Scott Wood
  2014-04-23  6:50 ` Chin Liang See
  2 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2014-04-18 11:30 UTC (permalink / raw)
  To: u-boot

This patch is a review feedback against Denali NAND controller driver.

http://patchwork.ozlabs.org/patch/333077/

This is not applicable to the mainline.

 --

This driver code has diverged too much from that of Linux Kernel.

The main cause was to drop "struct denali_nand_info *denali"
from the most of functions and to replace "denali->foo" with "denali.foo".

But is it necessary?
I think it just resulted in the difficulty of diffing and re-use of
the Linux code.

This patch revives "struct denali_nand_info *denali" and "denali->foo".

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Chin Liang See <clsee@altera.com>
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Scott Wood <scottwood@freescale.com>
---
 drivers/mtd/nand/denali.c | 608 +++++++++++++++++++++++++---------------------
 1 file changed, 327 insertions(+), 281 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index dcde3e6..565ca9e 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <malloc.h>
 #include <nand.h>
 #include <asm/errno.h>
 #include <asm/io.h>
@@ -15,7 +16,6 @@
 
 #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
@@ -40,6 +40,12 @@ static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
 
 #define SUPPORT_8BITECC		1
 
+/*
+ * this macro allows us to convert from an MTD structure to our own
+ * device context (denali) structure.
+ */
+#define mtd_to_denali(m) (((struct nand_chip *)mtd->priv)->priv)
+
 /* These constants are defined by the driver to enable common driver
  * configuration options. */
 #define SPARE_ACCESS		0x41
@@ -66,44 +72,53 @@ static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
 #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)
+static inline void clear_interrupt(struct denali_nand_info *denali,
+							uint32_t irq_mask)
 {
-	uint32_t intr_status_reg = 0;
-	intr_status_reg = INTR_STATUS(denali.flash_bank);
-	writel(irq_mask, denali.flash_reg + intr_status_reg);
+	uint32_t intr_status_reg;
+
+	intr_status_reg = INTR_STATUS(denali->flash_bank);
+
+	writel(irq_mask, denali->flash_reg + intr_status_reg);
 }
 
-static uint32_t read_interrupt_status(void)
+static uint32_t read_interrupt_status(struct denali_nand_info *denali)
 {
-	uint32_t intr_status_reg = 0;
-	intr_status_reg = INTR_STATUS(denali.flash_bank);
-	return readl(denali.flash_reg + intr_status_reg);
+	uint32_t intr_status_reg;
+
+	intr_status_reg = INTR_STATUS(denali->flash_bank);
+
+	return readl(denali->flash_reg + intr_status_reg);
 }
 
-static void clear_interrupts(void)
+static void clear_interrupts(struct denali_nand_info *denali)
 {
-	uint32_t status = 0;
-	status = read_interrupt_status();
-	clear_interrupt(status);
-	denali.irq_status = 0;
+	uint32_t status;
+
+	status = read_interrupt_status(denali);
+	clear_interrupt(denali, status);
+
+	denali->irq_status = 0;
 }
 
-static void denali_irq_enable(uint32_t int_mask)
+static void denali_irq_enable(struct denali_nand_info *denali,
+							uint32_t int_mask)
 {
 	int i;
-	for (i = 0; i < denali.max_banks; ++i)
-		writel(int_mask, denali.flash_reg + INTR_EN(i));
+
+	for (i = 0; i < denali->max_banks; ++i)
+		writel(int_mask, denali->flash_reg + INTR_EN(i));
 }
 
-static uint32_t wait_for_irq(uint32_t irq_mask)
+static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
 {
 	unsigned long timeout = 1000000;
 	uint32_t intr_status;
 
 	do {
-		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
+		intr_status = read_interrupt_status(denali) & DENALI_IRQ_ALL;
 		if (intr_status & irq_mask) {
-			denali.irq_status &= ~irq_mask;
+			denali->irq_status &= ~irq_mask;
 			/* our interrupt was detected */
 			break;
 		}
@@ -114,7 +129,7 @@ static uint32_t wait_for_irq(uint32_t irq_mask)
 	if (timeout == 0) {
 		/* timeout */
 		printf("Denali timeout with interrupt status %08x\n",
-		       read_interrupt_status());
+		       read_interrupt_status(denali));
 		intr_status = 0;
 	}
 	return intr_status;
@@ -126,80 +141,82 @@ static uint32_t wait_for_irq(uint32_t irq_mask)
  * 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)
+static void index_addr(struct denali_nand_info *denali,
+				uint32_t address, uint32_t data)
 {
-	writel(address, denali.flash_mem + INDEX_CTRL_REG);
-	writel(data, denali.flash_mem + INDEX_DATA_REG);
+	writel(address, denali->flash_mem + INDEX_CTRL_REG);
+	writel(data, denali->flash_mem + INDEX_DATA_REG);
 }
 
 /* Perform an indexed read of the device */
-static void index_addr_read_data(uint32_t address, uint32_t *pdata)
+static void index_addr_read_data(struct denali_nand_info *denali,
+				 uint32_t address, uint32_t *pdata)
 {
-	writel(address, denali.flash_mem + INDEX_CTRL_REG);
-	*pdata = readl(denali.flash_mem + INDEX_DATA_REG);
+	writel(address, denali->flash_mem + INDEX_CTRL_REG);
+	*pdata = readl(denali->flash_mem + INDEX_DATA_REG);
 }
 
 /* We need to buffer some data for some of the NAND core routines.
  * The operations manage buffering that data. */
-static void reset_buf(void)
+static void reset_buf(struct denali_nand_info *denali)
 {
-	denali.buf.head = 0;
-	denali.buf.tail = 0;
+	denali->buf.head = denali->buf.tail = 0;
 }
 
-static void write_byte_to_buf(uint8_t byte)
+static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
 {
-	BUG_ON(denali.buf.tail >= sizeof(denali.buf.buf));
-	denali.buf.buf[denali.buf.tail++] = byte;
+	denali->buf.buf[denali->buf.tail++] = byte;
 }
 
 /* resets a specific device connected to the core */
-static void reset_bank(void)
+static void reset_bank(struct denali_nand_info *denali)
 {
 	uint32_t irq_status;
 	uint32_t irq_mask = INTR_STATUS__RST_COMP |
 			    INTR_STATUS__TIME_OUT;
 
-	clear_interrupts();
+	clear_interrupts(denali);
 
-	writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
+	writel(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
 
-	irq_status = wait_for_irq(irq_mask);
+	irq_status = wait_for_irq(denali, irq_mask);
 	if (irq_status & INTR_STATUS__TIME_OUT)
 		debug(KERN_ERR "reset bank failed.\n");
 }
 
 /* Reset the flash controller */
-static uint32_t denali_nand_reset(void)
+static uint32_t denali_nand_reset(struct denali_nand_info *denali)
 {
 	uint32_t i;
 
-	for (i = 0; i < denali.max_banks; i++)
+	for (i = 0; i < denali->max_banks; i++)
 		writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
-		       denali.flash_reg + INTR_STATUS(i));
+		       denali->flash_reg + INTR_STATUS(i));
 
-	for (i = 0; i < denali.max_banks; i++) {
-		writel(1 << i, denali.flash_reg + DEVICE_RESET);
-		while (!(readl(denali.flash_reg +	INTR_STATUS(i)) &
+	for (i = 0; i < denali->max_banks; i++) {
+		writel(1 << i, denali->flash_reg + DEVICE_RESET);
+		while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
 			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
-			if (readl(denali.flash_reg + INTR_STATUS(i)) &
+			if (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++)
+	for (i = 0; i < denali->max_banks; i++)
 		writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
-		       denali.flash_reg + INTR_STATUS(i));
+		       denali->flash_reg + INTR_STATUS(i));
 
 	return 0;
 }
 
-/* this routine calculates the ONFI timing values for a given mode and
+/*
+ * 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(uint32_t mode)
+static void nand_onfi_timing_set(struct denali_nand_info *denali,
+								uint16_t mode)
 {
 	uint32_t trea[6] = {40, 30, 25, 20, 20, 16};
 	uint32_t trp[6] = {50, 25, 17, 15, 12, 10};
@@ -280,91 +297,93 @@ static void nand_onfi_timing_set(uint32_t mode)
 #endif
 
 	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
-	if ((readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
-	    (readl(denali.flash_reg + DEVICE_ID) == 0x88))
+	if ((readl(denali->flash_reg + MANUFACTURER_ID) == 0) &&
+	    (readl(denali->flash_reg + DEVICE_ID) == 0x88))
 		acc_clks = 6;
 
-	writel(acc_clks, denali.flash_reg + ACC_CLKS);
-	writel(re_2_we, denali.flash_reg + RE_2_WE);
-	writel(re_2_re, denali.flash_reg + RE_2_RE);
-	writel(we_2_re, denali.flash_reg + WE_2_RE);
-	writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
-	writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
-	writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
-	writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
+	writel(acc_clks, denali->flash_reg + ACC_CLKS);
+	writel(re_2_we, denali->flash_reg + RE_2_WE);
+	writel(re_2_re, denali->flash_reg + RE_2_RE);
+	writel(we_2_re, denali->flash_reg + WE_2_RE);
+	writel(addr_2_data, denali->flash_reg + ADDR_2_DATA);
+	writel(en_lo, denali->flash_reg + RDWR_EN_LO_CNT);
+	writel(en_hi, denali->flash_reg + RDWR_EN_HI_CNT);
+	writel(cs_cnt, denali->flash_reg + CS_SETUP_CNT);
 }
 
 /* queries the NAND device to see what ONFI modes it supports. */
-static uint32_t get_onfi_nand_para(void)
+static uint32_t get_onfi_nand_para(struct denali_nand_info *denali)
 {
 	int i;
-	/* we needn't to do a reset here because driver has already
+	/*
+	 * we needn't to do a reset here because driver has already
 	 * reset all the banks before
-	 * */
-	if (!(readl(denali.flash_reg + ONFI_TIMING_MODE) &
+	 */
+	if (!(readl(denali->flash_reg + ONFI_TIMING_MODE) &
 	    ONFI_TIMING_MODE__VALUE))
 		return -EIO;
 
 	for (i = 5; i > 0; i--) {
-		if (readl(denali.flash_reg + ONFI_TIMING_MODE) &
+		if (readl(denali->flash_reg + ONFI_TIMING_MODE) &
 			(0x01 << i))
 			break;
 	}
 
-	nand_onfi_timing_set(i);
+	nand_onfi_timing_set(denali, i);
 
 	/* By now, all the ONFI devices we know support the page cache */
 	/* rw feature. So here we enable the pipeline_rw_ahead feature */
 	return 0;
 }
 
-static void get_samsung_nand_para(uint8_t device_id)
+static void get_samsung_nand_para(struct denali_nand_info *denali,
+							uint8_t device_id)
 {
 	if (device_id == 0xd3) { /* Samsung K9WAG08U1A */
 		/* Set timing register values according to datasheet */
-		writel(5, denali.flash_reg + ACC_CLKS);
-		writel(20, denali.flash_reg + RE_2_WE);
-		writel(12, denali.flash_reg + WE_2_RE);
-		writel(14, denali.flash_reg + ADDR_2_DATA);
-		writel(3, denali.flash_reg + RDWR_EN_LO_CNT);
-		writel(2, denali.flash_reg + RDWR_EN_HI_CNT);
-		writel(2, denali.flash_reg + CS_SETUP_CNT);
+		writel(5, denali->flash_reg + ACC_CLKS);
+		writel(20, denali->flash_reg + RE_2_WE);
+		writel(12, denali->flash_reg + WE_2_RE);
+		writel(14, denali->flash_reg + ADDR_2_DATA);
+		writel(3, denali->flash_reg + RDWR_EN_LO_CNT);
+		writel(2, denali->flash_reg + RDWR_EN_HI_CNT);
+		writel(2, denali->flash_reg + CS_SETUP_CNT);
 	}
 }
 
-static void get_toshiba_nand_para(void)
+static void get_toshiba_nand_para(struct denali_nand_info *denali)
 {
 	uint32_t tmp;
 
 	/* Workaround to fix a controller bug which reports a wrong */
 	/* spare area size for some kind of Toshiba NAND device */
-	if ((readl(denali.flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
-	    (readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE)
-		== 64)){
-		writel(216, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
-		tmp = readl(denali.flash_reg + DEVICES_CONNECTED) *
-			readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
-		writel(tmp, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+	if ((readl(denali->flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
+	    (readl(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64)) {
+		writel(216, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
+		tmp = readl(denali->flash_reg + DEVICES_CONNECTED) *
+			readl(denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
+		writel(tmp, denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
 	}
 }
 
-static void get_hynix_nand_para(uint8_t device_id)
+static void get_hynix_nand_para(struct denali_nand_info *denali,
+							uint8_t device_id)
 {
 	uint32_t main_size, spare_size;
 
 	switch (device_id) {
 	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
 	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
-		writel(128, denali.flash_reg + PAGES_PER_BLOCK);
-		writel(4096, denali.flash_reg + DEVICE_MAIN_AREA_SIZE);
-		writel(224, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		writel(128, denali->flash_reg + PAGES_PER_BLOCK);
+		writel(4096, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
+		writel(224, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
 		main_size = 4096 *
-			readl(denali.flash_reg + DEVICES_CONNECTED);
+			readl(denali->flash_reg + DEVICES_CONNECTED);
 		spare_size = 224 *
-			readl(denali.flash_reg + DEVICES_CONNECTED);
-		writel(main_size, denali.flash_reg + LOGICAL_PAGE_DATA_SIZE);
-		writel(spare_size, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
-		writel(0, denali.flash_reg + DEVICE_WIDTH);
+			readl(denali->flash_reg + DEVICES_CONNECTED);
+		writel(main_size, denali->flash_reg + LOGICAL_PAGE_DATA_SIZE);
+		writel(spare_size, denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+		writel(0, denali->flash_reg + DEVICE_WIDTH);
 		break;
 	default:
 		debug(KERN_WARNING
@@ -374,27 +393,28 @@ static void get_hynix_nand_para(uint8_t device_id)
 	}
 }
 
-/* determines how many NAND chips are connected to the controller. Note for
+/*
+ * 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)
+static void find_valid_banks(struct denali_nand_info *denali)
 {
-	uint32_t id[denali.max_banks];
+	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);
-		index_addr_read_data((uint32_t)(MODE_11 | (i << 24) | 2),
-				     &id[i]);
+	denali->total_used_banks = 1;
+	for (i = 0; i < denali->max_banks; i++) {
+		index_addr(denali, (uint32_t)(MODE_11 | (i << 24) | 0), 0x90);
+		index_addr(denali, (uint32_t)(MODE_11 | (i << 24) | 1), 0);
+		index_addr_read_data(denali,
+				(uint32_t)(MODE_11 | (i << 24) | 2), &id[i]);
 
 		if (i == 0) {
 			if (!(id[i] & 0x0ff))
 				break;
 		} else {
 			if ((id[i] & 0x0ff) == (id[0] & 0x0ff))
-				denali.total_used_banks++;
+				denali->total_used_banks++;
 			else
 				break;
 		}
@@ -405,39 +425,40 @@ static void find_valid_banks(void)
  * Use the configuration feature register to determine the maximum number of
  * banks that the hardware supports.
  */
-static void detect_max_banks(void)
+static void detect_max_banks(struct denali_nand_info *denali)
 {
-	uint32_t features = readl(denali.flash_reg + FEATURES);
-	denali.max_banks = 2 << (features & FEATURES__N_BANKS);
+	uint32_t features = readl(denali->flash_reg + FEATURES);
+	denali->max_banks = 2 << (features & FEATURES__N_BANKS);
 }
 
-static void detect_partition_feature(void)
+static void detect_partition_feature(struct denali_nand_info *denali)
 {
-	/* For MRST platform, denali.fwblks represent the
+	/*
+	 * 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 (readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
-		if ((readl(denali.flash_reg + PERM_SRC_ID(1)) &
+	 */
+	if (readl(denali->flash_reg + FEATURES) & FEATURES__PARTITION) {
+		if ((readl(denali->flash_reg + PERM_SRC_ID(1)) &
 			PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
-			denali.fwblks =
-			    ((readl(denali.flash_reg + MIN_MAX_BANK(1)) &
+			denali->fwblks =
+			    ((readl(denali->flash_reg + MIN_MAX_BANK(1)) &
 			      MIN_MAX_BANK__MIN_VALUE) *
-			     denali.blksperchip)
+			     denali->blksperchip)
 			    +
-			    (readl(denali.flash_reg + MIN_BLK_ADDR(1)) &
+			    (readl(denali->flash_reg + MIN_BLK_ADDR(1)) &
 			    MIN_BLK_ADDR__VALUE);
 		} else {
-			denali.fwblks = SPECTRA_START_BLOCK;
+			denali->fwblks = SPECTRA_START_BLOCK;
 		}
 	} else {
-		denali.fwblks = SPECTRA_START_BLOCK;
+		denali->fwblks = SPECTRA_START_BLOCK;
 	}
 }
 
-static uint32_t denali_nand_timing_set(void)
+static uint32_t denali_nand_timing_set(struct denali_nand_info *denali)
 {
 	uint32_t id_bytes[5], addr;
 	uint8_t i, maf_id, device_id;
@@ -447,35 +468,35 @@ static uint32_t denali_nand_timing_set(void)
 	 * report the correct device ID by reading from
 	 * DEVICE_ID register
 	 * */
-	addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
-	index_addr((uint32_t)addr | 0, 0x90);
-	index_addr((uint32_t)addr | 1, 0);
+	addr = (uint32_t)MODE_11 | BANK(denali->flash_bank);
+	index_addr(denali, (uint32_t)addr | 0, 0x90);
+	index_addr(denali, (uint32_t)addr | 1, 0);
 	for (i = 0; i < 5; i++)
-		index_addr_read_data(addr | 2, &id_bytes[i]);
+		index_addr_read_data(denali, addr | 2, &id_bytes[i]);
 	maf_id = id_bytes[0];
 	device_id = id_bytes[1];
 
-	if (readl(denali.flash_reg + ONFI_DEVICE_NO_OF_LUNS) &
+	if (readl(denali->flash_reg + ONFI_DEVICE_NO_OF_LUNS) &
 		ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE) { /* ONFI 1.0 NAND */
-		if (get_onfi_nand_para())
+		if (get_onfi_nand_para(denali))
 			return -EIO;
 	} else if (maf_id == 0xEC) { /* Samsung NAND */
-		get_samsung_nand_para(device_id);
+		get_samsung_nand_para(denali, device_id);
 	} else if (maf_id == 0x98) { /* Toshiba NAND */
-		get_toshiba_nand_para();
+		get_toshiba_nand_para(denali);
 	} else if (maf_id == 0xAD) { /* Hynix NAND */
-		get_hynix_nand_para(device_id);
+		get_hynix_nand_para(denali, device_id);
 	}
 
-	find_valid_banks();
+	find_valid_banks(denali);
 
-	detect_partition_feature();
+	detect_partition_feature(denali);
 
 	/* If the user specified to override the default timings
 	 * with a specific ONFI mode, we apply those changes here.
 	 */
 	if (onfi_timing_mode != NAND_DEFAULT_TIMINGS)
-		nand_onfi_timing_set(onfi_timing_mode);
+		nand_onfi_timing_set(denali, onfi_timing_mode);
 
 	return 0;
 }
@@ -488,26 +509,27 @@ static inline bool is_flash_bank_valid(int flash_bank)
 	return (flash_bank >= 0 && flash_bank < 4);
 }
 
-static void denali_irq_init(void)
+static void denali_irq_init(struct denali_nand_info *denali)
 {
 	uint32_t int_mask = 0;
 	int i;
 
 	/* Disable global interrupts */
-	writel(0, denali.flash_reg + GLOBAL_INT_ENABLE);
+	writel(0, denali->flash_reg + GLOBAL_INT_ENABLE);
 
 	int_mask = DENALI_IRQ_ALL;
 
 	/* Clear all status bits */
-	for (i = 0; i < denali.max_banks; ++i)
-		writel(0xFFFF, denali.flash_reg + INTR_STATUS(i));
+	for (i = 0; i < denali->max_banks; ++i)
+		writel(0xFFFF, denali->flash_reg + INTR_STATUS(i));
 
-	denali_irq_enable(int_mask);
+	denali_irq_enable(denali, int_mask);
 }
 
 /* This helper function setups the registers for ECC and whether or not
  * the spare area will be transferred. */
-static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
+static void setup_ecc_for_xfer(struct denali_nand_info *denali, bool ecc_en,
+				bool transfer_spare)
 {
 	int ecc_en_flag = 0, transfer_spare_flag = 0;
 
@@ -516,40 +538,41 @@ static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
 	transfer_spare_flag = transfer_spare ? TRANSFER_SPARE_REG__FLAG : 0;
 
 	/* Enable spare area/ECC per user's request. */
-	writel(ecc_en_flag, denali.flash_reg + ECC_ENABLE);
+	writel(ecc_en_flag, denali->flash_reg + ECC_ENABLE);
 	/* applicable for MAP01 only */
-	writel(transfer_spare_flag, denali.flash_reg + TRANSFER_SPARE_REG);
+	writel(transfer_spare_flag, denali->flash_reg + TRANSFER_SPARE_REG);
 }
 
 /* sends a pipeline command operation to the controller. See the Denali NAND
  * controller's user guide for more information (section 4.2.3.6).
  */
-static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
+static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
+					bool ecc_en, bool transfer_spare,
 					int access_type, int op)
 {
-	uint32_t addr = 0x0, cmd = 0x0, irq_status;
+	uint32_t addr, cmd, irq_status;
 	static uint32_t page_count = 1;
 
-	setup_ecc_for_xfer(ecc_en, transfer_spare);
+	setup_ecc_for_xfer(denali, ecc_en, transfer_spare);
 
 	/* clear interrupts */
-	clear_interrupts();
+	clear_interrupts(denali);
 
-	addr = BANK(denali.flash_bank) | denali.page;
+	addr = BANK(denali->flash_bank) | denali->page;
 
 	/* setup the acccess type */
 	cmd = MODE_10 | addr;
-	index_addr(cmd, access_type);
+	index_addr(denali, cmd, access_type);
 
 	/* setup the pipeline command */
-	index_addr(cmd, 0x2000 | op | page_count);
+	index_addr(denali, cmd, 0x2000 | op | page_count);
 
 	cmd = MODE_01 | addr;
-	writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
+	writel(cmd, denali->flash_mem + INDEX_CTRL_REG);
 
 	if (op == DENALI_READ) {
 		/* wait for command to be accepted */
-		irq_status = wait_for_irq(INTR_STATUS__LOAD_COMP);
+		irq_status = wait_for_irq(denali, INTR_STATUS__LOAD_COMP);
 
 		if (irq_status == 0)
 			return -EIO;
@@ -559,7 +582,8 @@ static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
 }
 
 /* helper function that simply writes a buffer to the flash */
-static int write_data_to_flash_mem(const void *buf, int len)
+static int write_data_to_flash_mem(struct denali_nand_info *denali,
+						const uint8_t *buf, int len)
 {
 	uint32_t i = 0, *buf32;
 
@@ -570,12 +594,13 @@ static int write_data_to_flash_mem(const void *buf, int len)
 	/* write the data to the flash memory */
 	buf32 = (uint32_t *)buf;
 	for (i = 0; i < len / 4; i++)
-		writel(*buf32++, denali.flash_mem + INDEX_DATA_REG);
+		writel(*buf32++, denali->flash_mem + INDEX_DATA_REG);
 	return i * 4; /* intent is to return the number of bytes read */
 }
 
 /* helper function that simply reads a buffer from the flash */
-static int read_data_from_flash_mem(uint8_t *buf, int len)
+static int read_data_from_flash_mem(struct denali_nand_info *denali,
+						uint8_t *buf, int len)
 {
 	uint32_t i, *buf32;
 
@@ -593,42 +618,46 @@ static int read_data_from_flash_mem(uint8_t *buf, int len)
 	/* transfer the data from the flash */
 	buf32 = (uint32_t *)buf;
 	for (i = 0; i < len / 4; i++)
-		*buf32++ = readl(denali.flash_mem + INDEX_DATA_REG);
+		*buf32++ = readl(denali->flash_mem + INDEX_DATA_REG);
+
 	return i * 4; /* intent is to return the number of bytes read */
 }
 
-static void denali_mode_main_access(void)
+static void denali_mode_main_access(struct denali_nand_info *denali)
 {
 	uint32_t addr, cmd;
-	addr = BANK(denali.flash_bank) | denali.page;
+
+	addr = BANK(denali->flash_bank) | denali->page;
 	cmd = MODE_10 | addr;
-	index_addr(cmd, MAIN_ACCESS);
+	index_addr(denali, cmd, MAIN_ACCESS);
 }
 
-static void denali_mode_main_spare_access(void)
+static void denali_mode_main_spare_access(struct denali_nand_info *denali)
 {
 	uint32_t addr, cmd;
-	addr = BANK(denali.flash_bank) | denali.page;
+
+	addr = BANK(denali->flash_bank) | denali->page;
 	cmd = MODE_10 | addr;
-	index_addr(cmd, MAIN_SPARE_ACCESS);
+	index_addr(denali, cmd, MAIN_SPARE_ACCESS);
 }
 
 /* writes OOB data to the device */
 static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	uint32_t irq_status;
 	uint32_t irq_mask = INTR_STATUS__PROGRAM_COMP |
 						INTR_STATUS__PROGRAM_FAIL;
 	int status = 0;
 
-	denali.page = page;
+	denali->page = page;
 
-	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
+	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
 							DENALI_WRITE) == 0) {
-		write_data_to_flash_mem(buf, mtd->oobsize);
+		write_data_to_flash_mem(denali, buf, mtd->oobsize);
 
 		/* wait for operation to complete */
-		irq_status = wait_for_irq(irq_mask);
+		irq_status = wait_for_irq(denali, irq_mask);
 
 		if (irq_status == 0) {
 			dev_err(denali->dev, "OOB write failed\n");
@@ -644,22 +673,23 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 /* reads OOB data from the device */
 static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	uint32_t irq_mask = INTR_STATUS__LOAD_COMP,
 			 irq_status = 0, addr = 0x0, cmd = 0x0;
 
-	denali.page = page;
+	denali->page = page;
 
-	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
+	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
 							DENALI_READ) == 0) {
-		read_data_from_flash_mem(buf, mtd->oobsize);
+		read_data_from_flash_mem(denali, buf, mtd->oobsize);
 
 		/* wait for command to be accepted
 		 * can always use status0 bit as the mask is identical for each
 		 * bank. */
-		irq_status = wait_for_irq(irq_mask);
+		irq_status = wait_for_irq(denali, irq_mask);
 
 		if (irq_status == 0)
-			printf("page on OOB timeout %d\n", denali.page);
+			printf("page on OOB timeout %d\n", denali->page);
 
 		/* We set the device back to MAIN_ACCESS here as I observed
 		 * instability with the controller if you do a block erase
@@ -667,16 +697,16 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 		 * is reliable (according to the MTD test infrastructure)
 		 * if you are in MAIN_ACCESS.
 		 */
-		addr = BANK(denali.flash_bank) | denali.page;
+		addr = BANK(denali->flash_bank) | denali->page;
 		cmd = MODE_10 | addr;
-		index_addr(cmd, MAIN_ACCESS);
+		index_addr(denali, cmd, MAIN_ACCESS);
 	}
 }
 
 /* this function examines buffers to see if they contain data that
  * indicate that the buffer is part of an erased region of flash.
  */
-bool is_erased(uint8_t *buf, int len)
+static bool is_erased(uint8_t *buf, int len)
 {
 	int i = 0;
 	for (i = 0; i < len; i++)
@@ -685,80 +715,80 @@ bool is_erased(uint8_t *buf, int len)
 	return true;
 }
 
-
 /* programs the controller to either enable/disable DMA transfers */
-static void denali_enable_dma(bool en)
+static void denali_enable_dma(struct denali_nand_info *denali, bool en)
 {
 	uint32_t reg_val = 0x0;
 
 	if (en)
 		reg_val = DMA_ENABLE__FLAG;
 
-	writel(reg_val, denali.flash_reg + DMA_ENABLE);
-	readl(denali.flash_reg + DMA_ENABLE);
+	writel(reg_val, denali->flash_reg + DMA_ENABLE);
+	readl(denali->flash_reg + DMA_ENABLE);
 }
 
 /* setups the HW to perform the data DMA */
-static void denali_setup_dma(int op)
+static void denali_setup_dma(struct denali_nand_info *denali, int op)
 {
-	const int page_count = 1;
 	uint32_t mode;
-	uint32_t addr = (uint32_t)denali.buf.dma_buf;
+	const int page_count = 1;
+	uint32_t addr = (uint32_t)denali->buf.dma_buf;
 
-	flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
+	flush_dcache_range(addr, addr + sizeof(denali->buf.dma_buf));
 
 #ifdef CONFIG_NAND_DENALI_64BIT
-	mode = MODE_10 | BANK(denali.flash_bank) | denali.page;
+	mode = MODE_10 | BANK(denali->flash_bank) | denali->page;
 
 	/* DMA is a three step process */
 
 	/* 1. setup transfer type, interrupt when complete,
 	      burst len = 64 bytes, the number of pages */
-	index_addr(mode, 0x01002000 | (64 << 16) | op | page_count);
+	index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
 
 	/* 2. set memory low address bits 31:0 */
-	index_addr(mode, addr);
+	index_addr(denali, mode, addr);
 
 	/* 3. set memory high address bits 64:32 */
-	index_addr(mode, 0);
+	index_addr(denali, mode, 0);
 #else
-	mode = MODE_10 | BANK(denali.flash_bank);
+	mode = MODE_10 | BANK(denali->flash_bank);
 
 	/* DMA is a four step process */
 
 	/* 1. setup transfer type and # of pages */
-	index_addr(mode | denali.page, 0x2000 | op | page_count);
+	index_addr(denali, mode | denali->page, 0x2000 | op | page_count);
 
 	/* 2. set memory high address bits 23:8 */
-	index_addr(mode | ((uint32_t)(addr >> 16) << 8), 0x2200);
+	index_addr(denali, mode | ((uint32_t)(addr >> 16) << 8), 0x2200);
 
 	/* 3. set memory low address bits 23:8 */
-	index_addr(mode | ((uint32_t)addr << 8), 0x2300);
+	index_addr(denali, mode | ((uint32_t)addr << 8), 0x2300);
 
 	/* 4.  interrupt when complete, burst len = 64 bytes*/
-	index_addr(mode | 0x14000, 0x2400);
+	index_addr(denali, mode | 0x14000, 0x2400);
 #endif
 }
 
 /* Common DMA function */
-static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
+static uint32_t denali_dma_configuration(struct denali_nand_info *denali,
+					 uint32_t ops, bool raw_xfer,
 					 uint32_t irq_mask, int oob_required)
 {
 	uint32_t irq_status = 0;
 	/* setup_ecc_for_xfer(bool ecc_en, bool transfer_spare) */
-	setup_ecc_for_xfer(!raw_xfer, oob_required);
+	setup_ecc_for_xfer(denali, !raw_xfer, oob_required);
 
 	/* clear any previous interrupt flags */
-	clear_interrupts();
+	clear_interrupts(denali);
 
 	/* enable the DMA */
-	denali_enable_dma(true);
+	denali_enable_dma(denali, true);
 
 	/* setup the DMA */
-	denali_setup_dma(ops);
+	denali_setup_dma(denali, ops);
 
 	/* wait for operation to complete */
-	irq_status = wait_for_irq(irq_mask);
+	irq_status = wait_for_irq(denali, irq_mask);
 
 	/* if ECC fault happen, seems we need delay before turning off DMA.
 	 * If not, the controller will go into non responsive condition */
@@ -766,7 +796,7 @@ static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
 		udelay(100);
 
 	/* disable the DMA */
-	denali_enable_dma(false);
+	denali_enable_dma(denali, false);
 
 	return irq_status;
 }
@@ -774,33 +804,35 @@ static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
 static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 			const uint8_t *buf, bool raw_xfer, int oob_required)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+
 	uint32_t irq_status = 0;
 	uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP;
 
-	denali.status = 0;
+	denali->status = 0;
 
 	/* copy buffer into DMA buffer */
-	memcpy((void *)denali.buf.dma_buf, buf, mtd->writesize);
+	memcpy((void *)denali->buf.dma_buf, buf, mtd->writesize);
 
 	/* need extra memcpoy for raw transfer */
 	if (raw_xfer)
-		memcpy((void *)denali.buf.dma_buf + mtd->writesize,
+		memcpy((void *)denali->buf.dma_buf + mtd->writesize,
 		       chip->oob_poi, mtd->oobsize);
 
 	/* setting up DMA */
-	irq_status = denali_dma_configuration(DENALI_WRITE, raw_xfer, irq_mask,
-						oob_required);
+	irq_status = denali_dma_configuration(denali, DENALI_WRITE, raw_xfer,
+					      irq_mask, oob_required);
 
 	/* if timeout happen, error out */
 	if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
 		debug("DMA timeout for denali write_page\n");
-		denali.status = NAND_STATUS_FAIL;
+		denali->status = NAND_STATUS_FAIL;
 		return -EIO;
 	}
 
 	if (irq_status & INTR_STATUS__LOCKED_BLK) {
 		debug("Failed as write to locked block\n");
-		denali.status = NAND_STATUS_FAIL;
+		denali->status = NAND_STATUS_FAIL;
 		return -EIO;
 	}
 	return 0;
@@ -816,18 +848,18 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 				const uint8_t *buf, int oob_required)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+
 	/*
 	 * for regular page writes, we let HW handle all the ECC
 	 * data written to the device.
 	 */
-	debug("denali_write_page at page %08x\n", denali.page);
-
 	if (oob_required)
 		/* switch to main + spare access */
-		denali_mode_main_spare_access();
+		denali_mode_main_spare_access(denali);
 	else
 		/* switch to main access only */
-		denali_mode_main_access();
+		denali_mode_main_access(denali);
 
 	return write_page(mtd, chip, buf, false, oob_required);
 }
@@ -840,18 +872,19 @@ static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 					const uint8_t *buf, int oob_required)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+
 	/*
 	 * for raw page writes, we want to disable ECC and simply write
 	 * whatever data is in the buffer.
 	 */
-	debug("denali_write_page_raw at page %08x\n", denali.page);
 
 	if (oob_required)
 		/* switch to main + spare access */
-		denali_mode_main_spare_access();
+		denali_mode_main_spare_access(denali);
 	else
 		/* switch to main access only */
-		denali_mode_main_access();
+		denali_mode_main_access(denali);
 
 	return write_page(mtd, chip, buf, true, oob_required);
 }
@@ -866,24 +899,25 @@ static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
 static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int oob_required, int page)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+
 	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) {
+	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();
+		denali_mode_main_spare_access(denali);
 	else
 		/* switch to main access only */
-		denali_mode_main_access();
+		denali_mode_main_access(denali);
 
 	/* setting up the DMA where ecc_enable is false */
-	irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
-						oob_required);
+	irq_status = denali_dma_configuration(denali, DENALI_READ, true,
+					      irq_mask, oob_required);
 
 	/* if timeout happen, error out */
 	if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
@@ -892,44 +926,42 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	}
 
 	/* splitting the content to destination buffer holder */
-	memcpy(chip->oob_poi, (denali.buf.dma_buf + mtd->writesize),
+	memcpy(chip->oob_poi, (denali->buf.dma_buf + mtd->writesize),
 	       mtd->oobsize);
-	memcpy(buf, denali.buf.dma_buf, mtd->writesize);
-	debug("buf %02x %02x\n", buf[0], buf[1]);
-	debug("chip->oob_poi %02x %02x\n", chip->oob_poi[0], chip->oob_poi[1]);
+	memcpy(buf, denali->buf.dma_buf, mtd->writesize);
+
 	return 0;
 }
 
 static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int oob_required, int page)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	uint32_t irq_status, irq_mask =	INTR_STATUS__DMA_CMD_COMP;
 
-	debug("denali_read_page at page %08x\n", page);
-	if (denali.page != 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();
+		denali_mode_main_spare_access(denali);
 	else
 		/* switch to main access only */
-		denali_mode_main_access();
+		denali_mode_main_access(denali);
 
 	/* setting up the DMA where ecc_enable is true */
-	irq_status = denali_dma_configuration(DENALI_READ, false, irq_mask,
-						oob_required);
+	irq_status = denali_dma_configuration(denali, DENALI_READ, false,
+					      irq_mask, oob_required);
 
-	memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
-	debug("buf %02x %02x\n", buf[0], buf[1]);
+	memcpy(buf, (const void *)denali->buf.dma_buf, mtd->writesize);
 
 	/* check whether any ECC error */
 	if (irq_status & INTR_STATUS__ECC_UNCOR_ERR) {
 		/* is the ECC cause by erase page, check using read_page_raw */
 		debug("  Uncorrected ECC detected\n");
-		denali_read_page_raw(mtd, chip, buf, oob_required, denali.page);
+		denali_read_page_raw(mtd, chip, buf, oob_required, denali->page);
 
 		if (is_erased(buf, mtd->writesize) == true &&
 		    is_erased(chip->oob_poi, mtd->oobsize) == true) {
@@ -939,18 +971,10 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			return -EIO;
 		}
 	}
-	memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
+	memcpy(buf, (const void *)denali->buf.dma_buf, mtd->writesize);
 	return 0;
 }
 
-static uint8_t denali_read_byte(struct mtd_info *mtd)
-{
-	uint32_t addr, result;
-	addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
-	index_addr_read_data((uint32_t)addr | 2, &result);
-	return (uint8_t)result & 0xFF;
-}
-
 static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 				int page)
 {
@@ -959,91 +983,105 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
+static uint8_t denali_read_byte(struct mtd_info *mtd)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	uint32_t addr, result;
+
+	addr = (uint32_t)MODE_11 | BANK(denali->flash_bank);
+	index_addr_read_data(denali, addr | 2, &result);
+	return (uint8_t)result & 0xFF;
+}
+
 static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	uint32_t i, addr, result;
 
 	/* delay for tR (data transfer from Flash array to data register) */
 	udelay(25);
 
 	/* ensure device completed else additional delay and polling */
-	wait_for_irq(INTR_STATUS__INT_ACT);
+	wait_for_irq(denali, INTR_STATUS__INT_ACT);
 
-	addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
+	addr = (uint32_t)MODE_11 | BANK(denali->flash_bank);
 	for (i = 0; i < len; i++) {
-		index_addr_read_data((uint32_t)addr | 2, &result);
-		write_byte_to_buf(result);
+		index_addr_read_data(denali, (uint32_t)addr | 2, &result);
+		write_byte_to_buf(denali, result);
 	}
-	memcpy(buf, denali.buf.buf, len);
+	memcpy(buf, denali->buf.buf, len);
 }
 
 static void denali_select_chip(struct mtd_info *mtd, int chip)
 {
-	denali.flash_bank = chip;
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+
+	denali->flash_bank = chip;
 }
 
 static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
 {
-	int status = denali.status;
-	denali.status = 0;
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	int status = denali->status;
+	denali->status = 0;
 
 	return status;
 }
 
 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);
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	uint32_t cmd, irq_status;
 
 	/* clear interrupts */
-	clear_interrupts();
+	clear_interrupts(denali);
 
 	/* setup page read request for access type */
-	cmd = MODE_10 | BANK(denali.flash_bank) | page;
-	index_addr((uint32_t)cmd, 0x1);
+	cmd = MODE_10 | BANK(denali->flash_bank) | page;
+	index_addr(denali, cmd, 0x1);
 
 	/* wait for erase to complete or failure to occur */
-	irq_status = wait_for_irq(INTR_STATUS__ERASE_COMP |
-		INTR_STATUS__ERASE_FAIL);
+	irq_status = wait_for_irq(denali, INTR_STATUS__ERASE_COMP |
+					INTR_STATUS__ERASE_FAIL);
 
 	if (irq_status & INTR_STATUS__ERASE_FAIL ||
 	    irq_status & INTR_STATUS__LOCKED_BLK)
-		denali.status = NAND_STATUS_FAIL;
+		denali->status = NAND_STATUS_FAIL;
 	else
-		denali.status = 0;
+		denali->status = 0;
 }
 
 static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 			   int page)
 {
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	uint32_t addr;
 
 	switch (cmd) {
 	case NAND_CMD_PAGEPROG:
 		break;
 	case NAND_CMD_STATUS:
-		addr = MODE_11 | BANK(denali.flash_bank);
-		index_addr(addr | 0, cmd);
+		addr = MODE_11 | BANK(denali->flash_bank);
+		index_addr(denali, addr | 0, cmd);
 		break;
 	case NAND_CMD_PARAM:
-		clear_interrupts();
+		clear_interrupts(denali);
 	case NAND_CMD_READID:
-		reset_buf();
+		reset_buf(denali);
 		/* sometimes ManufactureId read from register is not right
 		 * e.g. some of Micron MT29F32G08QAA MLC NAND chips
 		 * So here we send READID cmd to NAND insteand
 		 * */
-		addr = MODE_11 | BANK(denali.flash_bank);
-		index_addr(addr | 0, cmd);
-		index_addr(addr | 1, col & 0xFF);
+		addr = MODE_11 | BANK(denali->flash_bank);
+		index_addr(denali, addr | 0, cmd);
+		index_addr(denali, addr | 1, col & 0xFF);
 		break;
 	case NAND_CMD_READ0:
 	case NAND_CMD_SEQIN:
-		denali.page = page;
+		denali->page = page;
 		break;
 	case NAND_CMD_RESET:
-		reset_bank();
+		reset_bank(denali);
 		break;
 	case NAND_CMD_READOOB:
 		/* TODO: Read OOB data */
@@ -1060,20 +1098,20 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 		/* nothing to do here as it was done during NAND_CMD_ERASE1 */
 		break;
 	case NAND_CMD_UNLOCK1:
-		addr = MODE_10 | BANK(denali.flash_bank) | page;
-		index_addr(addr | 0, DENALI_UNLOCK_START);
+		addr = MODE_10 | BANK(denali->flash_bank) | page;
+		index_addr(denali, addr | 0, DENALI_UNLOCK_START);
 		break;
 	case NAND_CMD_UNLOCK2:
-		addr = MODE_10 | BANK(denali.flash_bank) | page;
-		index_addr(addr | 0, DENALI_UNLOCK_END);
+		addr = MODE_10 | BANK(denali->flash_bank) | page;
+		index_addr(denali, addr | 0, DENALI_UNLOCK_END);
 		break;
 	case NAND_CMD_LOCK:
-		addr = MODE_10 | BANK(denali.flash_bank);
-		index_addr(addr | 0, DENALI_LOCK);
+		addr = MODE_10 | BANK(denali->flash_bank);
+		index_addr(denali, addr | 0, DENALI_LOCK);
 		break;
 	case NAND_CMD_LOCK_TIGHT:
-		addr = MODE_10 | BANK(denali.flash_bank);
-		index_addr(addr | 0, DENALI_LOCK_TIGHT);
+		addr = MODE_10 | BANK(denali->flash_bank);
+		index_addr(denali, addr | 0, DENALI_LOCK_TIGHT);
 		break;
 	default:
 		printf(": unsupported command received 0x%x\n", cmd);
@@ -1083,34 +1121,42 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 /* end NAND core entry points */
 
 /* Initialization code to bring the device up to a known good state */
-static void denali_hw_init(void)
+static void denali_hw_init(struct denali_nand_info *denali)
 {
 	/*
 	 * tell driver how many bit controller will skip before writing
 	 * ECC code in OOB. This is normally used for bad block marker
 	 */
 	writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
-	       denali.flash_reg + SPARE_AREA_SKIP_BYTES);
-	detect_max_banks();
-	denali_nand_reset();
-	writel(0x0F, denali.flash_reg + RB_PIN_ENABLED);
+	       denali->flash_reg + SPARE_AREA_SKIP_BYTES);
+	detect_max_banks(denali);
+	denali_nand_reset(denali);
+	writel(0x0F, denali->flash_reg + RB_PIN_ENABLED);
 	writel(CHIP_EN_DONT_CARE__FLAG,
-	       denali.flash_reg + CHIP_ENABLE_DONT_CARE);
-	writel(0xffff, denali.flash_reg + SPARE_AREA_MARKER);
+	       denali->flash_reg + CHIP_ENABLE_DONT_CARE);
+	writel(0xffff, denali->flash_reg + SPARE_AREA_MARKER);
 
 	/* Should set value for these registers when init */
-	writel(0, denali.flash_reg + TWO_ROW_ADDR_CYCLES);
-	writel(1, denali.flash_reg + ECC_ENABLE);
-	denali_nand_timing_set();
-	denali_irq_init();
+	writel(0, denali->flash_reg + TWO_ROW_ADDR_CYCLES);
+	writel(1, denali->flash_reg + ECC_ENABLE);
+	denali_nand_timing_set(denali);
+	denali_irq_init(denali);
 }
 
 static struct nand_ecclayout nand_oob;
 
 static int denali_nand_init(struct nand_chip *nand)
 {
-	denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
-	denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
+	struct denali_nand_info *denali;
+
+	denali = malloc(sizeof(*denali));
+	if (!denali)
+		return -ENOMEM;
+
+	nand->priv = denali;
+
+	denali->flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
+	denali->flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
 
 #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
 	/* check whether flash got BBT table (located at end of flash). As we
@@ -1137,7 +1183,7 @@ static int denali_nand_init(struct nand_chip *nand)
 	 * Tell driver the ecc strength. This register may be already set
 	 * correctly. So we read this value out.
 	 */
-	nand->ecc.strength = readl(denali.flash_reg + ECC_CORRECTION);
+	nand->ecc.strength = readl(denali->flash_reg + ECC_CORRECTION);
 	switch (nand->ecc.size) {
 	case 512:
 		nand->ecc.bytes = (nand->ecc.strength * 13 + 15) / 16 * 2;
@@ -1158,7 +1204,7 @@ static int denali_nand_init(struct nand_chip *nand)
 	nand->read_buf = denali_read_buf;
 	nand->select_chip = denali_select_chip;
 	nand->waitfunc = denali_waitfunc;
-	denali_hw_init();
+	denali_hw_init(denali);
 	return 0;
 }
 
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-18 11:30 [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Masahiro Yamada
  2014-04-18 11:30 ` [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel Masahiro Yamada
@ 2014-04-21 22:21 ` Scott Wood
  2014-04-22  1:04   ` Masahiro Yamada
  2014-04-23  6:50 ` Chin Liang See
  2 siblings, 1 reply; 10+ messages in thread
From: Scott Wood @ 2014-04-21 22:21 UTC (permalink / raw)
  To: u-boot

On Fri, 2014-04-18 at 20:30 +0900, Masahiro Yamada wrote:
> [2] Make denali_read_oob()  10x faster.
> 
> One of the fatal issues of v7 is "nand bad" command is extremely slow.
> 
> This is the benchmark of v7
> 
>   => time nand bad
> 
>   Device 0 bad blocks:
> 
>   time: 11.300 seconds, 11300 ticks
> 
> It is really really painful to wait more than 10 seconds just for bad block
> scanning to boot Linux.

Making bad block scans faster is a good thing, but why do you need to
scan them just to boot Linux?  Aren't you using an on-flash BBT?

-Scott

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-21 22:21 ` [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Scott Wood
@ 2014-04-22  1:04   ` Masahiro Yamada
  2014-04-22 19:12     ` Scott Wood
  0 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2014-04-22  1:04 UTC (permalink / raw)
  To: u-boot

Hi Scott,


> > It is really really painful to wait more than 10 seconds just for bad block
> > scanning to boot Linux.
> 
> Making bad block scans faster is a good thing, but why do you need to
> scan them just to boot Linux?  Aren't you using an on-flash BBT?

I did not know that.
I thought all blocks must be scanned.

Could you teach me the better way?

Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-22  1:04   ` Masahiro Yamada
@ 2014-04-22 19:12     ` Scott Wood
  2014-04-23  7:15       ` Chin Liang See
  0 siblings, 1 reply; 10+ messages in thread
From: Scott Wood @ 2014-04-22 19:12 UTC (permalink / raw)
  To: u-boot

On Tue, 2014-04-22 at 10:04 +0900, Masahiro Yamada wrote:
> Hi Scott,
> 
> 
> > > It is really really painful to wait more than 10 seconds just for bad block
> > > scanning to boot Linux.
> > 
> > Making bad block scans faster is a good thing, but why do you need to
> > scan them just to boot Linux?  Aren't you using an on-flash BBT?
> 
> I did not know that.
> I thought all blocks must be scanned.
> 
> Could you teach me the better way?

If you use NAND_BBT_USE_FLASH, and NAND_BBT_CREATE is present in the bbt
descriptor (this is true of the default descriptors), then the scanning
should only need to happen on first use.  On subsequent boots only the
bad block table should need to be read.

-Scott

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-18 11:30 [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Masahiro Yamada
  2014-04-18 11:30 ` [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel Masahiro Yamada
  2014-04-21 22:21 ` [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Scott Wood
@ 2014-04-23  6:50 ` Chin Liang See
  2014-04-24 10:20   ` Masahiro Yamada
  2 siblings, 1 reply; 10+ messages in thread
From: Chin Liang See @ 2014-04-23  6:50 UTC (permalink / raw)
  To: u-boot

Hi Masahiro,

I am back :)
Sorry for late reply as I was aways for few weeks.


On Fri, 2014-04-18 at 20:30 +0900, Masahiro Yamada wrote:
> This patch is a review feedback against Denali NAND controller driver.
> http://patchwork.ozlabs.org/patch/333077/
> 
> This is not applicable to the mainline.
> 
>   ---
> 
> Hi Chin,
> 
> This patch fixes some issues.
> 
> [1] Fix denali_write_oob() handler.
> 
> As for v7, "nand markbad" did not work at all.
> 
> With this patch, it works.

Actually this was tested before.
Here is the test result (as this was written as SOCFPGA documentation)

SOCFPGA_CYCLONE5 # nand markbad 0
Bad block table written to 0x00003ffe0000, version 0x02
Bad block table written to 0x00003ffc0000, version 0x02
block 0x00000000 successfully marked as bad
SOCFPGA_CYCLONE5 # nand bad

Device 0 bad blocks:
00000000
3ff80000
3ffa0000
3ffc0000
3ffe0000
SOCFPGA_CYCLONE5 #


> 
> [2] Make denali_read_oob()  10x faster.
> 
> One of the fatal issues of v7 is "nand bad" command is extremely slow.
> 
> This is the benchmark of v7
> 
>   => time nand bad
> 
>   Device 0 bad blocks:
> 
>   time: 11.300 seconds, 11300 ticks
> 
> It is really really painful to wait more than 10 seconds just for bad block
> scanning to boot Linux.
> 
> In v7, denali_read_oob() calls denali_read_page_raw().
> This causes the transfering main area data and memcpy of it,
> which leads to significant performance regression.
> 
> Like Linux Kernel, dedicated denali_read_oob() must be impilemented.
> 
> With this patch, "nand bad" command gets much faster!
> 
> This is my benchmark:
> 
>   => time nand bad
> 
>   Device 0 bad blocks:
> 
>   time: 0.998 seconds, 998 ticks

I believe the bad block scanning would be required for new chip only.
This is when the bad block table is not existing within the NAND chip.
Once its available, the scanning would not be required as the BBT would
be loaded into memory. From there, the read and write would run much
much faster. Nevertheless, these code will help to speed up the bad
block scanning for the first time. I can apply this to v8 once I get
your feedback on other comments.

> 
> [3] Remove false comment
> 
>  /* Writes OOB data to the device.
>   * This code unused under normal U-Boot console as normally page write raw
>   * to be used for write oob data with main data.
>   */
>   static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
> 
> This comment is telling a lie.
> write_oob_data() is called from "nand markbad" command.
> It must be deleted.

Actually this comment is correct during integration into 2013 release. I
set the option to use NAND_USE_FLASH_BBT where it will not call
nand_do_write_oob. Do note that we will always write bad block info into
bad block table which are located at last 4 blocks of the NAND flash.

In latest code, the function "nand_do_write_oob" will be called if
write_oob is set (where NAND_BBT_NO_OOB_BBM is clear). From the
description of NAND_BBT_NO_OOB_BBM, user need to set it if we don't want
the bad block table located at OOB. 

For our case, it should be true as we are using HW ECC. Our data might
span into OOB region. Definitely we don't want this bad block table
overwritten our data then. Wonder is this match with your understanding.

In short, the function write_oob_data shouldn't be called for our case.

Thanks
Chin Liang

> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Cc: David Woodhouse <David.Woodhouse@intel.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/mtd/nand/denali.c | 136 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 91 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 348e244..dcde3e6 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -527,46 +527,34 @@ static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
>  static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
>  					int access_type, int op)
>  {
> -	uint32_t addr = 0x0, cmd = 0x0, irq_status = 0,	 irq_mask = 0;
> -	uint32_t page_count = 1;	/* always read a page */
> -
> -	if (op == DENALI_READ)
> -		irq_mask = INTR_STATUS__LOAD_COMP;
> -	else if (op == DENALI_WRITE)
> -		irq_mask = INTR_STATUS__PROGRAM_COMP |
> -			INTR_STATUS__PROGRAM_FAIL;
> -	else
> -		BUG();
> +	uint32_t addr = 0x0, cmd = 0x0, irq_status;
> +	static uint32_t page_count = 1;
> +
> +	setup_ecc_for_xfer(ecc_en, transfer_spare);
>  
>  	/* clear interrupts */
>  	clear_interrupts();
>  
> -	/* setup ECC and transfer spare reg */
> -	setup_ecc_for_xfer(ecc_en, transfer_spare);
> -
>  	addr = BANK(denali.flash_bank) | denali.page;
>  
>  	/* setup the acccess type */
>  	cmd = MODE_10 | addr;
> -	index_addr((uint32_t)cmd, access_type);
> +	index_addr(cmd, access_type);
>  
>  	/* setup the pipeline command */
> -	if (access_type == SPARE_ACCESS && op == DENALI_WRITE)
> -		index_addr((uint32_t)cmd, DENALI_BUFFER_WRITE);
> -	else if (access_type == SPARE_ACCESS && op == DENALI_READ)
> -		index_addr((uint32_t)cmd, DENALI_BUFFER_LOAD);
> -	else
> -		index_addr((uint32_t)cmd, 0x2000 | op | page_count);
> +	index_addr(cmd, 0x2000 | op | page_count);
>  
> -	/* wait for command to be accepted */
> -	irq_status = wait_for_irq(irq_mask);
> -	if ((irq_status & irq_mask) != irq_mask)
> -		return -EIO;
> +	cmd = MODE_01 | addr;
> +	writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
>  
> -	if (access_type != SPARE_ACCESS) {
> -		cmd = MODE_01 | addr;
> -		writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
> +	if (op == DENALI_READ) {
> +		/* wait for command to be accepted */
> +		irq_status = wait_for_irq(INTR_STATUS__LOAD_COMP);
> +
> +		if (irq_status == 0)
> +			return -EIO;
>  	}
> +
>  	return 0;
>  }
>  
> @@ -586,6 +574,29 @@ static int write_data_to_flash_mem(const void *buf, int len)
>  	return i * 4; /* intent is to return the number of bytes read */
>  }
>  
> +/* helper function that simply reads a buffer from the flash */
> +static int read_data_from_flash_mem(uint8_t *buf, int len)
> +{
> +	uint32_t i, *buf32;
> +
> +	/*
> +	 * we assume that len will be a multiple of 4, if not
> +	 * it would be nice to know about it ASAP rather than
> +	 * have random failures...
> +	 * This assumption is based on the fact that this
> +	 * function is designed to be used to read flash pages,
> +	 * which are typically multiples of 4...
> +	 */
> +
> +	BUG_ON((len % 4) != 0);
> +
> +	/* transfer the data from the flash */
> +	buf32 = (uint32_t *)buf;
> +	for (i = 0; i < len / 4; i++)
> +		*buf32++ = readl(denali.flash_mem + INDEX_DATA_REG);
> +	return i * 4; /* intent is to return the number of bytes read */
> +}
> +
>  static void denali_mode_main_access(void)
>  {
>  	uint32_t addr, cmd;
> @@ -602,29 +613,64 @@ static void denali_mode_main_spare_access(void)
>  	index_addr(cmd, MAIN_SPARE_ACCESS);
>  }
>  
> -/* Writes OOB data to the device.
> - * This code unused under normal U-Boot console as normally page write raw
> - * to be used for write oob data with main data.
> - */
> +/* writes OOB data to the device */
>  static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  {
> -	uint32_t cmd;
> +	uint32_t irq_status;
> +	uint32_t irq_mask = INTR_STATUS__PROGRAM_COMP |
> +						INTR_STATUS__PROGRAM_FAIL;
> +	int status = 0;
>  
>  	denali.page = page;
> -	debug("* write_oob_data *\n");
>  
> -	/* We need to write to buffer first through MAP00 command */
> -	cmd = MODE_00 | BANK(denali.flash_bank);
> -	writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
> +	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
> +							DENALI_WRITE) == 0) {
> +		write_data_to_flash_mem(buf, mtd->oobsize);
>  
> -	/* send the data into flash buffer */
> -	write_data_to_flash_mem(buf, mtd->oobsize);
> +		/* wait for operation to complete */
> +		irq_status = wait_for_irq(irq_mask);
>  
> -	/* activate the write through MAP10 commands */
> -	if (denali_send_pipeline_cmd(false, false, SPARE_ACCESS, DENALI_WRITE))
> -		return -EIO;
> +		if (irq_status == 0) {
> +			dev_err(denali->dev, "OOB write failed\n");
> +			status = -EIO;
> +		}
> +	} else {
> +		printf("unable to send pipeline command\n");
> +		status = -EIO;
> +	}
> +	return status;
> +}
>  
> -	return 0;
> +/* reads OOB data from the device */
> +static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
> +{
> +	uint32_t irq_mask = INTR_STATUS__LOAD_COMP,
> +			 irq_status = 0, addr = 0x0, cmd = 0x0;
> +
> +	denali.page = page;
> +
> +	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
> +							DENALI_READ) == 0) {
> +		read_data_from_flash_mem(buf, mtd->oobsize);
> +
> +		/* wait for command to be accepted
> +		 * can always use status0 bit as the mask is identical for each
> +		 * bank. */
> +		irq_status = wait_for_irq(irq_mask);
> +
> +		if (irq_status == 0)
> +			printf("page on OOB timeout %d\n", denali.page);
> +
> +		/* We set the device back to MAIN_ACCESS here as I observed
> +		 * instability with the controller if you do a block erase
> +		 * and the last transaction was a SPARE_ACCESS. Block erase
> +		 * is reliable (according to the MTD test infrastructure)
> +		 * if you are in MAIN_ACCESS.
> +		 */
> +		addr = BANK(denali.flash_bank) | denali.page;
> +		cmd = MODE_10 | addr;
> +		index_addr(cmd, MAIN_ACCESS);
> +	}
>  }
>  
>  /* this function examines buffers to see if they contain data that
> @@ -908,9 +954,9 @@ static uint8_t denali_read_byte(struct mtd_info *mtd)
>  static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  				int page)
>  {
> -	debug("denali_read_oob at page %08x\n", page);
> -	denali.page = page;
> -	return denali_read_page_raw(mtd, chip, denali.buf.buf, 1, page);
> +	read_oob_data(mtd, chip->oob_poi, page);
> +
> +	return 0;
>  }
>  
>  static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel
  2014-04-18 11:30 ` [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel Masahiro Yamada
@ 2014-04-23  7:02   ` Chin Liang See
  0 siblings, 0 replies; 10+ messages in thread
From: Chin Liang See @ 2014-04-23  7:02 UTC (permalink / raw)
  To: u-boot

Hi Masahiro,

On Fri, 2014-04-18 at 20:30 +0900, Masahiro Yamada wrote:
> This patch is a review feedback against Denali NAND controller driver.
> 
> http://patchwork.ozlabs.org/patch/333077/
> 
> This is not applicable to the mainline.
> 
>  --
> 
> This driver code has diverged too much from that of Linux Kernel.
> 
> The main cause was to drop "struct denali_nand_info *denali"
> from the most of functions and to replace "denali->foo" with "denali.foo".
> 
> But is it necessary?
> I think it just resulted in the difficulty of diffing and re-use of
> the Linux code.
> 
> This patch revives "struct denali_nand_info *denali" and "denali->foo".

Sure, definitely we can fix this too. 

I forget what is the reason I changed that previously. I suspect it
might because I try to prevent to use malloc. As this driver will be
used through the whole run time, it won't be free during the run. With
that, its much better to convert it to BSS. I can prompt the warning
during build time instead hitting error during run time :) I believe
kernel can do that as kernel can unload the module. Just my 2 cents
thought

Thanks
Chin Liang

> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Cc: David Woodhouse <David.Woodhouse@intel.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/mtd/nand/denali.c | 608 +++++++++++++++++++++++++---------------------
>  1 file changed, 327 insertions(+), 281 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index dcde3e6..565ca9e 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <common.h>
> +#include <malloc.h>
>  #include <nand.h>
>  #include <asm/errno.h>
>  #include <asm/io.h>
> @@ -15,7 +16,6 @@
>  
>  #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
> @@ -40,6 +40,12 @@ static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
>  
>  #define SUPPORT_8BITECC		1
>  
> +/*
> + * this macro allows us to convert from an MTD structure to our own
> + * device context (denali) structure.
> + */
> +#define mtd_to_denali(m) (((struct nand_chip *)mtd->priv)->priv)
> +
>  /* These constants are defined by the driver to enable common driver
>   * configuration options. */
>  #define SPARE_ACCESS		0x41
> @@ -66,44 +72,53 @@ static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
>  #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)
> +static inline void clear_interrupt(struct denali_nand_info *denali,
> +							uint32_t irq_mask)
>  {
> -	uint32_t intr_status_reg = 0;
> -	intr_status_reg = INTR_STATUS(denali.flash_bank);
> -	writel(irq_mask, denali.flash_reg + intr_status_reg);
> +	uint32_t intr_status_reg;
> +
> +	intr_status_reg = INTR_STATUS(denali->flash_bank);
> +
> +	writel(irq_mask, denali->flash_reg + intr_status_reg);
>  }
>  
> -static uint32_t read_interrupt_status(void)
> +static uint32_t read_interrupt_status(struct denali_nand_info *denali)
>  {
> -	uint32_t intr_status_reg = 0;
> -	intr_status_reg = INTR_STATUS(denali.flash_bank);
> -	return readl(denali.flash_reg + intr_status_reg);
> +	uint32_t intr_status_reg;
> +
> +	intr_status_reg = INTR_STATUS(denali->flash_bank);
> +
> +	return readl(denali->flash_reg + intr_status_reg);
>  }
>  
> -static void clear_interrupts(void)
> +static void clear_interrupts(struct denali_nand_info *denali)
>  {
> -	uint32_t status = 0;
> -	status = read_interrupt_status();
> -	clear_interrupt(status);
> -	denali.irq_status = 0;
> +	uint32_t status;
> +
> +	status = read_interrupt_status(denali);
> +	clear_interrupt(denali, status);
> +
> +	denali->irq_status = 0;
>  }
>  
> -static void denali_irq_enable(uint32_t int_mask)
> +static void denali_irq_enable(struct denali_nand_info *denali,
> +							uint32_t int_mask)
>  {
>  	int i;
> -	for (i = 0; i < denali.max_banks; ++i)
> -		writel(int_mask, denali.flash_reg + INTR_EN(i));
> +
> +	for (i = 0; i < denali->max_banks; ++i)
> +		writel(int_mask, denali->flash_reg + INTR_EN(i));
>  }
>  
> -static uint32_t wait_for_irq(uint32_t irq_mask)
> +static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
>  {
>  	unsigned long timeout = 1000000;
>  	uint32_t intr_status;
>  
>  	do {
> -		intr_status = read_interrupt_status() & DENALI_IRQ_ALL;
> +		intr_status = read_interrupt_status(denali) & DENALI_IRQ_ALL;
>  		if (intr_status & irq_mask) {
> -			denali.irq_status &= ~irq_mask;
> +			denali->irq_status &= ~irq_mask;
>  			/* our interrupt was detected */
>  			break;
>  		}
> @@ -114,7 +129,7 @@ static uint32_t wait_for_irq(uint32_t irq_mask)
>  	if (timeout == 0) {
>  		/* timeout */
>  		printf("Denali timeout with interrupt status %08x\n",
> -		       read_interrupt_status());
> +		       read_interrupt_status(denali));
>  		intr_status = 0;
>  	}
>  	return intr_status;
> @@ -126,80 +141,82 @@ static uint32_t wait_for_irq(uint32_t irq_mask)
>   * 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)
> +static void index_addr(struct denali_nand_info *denali,
> +				uint32_t address, uint32_t data)
>  {
> -	writel(address, denali.flash_mem + INDEX_CTRL_REG);
> -	writel(data, denali.flash_mem + INDEX_DATA_REG);
> +	writel(address, denali->flash_mem + INDEX_CTRL_REG);
> +	writel(data, denali->flash_mem + INDEX_DATA_REG);
>  }
>  
>  /* Perform an indexed read of the device */
> -static void index_addr_read_data(uint32_t address, uint32_t *pdata)
> +static void index_addr_read_data(struct denali_nand_info *denali,
> +				 uint32_t address, uint32_t *pdata)
>  {
> -	writel(address, denali.flash_mem + INDEX_CTRL_REG);
> -	*pdata = readl(denali.flash_mem + INDEX_DATA_REG);
> +	writel(address, denali->flash_mem + INDEX_CTRL_REG);
> +	*pdata = readl(denali->flash_mem + INDEX_DATA_REG);
>  }
>  
>  /* We need to buffer some data for some of the NAND core routines.
>   * The operations manage buffering that data. */
> -static void reset_buf(void)
> +static void reset_buf(struct denali_nand_info *denali)
>  {
> -	denali.buf.head = 0;
> -	denali.buf.tail = 0;
> +	denali->buf.head = denali->buf.tail = 0;
>  }
>  
> -static void write_byte_to_buf(uint8_t byte)
> +static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
>  {
> -	BUG_ON(denali.buf.tail >= sizeof(denali.buf.buf));
> -	denali.buf.buf[denali.buf.tail++] = byte;
> +	denali->buf.buf[denali->buf.tail++] = byte;
>  }
>  
>  /* resets a specific device connected to the core */
> -static void reset_bank(void)
> +static void reset_bank(struct denali_nand_info *denali)
>  {
>  	uint32_t irq_status;
>  	uint32_t irq_mask = INTR_STATUS__RST_COMP |
>  			    INTR_STATUS__TIME_OUT;
>  
> -	clear_interrupts();
> +	clear_interrupts(denali);
>  
> -	writel(1 << denali.flash_bank, denali.flash_reg + DEVICE_RESET);
> +	writel(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
>  
> -	irq_status = wait_for_irq(irq_mask);
> +	irq_status = wait_for_irq(denali, irq_mask);
>  	if (irq_status & INTR_STATUS__TIME_OUT)
>  		debug(KERN_ERR "reset bank failed.\n");
>  }
>  
>  /* Reset the flash controller */
> -static uint32_t denali_nand_reset(void)
> +static uint32_t denali_nand_reset(struct denali_nand_info *denali)
>  {
>  	uint32_t i;
>  
> -	for (i = 0; i < denali.max_banks; i++)
> +	for (i = 0; i < denali->max_banks; i++)
>  		writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
> -		       denali.flash_reg + INTR_STATUS(i));
> +		       denali->flash_reg + INTR_STATUS(i));
>  
> -	for (i = 0; i < denali.max_banks; i++) {
> -		writel(1 << i, denali.flash_reg + DEVICE_RESET);
> -		while (!(readl(denali.flash_reg +	INTR_STATUS(i)) &
> +	for (i = 0; i < denali->max_banks; i++) {
> +		writel(1 << i, denali->flash_reg + DEVICE_RESET);
> +		while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
>  			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
> -			if (readl(denali.flash_reg + INTR_STATUS(i)) &
> +			if (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++)
> +	for (i = 0; i < denali->max_banks; i++)
>  		writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
> -		       denali.flash_reg + INTR_STATUS(i));
> +		       denali->flash_reg + INTR_STATUS(i));
>  
>  	return 0;
>  }
>  
> -/* this routine calculates the ONFI timing values for a given mode and
> +/*
> + * 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(uint32_t mode)
> +static void nand_onfi_timing_set(struct denali_nand_info *denali,
> +								uint16_t mode)
>  {
>  	uint32_t trea[6] = {40, 30, 25, 20, 20, 16};
>  	uint32_t trp[6] = {50, 25, 17, 15, 12, 10};
> @@ -280,91 +297,93 @@ static void nand_onfi_timing_set(uint32_t mode)
>  #endif
>  
>  	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
> -	if ((readl(denali.flash_reg + MANUFACTURER_ID) == 0) &&
> -	    (readl(denali.flash_reg + DEVICE_ID) == 0x88))
> +	if ((readl(denali->flash_reg + MANUFACTURER_ID) == 0) &&
> +	    (readl(denali->flash_reg + DEVICE_ID) == 0x88))
>  		acc_clks = 6;
>  
> -	writel(acc_clks, denali.flash_reg + ACC_CLKS);
> -	writel(re_2_we, denali.flash_reg + RE_2_WE);
> -	writel(re_2_re, denali.flash_reg + RE_2_RE);
> -	writel(we_2_re, denali.flash_reg + WE_2_RE);
> -	writel(addr_2_data, denali.flash_reg + ADDR_2_DATA);
> -	writel(en_lo, denali.flash_reg + RDWR_EN_LO_CNT);
> -	writel(en_hi, denali.flash_reg + RDWR_EN_HI_CNT);
> -	writel(cs_cnt, denali.flash_reg + CS_SETUP_CNT);
> +	writel(acc_clks, denali->flash_reg + ACC_CLKS);
> +	writel(re_2_we, denali->flash_reg + RE_2_WE);
> +	writel(re_2_re, denali->flash_reg + RE_2_RE);
> +	writel(we_2_re, denali->flash_reg + WE_2_RE);
> +	writel(addr_2_data, denali->flash_reg + ADDR_2_DATA);
> +	writel(en_lo, denali->flash_reg + RDWR_EN_LO_CNT);
> +	writel(en_hi, denali->flash_reg + RDWR_EN_HI_CNT);
> +	writel(cs_cnt, denali->flash_reg + CS_SETUP_CNT);
>  }
>  
>  /* queries the NAND device to see what ONFI modes it supports. */
> -static uint32_t get_onfi_nand_para(void)
> +static uint32_t get_onfi_nand_para(struct denali_nand_info *denali)
>  {
>  	int i;
> -	/* we needn't to do a reset here because driver has already
> +	/*
> +	 * we needn't to do a reset here because driver has already
>  	 * reset all the banks before
> -	 * */
> -	if (!(readl(denali.flash_reg + ONFI_TIMING_MODE) &
> +	 */
> +	if (!(readl(denali->flash_reg + ONFI_TIMING_MODE) &
>  	    ONFI_TIMING_MODE__VALUE))
>  		return -EIO;
>  
>  	for (i = 5; i > 0; i--) {
> -		if (readl(denali.flash_reg + ONFI_TIMING_MODE) &
> +		if (readl(denali->flash_reg + ONFI_TIMING_MODE) &
>  			(0x01 << i))
>  			break;
>  	}
>  
> -	nand_onfi_timing_set(i);
> +	nand_onfi_timing_set(denali, i);
>  
>  	/* By now, all the ONFI devices we know support the page cache */
>  	/* rw feature. So here we enable the pipeline_rw_ahead feature */
>  	return 0;
>  }
>  
> -static void get_samsung_nand_para(uint8_t device_id)
> +static void get_samsung_nand_para(struct denali_nand_info *denali,
> +							uint8_t device_id)
>  {
>  	if (device_id == 0xd3) { /* Samsung K9WAG08U1A */
>  		/* Set timing register values according to datasheet */
> -		writel(5, denali.flash_reg + ACC_CLKS);
> -		writel(20, denali.flash_reg + RE_2_WE);
> -		writel(12, denali.flash_reg + WE_2_RE);
> -		writel(14, denali.flash_reg + ADDR_2_DATA);
> -		writel(3, denali.flash_reg + RDWR_EN_LO_CNT);
> -		writel(2, denali.flash_reg + RDWR_EN_HI_CNT);
> -		writel(2, denali.flash_reg + CS_SETUP_CNT);
> +		writel(5, denali->flash_reg + ACC_CLKS);
> +		writel(20, denali->flash_reg + RE_2_WE);
> +		writel(12, denali->flash_reg + WE_2_RE);
> +		writel(14, denali->flash_reg + ADDR_2_DATA);
> +		writel(3, denali->flash_reg + RDWR_EN_LO_CNT);
> +		writel(2, denali->flash_reg + RDWR_EN_HI_CNT);
> +		writel(2, denali->flash_reg + CS_SETUP_CNT);
>  	}
>  }
>  
> -static void get_toshiba_nand_para(void)
> +static void get_toshiba_nand_para(struct denali_nand_info *denali)
>  {
>  	uint32_t tmp;
>  
>  	/* Workaround to fix a controller bug which reports a wrong */
>  	/* spare area size for some kind of Toshiba NAND device */
> -	if ((readl(denali.flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
> -	    (readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE)
> -		== 64)){
> -		writel(216, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
> -		tmp = readl(denali.flash_reg + DEVICES_CONNECTED) *
> -			readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
> -		writel(tmp, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
> +	if ((readl(denali->flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
> +	    (readl(denali->flash_reg + DEVICE_SPARE_AREA_SIZE) == 64)) {
> +		writel(216, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
> +		tmp = readl(denali->flash_reg + DEVICES_CONNECTED) *
> +			readl(denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
> +		writel(tmp, denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
>  	}
>  }
>  
> -static void get_hynix_nand_para(uint8_t device_id)
> +static void get_hynix_nand_para(struct denali_nand_info *denali,
> +							uint8_t device_id)
>  {
>  	uint32_t main_size, spare_size;
>  
>  	switch (device_id) {
>  	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
>  	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
> -		writel(128, denali.flash_reg + PAGES_PER_BLOCK);
> -		writel(4096, denali.flash_reg + DEVICE_MAIN_AREA_SIZE);
> -		writel(224, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
> +		writel(128, denali->flash_reg + PAGES_PER_BLOCK);
> +		writel(4096, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
> +		writel(224, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
>  		main_size = 4096 *
> -			readl(denali.flash_reg + DEVICES_CONNECTED);
> +			readl(denali->flash_reg + DEVICES_CONNECTED);
>  		spare_size = 224 *
> -			readl(denali.flash_reg + DEVICES_CONNECTED);
> -		writel(main_size, denali.flash_reg + LOGICAL_PAGE_DATA_SIZE);
> -		writel(spare_size, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
> -		writel(0, denali.flash_reg + DEVICE_WIDTH);
> +			readl(denali->flash_reg + DEVICES_CONNECTED);
> +		writel(main_size, denali->flash_reg + LOGICAL_PAGE_DATA_SIZE);
> +		writel(spare_size, denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
> +		writel(0, denali->flash_reg + DEVICE_WIDTH);
>  		break;
>  	default:
>  		debug(KERN_WARNING
> @@ -374,27 +393,28 @@ static void get_hynix_nand_para(uint8_t device_id)
>  	}
>  }
>  
> -/* determines how many NAND chips are connected to the controller. Note for
> +/*
> + * 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)
> +static void find_valid_banks(struct denali_nand_info *denali)
>  {
> -	uint32_t id[denali.max_banks];
> +	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);
> -		index_addr_read_data((uint32_t)(MODE_11 | (i << 24) | 2),
> -				     &id[i]);
> +	denali->total_used_banks = 1;
> +	for (i = 0; i < denali->max_banks; i++) {
> +		index_addr(denali, (uint32_t)(MODE_11 | (i << 24) | 0), 0x90);
> +		index_addr(denali, (uint32_t)(MODE_11 | (i << 24) | 1), 0);
> +		index_addr_read_data(denali,
> +				(uint32_t)(MODE_11 | (i << 24) | 2), &id[i]);
>  
>  		if (i == 0) {
>  			if (!(id[i] & 0x0ff))
>  				break;
>  		} else {
>  			if ((id[i] & 0x0ff) == (id[0] & 0x0ff))
> -				denali.total_used_banks++;
> +				denali->total_used_banks++;
>  			else
>  				break;
>  		}
> @@ -405,39 +425,40 @@ static void find_valid_banks(void)
>   * Use the configuration feature register to determine the maximum number of
>   * banks that the hardware supports.
>   */
> -static void detect_max_banks(void)
> +static void detect_max_banks(struct denali_nand_info *denali)
>  {
> -	uint32_t features = readl(denali.flash_reg + FEATURES);
> -	denali.max_banks = 2 << (features & FEATURES__N_BANKS);
> +	uint32_t features = readl(denali->flash_reg + FEATURES);
> +	denali->max_banks = 2 << (features & FEATURES__N_BANKS);
>  }
>  
> -static void detect_partition_feature(void)
> +static void detect_partition_feature(struct denali_nand_info *denali)
>  {
> -	/* For MRST platform, denali.fwblks represent the
> +	/*
> +	 * 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 (readl(denali.flash_reg + FEATURES) & FEATURES__PARTITION) {
> -		if ((readl(denali.flash_reg + PERM_SRC_ID(1)) &
> +	 */
> +	if (readl(denali->flash_reg + FEATURES) & FEATURES__PARTITION) {
> +		if ((readl(denali->flash_reg + PERM_SRC_ID(1)) &
>  			PERM_SRC_ID__SRCID) == SPECTRA_PARTITION_ID) {
> -			denali.fwblks =
> -			    ((readl(denali.flash_reg + MIN_MAX_BANK(1)) &
> +			denali->fwblks =
> +			    ((readl(denali->flash_reg + MIN_MAX_BANK(1)) &
>  			      MIN_MAX_BANK__MIN_VALUE) *
> -			     denali.blksperchip)
> +			     denali->blksperchip)
>  			    +
> -			    (readl(denali.flash_reg + MIN_BLK_ADDR(1)) &
> +			    (readl(denali->flash_reg + MIN_BLK_ADDR(1)) &
>  			    MIN_BLK_ADDR__VALUE);
>  		} else {
> -			denali.fwblks = SPECTRA_START_BLOCK;
> +			denali->fwblks = SPECTRA_START_BLOCK;
>  		}
>  	} else {
> -		denali.fwblks = SPECTRA_START_BLOCK;
> +		denali->fwblks = SPECTRA_START_BLOCK;
>  	}
>  }
>  
> -static uint32_t denali_nand_timing_set(void)
> +static uint32_t denali_nand_timing_set(struct denali_nand_info *denali)
>  {
>  	uint32_t id_bytes[5], addr;
>  	uint8_t i, maf_id, device_id;
> @@ -447,35 +468,35 @@ static uint32_t denali_nand_timing_set(void)
>  	 * report the correct device ID by reading from
>  	 * DEVICE_ID register
>  	 * */
> -	addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
> -	index_addr((uint32_t)addr | 0, 0x90);
> -	index_addr((uint32_t)addr | 1, 0);
> +	addr = (uint32_t)MODE_11 | BANK(denali->flash_bank);
> +	index_addr(denali, (uint32_t)addr | 0, 0x90);
> +	index_addr(denali, (uint32_t)addr | 1, 0);
>  	for (i = 0; i < 5; i++)
> -		index_addr_read_data(addr | 2, &id_bytes[i]);
> +		index_addr_read_data(denali, addr | 2, &id_bytes[i]);
>  	maf_id = id_bytes[0];
>  	device_id = id_bytes[1];
>  
> -	if (readl(denali.flash_reg + ONFI_DEVICE_NO_OF_LUNS) &
> +	if (readl(denali->flash_reg + ONFI_DEVICE_NO_OF_LUNS) &
>  		ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE) { /* ONFI 1.0 NAND */
> -		if (get_onfi_nand_para())
> +		if (get_onfi_nand_para(denali))
>  			return -EIO;
>  	} else if (maf_id == 0xEC) { /* Samsung NAND */
> -		get_samsung_nand_para(device_id);
> +		get_samsung_nand_para(denali, device_id);
>  	} else if (maf_id == 0x98) { /* Toshiba NAND */
> -		get_toshiba_nand_para();
> +		get_toshiba_nand_para(denali);
>  	} else if (maf_id == 0xAD) { /* Hynix NAND */
> -		get_hynix_nand_para(device_id);
> +		get_hynix_nand_para(denali, device_id);
>  	}
>  
> -	find_valid_banks();
> +	find_valid_banks(denali);
>  
> -	detect_partition_feature();
> +	detect_partition_feature(denali);
>  
>  	/* If the user specified to override the default timings
>  	 * with a specific ONFI mode, we apply those changes here.
>  	 */
>  	if (onfi_timing_mode != NAND_DEFAULT_TIMINGS)
> -		nand_onfi_timing_set(onfi_timing_mode);
> +		nand_onfi_timing_set(denali, onfi_timing_mode);
>  
>  	return 0;
>  }
> @@ -488,26 +509,27 @@ static inline bool is_flash_bank_valid(int flash_bank)
>  	return (flash_bank >= 0 && flash_bank < 4);
>  }
>  
> -static void denali_irq_init(void)
> +static void denali_irq_init(struct denali_nand_info *denali)
>  {
>  	uint32_t int_mask = 0;
>  	int i;
>  
>  	/* Disable global interrupts */
> -	writel(0, denali.flash_reg + GLOBAL_INT_ENABLE);
> +	writel(0, denali->flash_reg + GLOBAL_INT_ENABLE);
>  
>  	int_mask = DENALI_IRQ_ALL;
>  
>  	/* Clear all status bits */
> -	for (i = 0; i < denali.max_banks; ++i)
> -		writel(0xFFFF, denali.flash_reg + INTR_STATUS(i));
> +	for (i = 0; i < denali->max_banks; ++i)
> +		writel(0xFFFF, denali->flash_reg + INTR_STATUS(i));
>  
> -	denali_irq_enable(int_mask);
> +	denali_irq_enable(denali, int_mask);
>  }
>  
>  /* This helper function setups the registers for ECC and whether or not
>   * the spare area will be transferred. */
> -static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
> +static void setup_ecc_for_xfer(struct denali_nand_info *denali, bool ecc_en,
> +				bool transfer_spare)
>  {
>  	int ecc_en_flag = 0, transfer_spare_flag = 0;
>  
> @@ -516,40 +538,41 @@ static void setup_ecc_for_xfer(bool ecc_en, bool transfer_spare)
>  	transfer_spare_flag = transfer_spare ? TRANSFER_SPARE_REG__FLAG : 0;
>  
>  	/* Enable spare area/ECC per user's request. */
> -	writel(ecc_en_flag, denali.flash_reg + ECC_ENABLE);
> +	writel(ecc_en_flag, denali->flash_reg + ECC_ENABLE);
>  	/* applicable for MAP01 only */
> -	writel(transfer_spare_flag, denali.flash_reg + TRANSFER_SPARE_REG);
> +	writel(transfer_spare_flag, denali->flash_reg + TRANSFER_SPARE_REG);
>  }
>  
>  /* sends a pipeline command operation to the controller. See the Denali NAND
>   * controller's user guide for more information (section 4.2.3.6).
>   */
> -static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
> +static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
> +					bool ecc_en, bool transfer_spare,
>  					int access_type, int op)
>  {
> -	uint32_t addr = 0x0, cmd = 0x0, irq_status;
> +	uint32_t addr, cmd, irq_status;
>  	static uint32_t page_count = 1;
>  
> -	setup_ecc_for_xfer(ecc_en, transfer_spare);
> +	setup_ecc_for_xfer(denali, ecc_en, transfer_spare);
>  
>  	/* clear interrupts */
> -	clear_interrupts();
> +	clear_interrupts(denali);
>  
> -	addr = BANK(denali.flash_bank) | denali.page;
> +	addr = BANK(denali->flash_bank) | denali->page;
>  
>  	/* setup the acccess type */
>  	cmd = MODE_10 | addr;
> -	index_addr(cmd, access_type);
> +	index_addr(denali, cmd, access_type);
>  
>  	/* setup the pipeline command */
> -	index_addr(cmd, 0x2000 | op | page_count);
> +	index_addr(denali, cmd, 0x2000 | op | page_count);
>  
>  	cmd = MODE_01 | addr;
> -	writel(cmd, denali.flash_mem + INDEX_CTRL_REG);
> +	writel(cmd, denali->flash_mem + INDEX_CTRL_REG);
>  
>  	if (op == DENALI_READ) {
>  		/* wait for command to be accepted */
> -		irq_status = wait_for_irq(INTR_STATUS__LOAD_COMP);
> +		irq_status = wait_for_irq(denali, INTR_STATUS__LOAD_COMP);
>  
>  		if (irq_status == 0)
>  			return -EIO;
> @@ -559,7 +582,8 @@ static int denali_send_pipeline_cmd(bool ecc_en, bool transfer_spare,
>  }
>  
>  /* helper function that simply writes a buffer to the flash */
> -static int write_data_to_flash_mem(const void *buf, int len)
> +static int write_data_to_flash_mem(struct denali_nand_info *denali,
> +						const uint8_t *buf, int len)
>  {
>  	uint32_t i = 0, *buf32;
>  
> @@ -570,12 +594,13 @@ static int write_data_to_flash_mem(const void *buf, int len)
>  	/* write the data to the flash memory */
>  	buf32 = (uint32_t *)buf;
>  	for (i = 0; i < len / 4; i++)
> -		writel(*buf32++, denali.flash_mem + INDEX_DATA_REG);
> +		writel(*buf32++, denali->flash_mem + INDEX_DATA_REG);
>  	return i * 4; /* intent is to return the number of bytes read */
>  }
>  
>  /* helper function that simply reads a buffer from the flash */
> -static int read_data_from_flash_mem(uint8_t *buf, int len)
> +static int read_data_from_flash_mem(struct denali_nand_info *denali,
> +						uint8_t *buf, int len)
>  {
>  	uint32_t i, *buf32;
>  
> @@ -593,42 +618,46 @@ static int read_data_from_flash_mem(uint8_t *buf, int len)
>  	/* transfer the data from the flash */
>  	buf32 = (uint32_t *)buf;
>  	for (i = 0; i < len / 4; i++)
> -		*buf32++ = readl(denali.flash_mem + INDEX_DATA_REG);
> +		*buf32++ = readl(denali->flash_mem + INDEX_DATA_REG);
> +
>  	return i * 4; /* intent is to return the number of bytes read */
>  }
>  
> -static void denali_mode_main_access(void)
> +static void denali_mode_main_access(struct denali_nand_info *denali)
>  {
>  	uint32_t addr, cmd;
> -	addr = BANK(denali.flash_bank) | denali.page;
> +
> +	addr = BANK(denali->flash_bank) | denali->page;
>  	cmd = MODE_10 | addr;
> -	index_addr(cmd, MAIN_ACCESS);
> +	index_addr(denali, cmd, MAIN_ACCESS);
>  }
>  
> -static void denali_mode_main_spare_access(void)
> +static void denali_mode_main_spare_access(struct denali_nand_info *denali)
>  {
>  	uint32_t addr, cmd;
> -	addr = BANK(denali.flash_bank) | denali.page;
> +
> +	addr = BANK(denali->flash_bank) | denali->page;
>  	cmd = MODE_10 | addr;
> -	index_addr(cmd, MAIN_SPARE_ACCESS);
> +	index_addr(denali, cmd, MAIN_SPARE_ACCESS);
>  }
>  
>  /* writes OOB data to the device */
>  static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
>  	uint32_t irq_status;
>  	uint32_t irq_mask = INTR_STATUS__PROGRAM_COMP |
>  						INTR_STATUS__PROGRAM_FAIL;
>  	int status = 0;
>  
> -	denali.page = page;
> +	denali->page = page;
>  
> -	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
> +	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
>  							DENALI_WRITE) == 0) {
> -		write_data_to_flash_mem(buf, mtd->oobsize);
> +		write_data_to_flash_mem(denali, buf, mtd->oobsize);
>  
>  		/* wait for operation to complete */
> -		irq_status = wait_for_irq(irq_mask);
> +		irq_status = wait_for_irq(denali, irq_mask);
>  
>  		if (irq_status == 0) {
>  			dev_err(denali->dev, "OOB write failed\n");
> @@ -644,22 +673,23 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  /* reads OOB data from the device */
>  static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
>  	uint32_t irq_mask = INTR_STATUS__LOAD_COMP,
>  			 irq_status = 0, addr = 0x0, cmd = 0x0;
>  
> -	denali.page = page;
> +	denali->page = page;
>  
> -	if (denali_send_pipeline_cmd(false, true, SPARE_ACCESS,
> +	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
>  							DENALI_READ) == 0) {
> -		read_data_from_flash_mem(buf, mtd->oobsize);
> +		read_data_from_flash_mem(denali, buf, mtd->oobsize);
>  
>  		/* wait for command to be accepted
>  		 * can always use status0 bit as the mask is identical for each
>  		 * bank. */
> -		irq_status = wait_for_irq(irq_mask);
> +		irq_status = wait_for_irq(denali, irq_mask);
>  
>  		if (irq_status == 0)
> -			printf("page on OOB timeout %d\n", denali.page);
> +			printf("page on OOB timeout %d\n", denali->page);
>  
>  		/* We set the device back to MAIN_ACCESS here as I observed
>  		 * instability with the controller if you do a block erase
> @@ -667,16 +697,16 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  		 * is reliable (according to the MTD test infrastructure)
>  		 * if you are in MAIN_ACCESS.
>  		 */
> -		addr = BANK(denali.flash_bank) | denali.page;
> +		addr = BANK(denali->flash_bank) | denali->page;
>  		cmd = MODE_10 | addr;
> -		index_addr(cmd, MAIN_ACCESS);
> +		index_addr(denali, cmd, MAIN_ACCESS);
>  	}
>  }
>  
>  /* this function examines buffers to see if they contain data that
>   * indicate that the buffer is part of an erased region of flash.
>   */
> -bool is_erased(uint8_t *buf, int len)
> +static bool is_erased(uint8_t *buf, int len)
>  {
>  	int i = 0;
>  	for (i = 0; i < len; i++)
> @@ -685,80 +715,80 @@ bool is_erased(uint8_t *buf, int len)
>  	return true;
>  }
>  
> -
>  /* programs the controller to either enable/disable DMA transfers */
> -static void denali_enable_dma(bool en)
> +static void denali_enable_dma(struct denali_nand_info *denali, bool en)
>  {
>  	uint32_t reg_val = 0x0;
>  
>  	if (en)
>  		reg_val = DMA_ENABLE__FLAG;
>  
> -	writel(reg_val, denali.flash_reg + DMA_ENABLE);
> -	readl(denali.flash_reg + DMA_ENABLE);
> +	writel(reg_val, denali->flash_reg + DMA_ENABLE);
> +	readl(denali->flash_reg + DMA_ENABLE);
>  }
>  
>  /* setups the HW to perform the data DMA */
> -static void denali_setup_dma(int op)
> +static void denali_setup_dma(struct denali_nand_info *denali, int op)
>  {
> -	const int page_count = 1;
>  	uint32_t mode;
> -	uint32_t addr = (uint32_t)denali.buf.dma_buf;
> +	const int page_count = 1;
> +	uint32_t addr = (uint32_t)denali->buf.dma_buf;
>  
> -	flush_dcache_range(addr, addr + sizeof(denali.buf.dma_buf));
> +	flush_dcache_range(addr, addr + sizeof(denali->buf.dma_buf));
>  
>  #ifdef CONFIG_NAND_DENALI_64BIT
> -	mode = MODE_10 | BANK(denali.flash_bank) | denali.page;
> +	mode = MODE_10 | BANK(denali->flash_bank) | denali->page;
>  
>  	/* DMA is a three step process */
>  
>  	/* 1. setup transfer type, interrupt when complete,
>  	      burst len = 64 bytes, the number of pages */
> -	index_addr(mode, 0x01002000 | (64 << 16) | op | page_count);
> +	index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
>  
>  	/* 2. set memory low address bits 31:0 */
> -	index_addr(mode, addr);
> +	index_addr(denali, mode, addr);
>  
>  	/* 3. set memory high address bits 64:32 */
> -	index_addr(mode, 0);
> +	index_addr(denali, mode, 0);
>  #else
> -	mode = MODE_10 | BANK(denali.flash_bank);
> +	mode = MODE_10 | BANK(denali->flash_bank);
>  
>  	/* DMA is a four step process */
>  
>  	/* 1. setup transfer type and # of pages */
> -	index_addr(mode | denali.page, 0x2000 | op | page_count);
> +	index_addr(denali, mode | denali->page, 0x2000 | op | page_count);
>  
>  	/* 2. set memory high address bits 23:8 */
> -	index_addr(mode | ((uint32_t)(addr >> 16) << 8), 0x2200);
> +	index_addr(denali, mode | ((uint32_t)(addr >> 16) << 8), 0x2200);
>  
>  	/* 3. set memory low address bits 23:8 */
> -	index_addr(mode | ((uint32_t)addr << 8), 0x2300);
> +	index_addr(denali, mode | ((uint32_t)addr << 8), 0x2300);
>  
>  	/* 4.  interrupt when complete, burst len = 64 bytes*/
> -	index_addr(mode | 0x14000, 0x2400);
> +	index_addr(denali, mode | 0x14000, 0x2400);
>  #endif
>  }
>  
>  /* Common DMA function */
> -static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
> +static uint32_t denali_dma_configuration(struct denali_nand_info *denali,
> +					 uint32_t ops, bool raw_xfer,
>  					 uint32_t irq_mask, int oob_required)
>  {
>  	uint32_t irq_status = 0;
>  	/* setup_ecc_for_xfer(bool ecc_en, bool transfer_spare) */
> -	setup_ecc_for_xfer(!raw_xfer, oob_required);
> +	setup_ecc_for_xfer(denali, !raw_xfer, oob_required);
>  
>  	/* clear any previous interrupt flags */
> -	clear_interrupts();
> +	clear_interrupts(denali);
>  
>  	/* enable the DMA */
> -	denali_enable_dma(true);
> +	denali_enable_dma(denali, true);
>  
>  	/* setup the DMA */
> -	denali_setup_dma(ops);
> +	denali_setup_dma(denali, ops);
>  
>  	/* wait for operation to complete */
> -	irq_status = wait_for_irq(irq_mask);
> +	irq_status = wait_for_irq(denali, irq_mask);
>  
>  	/* if ECC fault happen, seems we need delay before turning off DMA.
>  	 * If not, the controller will go into non responsive condition */
> @@ -766,7 +796,7 @@ static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
>  		udelay(100);
>  
>  	/* disable the DMA */
> -	denali_enable_dma(false);
> +	denali_enable_dma(denali, false);
>  
>  	return irq_status;
>  }
> @@ -774,33 +804,35 @@ static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
>  static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			const uint8_t *buf, bool raw_xfer, int oob_required)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +
>  	uint32_t irq_status = 0;
>  	uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP;
>  
> -	denali.status = 0;
> +	denali->status = 0;
>  
>  	/* copy buffer into DMA buffer */
> -	memcpy((void *)denali.buf.dma_buf, buf, mtd->writesize);
> +	memcpy((void *)denali->buf.dma_buf, buf, mtd->writesize);
>  
>  	/* need extra memcpoy for raw transfer */
>  	if (raw_xfer)
> -		memcpy((void *)denali.buf.dma_buf + mtd->writesize,
> +		memcpy((void *)denali->buf.dma_buf + mtd->writesize,
>  		       chip->oob_poi, mtd->oobsize);
>  
>  	/* setting up DMA */
> -	irq_status = denali_dma_configuration(DENALI_WRITE, raw_xfer, irq_mask,
> -						oob_required);
> +	irq_status = denali_dma_configuration(denali, DENALI_WRITE, raw_xfer,
> +					      irq_mask, oob_required);
>  
>  	/* if timeout happen, error out */
>  	if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
>  		debug("DMA timeout for denali write_page\n");
> -		denali.status = NAND_STATUS_FAIL;
> +		denali->status = NAND_STATUS_FAIL;
>  		return -EIO;
>  	}
>  
>  	if (irq_status & INTR_STATUS__LOCKED_BLK) {
>  		debug("Failed as write to locked block\n");
> -		denali.status = NAND_STATUS_FAIL;
> +		denali->status = NAND_STATUS_FAIL;
>  		return -EIO;
>  	}
>  	return 0;
> @@ -816,18 +848,18 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  				const uint8_t *buf, int oob_required)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +
>  	/*
>  	 * for regular page writes, we let HW handle all the ECC
>  	 * data written to the device.
>  	 */
> -	debug("denali_write_page at page %08x\n", denali.page);
> -
>  	if (oob_required)
>  		/* switch to main + spare access */
> -		denali_mode_main_spare_access();
> +		denali_mode_main_spare_access(denali);
>  	else
>  		/* switch to main access only */
> -		denali_mode_main_access();
> +		denali_mode_main_access(denali);
>  
>  	return write_page(mtd, chip, buf, false, oob_required);
>  }
> @@ -840,18 +872,19 @@ static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  					const uint8_t *buf, int oob_required)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +
>  	/*
>  	 * for raw page writes, we want to disable ECC and simply write
>  	 * whatever data is in the buffer.
>  	 */
> -	debug("denali_write_page_raw at page %08x\n", denali.page);
>  
>  	if (oob_required)
>  		/* switch to main + spare access */
> -		denali_mode_main_spare_access();
> +		denali_mode_main_spare_access(denali);
>  	else
>  		/* switch to main access only */
> -		denali_mode_main_access();
> +		denali_mode_main_access(denali);
>  
>  	return write_page(mtd, chip, buf, true, oob_required);
>  }
> @@ -866,24 +899,25 @@ static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  				uint8_t *buf, int oob_required, int page)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +
>  	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) {
> +	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();
> +		denali_mode_main_spare_access(denali);
>  	else
>  		/* switch to main access only */
> -		denali_mode_main_access();
> +		denali_mode_main_access(denali);
>  
>  	/* setting up the DMA where ecc_enable is false */
> -	irq_status = denali_dma_configuration(DENALI_READ, true, irq_mask,
> -						oob_required);
> +	irq_status = denali_dma_configuration(denali, DENALI_READ, true,
> +					      irq_mask, oob_required);
>  
>  	/* if timeout happen, error out */
>  	if (!(irq_status & INTR_STATUS__DMA_CMD_COMP)) {
> @@ -892,44 +926,42 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  	}
>  
>  	/* splitting the content to destination buffer holder */
> -	memcpy(chip->oob_poi, (denali.buf.dma_buf + mtd->writesize),
> +	memcpy(chip->oob_poi, (denali->buf.dma_buf + mtd->writesize),
>  	       mtd->oobsize);
> -	memcpy(buf, denali.buf.dma_buf, mtd->writesize);
> -	debug("buf %02x %02x\n", buf[0], buf[1]);
> -	debug("chip->oob_poi %02x %02x\n", chip->oob_poi[0], chip->oob_poi[1]);
> +	memcpy(buf, denali->buf.dma_buf, mtd->writesize);
> +
>  	return 0;
>  }
>  
>  static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  				uint8_t *buf, int oob_required, int page)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
>  	uint32_t irq_status, irq_mask =	INTR_STATUS__DMA_CMD_COMP;
>  
> -	debug("denali_read_page at page %08x\n", page);
> -	if (denali.page != 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();
> +		denali_mode_main_spare_access(denali);
>  	else
>  		/* switch to main access only */
> -		denali_mode_main_access();
> +		denali_mode_main_access(denali);
>  
>  	/* setting up the DMA where ecc_enable is true */
> -	irq_status = denali_dma_configuration(DENALI_READ, false, irq_mask,
> -						oob_required);
> +	irq_status = denali_dma_configuration(denali, DENALI_READ, false,
> +					      irq_mask, oob_required);
>  
> -	memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
> -	debug("buf %02x %02x\n", buf[0], buf[1]);
> +	memcpy(buf, (const void *)denali->buf.dma_buf, mtd->writesize);
>  
>  	/* check whether any ECC error */
>  	if (irq_status & INTR_STATUS__ECC_UNCOR_ERR) {
>  		/* is the ECC cause by erase page, check using read_page_raw */
>  		debug("  Uncorrected ECC detected\n");
> -		denali_read_page_raw(mtd, chip, buf, oob_required, denali.page);
> +		denali_read_page_raw(mtd, chip, buf, oob_required, denali->page);
>  
>  		if (is_erased(buf, mtd->writesize) == true &&
>  		    is_erased(chip->oob_poi, mtd->oobsize) == true) {
> @@ -939,18 +971,10 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			return -EIO;
>  		}
>  	}
> -	memcpy(buf, (const void *)denali.buf.dma_buf, mtd->writesize);
> +	memcpy(buf, (const void *)denali->buf.dma_buf, mtd->writesize);
>  	return 0;
>  }
>  
> -static uint8_t denali_read_byte(struct mtd_info *mtd)
> -{
> -	uint32_t addr, result;
> -	addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
> -	index_addr_read_data((uint32_t)addr | 2, &result);
> -	return (uint8_t)result & 0xFF;
> -}
> -
>  static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  				int page)
>  {
> @@ -959,91 +983,105 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  	return 0;
>  }
>  
> +static uint8_t denali_read_byte(struct mtd_info *mtd)
> +{
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +	uint32_t addr, result;
> +
> +	addr = (uint32_t)MODE_11 | BANK(denali->flash_bank);
> +	index_addr_read_data(denali, addr | 2, &result);
> +	return (uint8_t)result & 0xFF;
> +}
> +
>  static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
>  	uint32_t i, addr, result;
>  
>  	/* delay for tR (data transfer from Flash array to data register) */
>  	udelay(25);
>  
>  	/* ensure device completed else additional delay and polling */
> -	wait_for_irq(INTR_STATUS__INT_ACT);
> +	wait_for_irq(denali, INTR_STATUS__INT_ACT);
>  
> -	addr = (uint32_t)MODE_11 | BANK(denali.flash_bank);
> +	addr = (uint32_t)MODE_11 | BANK(denali->flash_bank);
>  	for (i = 0; i < len; i++) {
> -		index_addr_read_data((uint32_t)addr | 2, &result);
> -		write_byte_to_buf(result);
> +		index_addr_read_data(denali, (uint32_t)addr | 2, &result);
> +		write_byte_to_buf(denali, result);
>  	}
> -	memcpy(buf, denali.buf.buf, len);
> +	memcpy(buf, denali->buf.buf, len);
>  }
>  
>  static void denali_select_chip(struct mtd_info *mtd, int chip)
>  {
> -	denali.flash_bank = chip;
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +
> +	denali->flash_bank = chip;
>  }
>  
>  static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
>  {
> -	int status = denali.status;
> -	denali.status = 0;
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +	int status = denali->status;
> +	denali->status = 0;
>  
>  	return status;
>  }
>  
>  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);
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
> +	uint32_t cmd, irq_status;
>  
>  	/* clear interrupts */
> -	clear_interrupts();
> +	clear_interrupts(denali);
>  
>  	/* setup page read request for access type */
> -	cmd = MODE_10 | BANK(denali.flash_bank) | page;
> -	index_addr((uint32_t)cmd, 0x1);
> +	cmd = MODE_10 | BANK(denali->flash_bank) | page;
> +	index_addr(denali, cmd, 0x1);
>  
>  	/* wait for erase to complete or failure to occur */
> -	irq_status = wait_for_irq(INTR_STATUS__ERASE_COMP |
> -		INTR_STATUS__ERASE_FAIL);
> +	irq_status = wait_for_irq(denali, INTR_STATUS__ERASE_COMP |
> +					INTR_STATUS__ERASE_FAIL);
>  
>  	if (irq_status & INTR_STATUS__ERASE_FAIL ||
>  	    irq_status & INTR_STATUS__LOCKED_BLK)
> -		denali.status = NAND_STATUS_FAIL;
> +		denali->status = NAND_STATUS_FAIL;
>  	else
> -		denali.status = 0;
> +		denali->status = 0;
>  }
>  
>  static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
>  			   int page)
>  {
> +	struct denali_nand_info *denali = mtd_to_denali(mtd);
>  	uint32_t addr;
>  
>  	switch (cmd) {
>  	case NAND_CMD_PAGEPROG:
>  		break;
>  	case NAND_CMD_STATUS:
> -		addr = MODE_11 | BANK(denali.flash_bank);
> -		index_addr(addr | 0, cmd);
> +		addr = MODE_11 | BANK(denali->flash_bank);
> +		index_addr(denali, addr | 0, cmd);
>  		break;
>  	case NAND_CMD_PARAM:
> -		clear_interrupts();
> +		clear_interrupts(denali);
>  	case NAND_CMD_READID:
> -		reset_buf();
> +		reset_buf(denali);
>  		/* sometimes ManufactureId read from register is not right
>  		 * e.g. some of Micron MT29F32G08QAA MLC NAND chips
>  		 * So here we send READID cmd to NAND insteand
>  		 * */
> -		addr = MODE_11 | BANK(denali.flash_bank);
> -		index_addr(addr | 0, cmd);
> -		index_addr(addr | 1, col & 0xFF);
> +		addr = MODE_11 | BANK(denali->flash_bank);
> +		index_addr(denali, addr | 0, cmd);
> +		index_addr(denali, addr | 1, col & 0xFF);
>  		break;
>  	case NAND_CMD_READ0:
>  	case NAND_CMD_SEQIN:
> -		denali.page = page;
> +		denali->page = page;
>  		break;
>  	case NAND_CMD_RESET:
> -		reset_bank();
> +		reset_bank(denali);
>  		break;
>  	case NAND_CMD_READOOB:
>  		/* TODO: Read OOB data */
> @@ -1060,20 +1098,20 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
>  		/* nothing to do here as it was done during NAND_CMD_ERASE1 */
>  		break;
>  	case NAND_CMD_UNLOCK1:
> -		addr = MODE_10 | BANK(denali.flash_bank) | page;
> -		index_addr(addr | 0, DENALI_UNLOCK_START);
> +		addr = MODE_10 | BANK(denali->flash_bank) | page;
> +		index_addr(denali, addr | 0, DENALI_UNLOCK_START);
>  		break;
>  	case NAND_CMD_UNLOCK2:
> -		addr = MODE_10 | BANK(denali.flash_bank) | page;
> -		index_addr(addr | 0, DENALI_UNLOCK_END);
> +		addr = MODE_10 | BANK(denali->flash_bank) | page;
> +		index_addr(denali, addr | 0, DENALI_UNLOCK_END);
>  		break;
>  	case NAND_CMD_LOCK:
> -		addr = MODE_10 | BANK(denali.flash_bank);
> -		index_addr(addr | 0, DENALI_LOCK);
> +		addr = MODE_10 | BANK(denali->flash_bank);
> +		index_addr(denali, addr | 0, DENALI_LOCK);
>  		break;
>  	case NAND_CMD_LOCK_TIGHT:
> -		addr = MODE_10 | BANK(denali.flash_bank);
> -		index_addr(addr | 0, DENALI_LOCK_TIGHT);
> +		addr = MODE_10 | BANK(denali->flash_bank);
> +		index_addr(denali, addr | 0, DENALI_LOCK_TIGHT);
>  		break;
>  	default:
>  		printf(": unsupported command received 0x%x\n", cmd);
> @@ -1083,34 +1121,42 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
>  /* end NAND core entry points */
>  
>  /* Initialization code to bring the device up to a known good state */
> -static void denali_hw_init(void)
> +static void denali_hw_init(struct denali_nand_info *denali)
>  {
>  	/*
>  	 * tell driver how many bit controller will skip before writing
>  	 * ECC code in OOB. This is normally used for bad block marker
>  	 */
>  	writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
> -	       denali.flash_reg + SPARE_AREA_SKIP_BYTES);
> -	detect_max_banks();
> -	denali_nand_reset();
> -	writel(0x0F, denali.flash_reg + RB_PIN_ENABLED);
> +	       denali->flash_reg + SPARE_AREA_SKIP_BYTES);
> +	detect_max_banks(denali);
> +	denali_nand_reset(denali);
> +	writel(0x0F, denali->flash_reg + RB_PIN_ENABLED);
>  	writel(CHIP_EN_DONT_CARE__FLAG,
> -	       denali.flash_reg + CHIP_ENABLE_DONT_CARE);
> -	writel(0xffff, denali.flash_reg + SPARE_AREA_MARKER);
> +	       denali->flash_reg + CHIP_ENABLE_DONT_CARE);
> +	writel(0xffff, denali->flash_reg + SPARE_AREA_MARKER);
>  
>  	/* Should set value for these registers when init */
> -	writel(0, denali.flash_reg + TWO_ROW_ADDR_CYCLES);
> -	writel(1, denali.flash_reg + ECC_ENABLE);
> -	denali_nand_timing_set();
> -	denali_irq_init();
> +	writel(0, denali->flash_reg + TWO_ROW_ADDR_CYCLES);
> +	writel(1, denali->flash_reg + ECC_ENABLE);
> +	denali_nand_timing_set(denali);
> +	denali_irq_init(denali);
>  }
>  
>  static struct nand_ecclayout nand_oob;
>  
>  static int denali_nand_init(struct nand_chip *nand)
>  {
> -	denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
> -	denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
> +	struct denali_nand_info *denali;
> +
> +	denali = malloc(sizeof(*denali));
> +	if (!denali)
> +		return -ENOMEM;
> +
> +	nand->priv = denali;
> +
> +	denali->flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
> +	denali->flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
>  
>  #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
>  	/* check whether flash got BBT table (located at end of flash). As we
> @@ -1137,7 +1183,7 @@ static int denali_nand_init(struct nand_chip *nand)
>  	 * Tell driver the ecc strength. This register may be already set
>  	 * correctly. So we read this value out.
>  	 */
> -	nand->ecc.strength = readl(denali.flash_reg + ECC_CORRECTION);
> +	nand->ecc.strength = readl(denali->flash_reg + ECC_CORRECTION);
>  	switch (nand->ecc.size) {
>  	case 512:
>  		nand->ecc.bytes = (nand->ecc.strength * 13 + 15) / 16 * 2;
> @@ -1158,7 +1204,7 @@ static int denali_nand_init(struct nand_chip *nand)
>  	nand->read_buf = denali_read_buf;
>  	nand->select_chip = denali_select_chip;
>  	nand->waitfunc = denali_waitfunc;
> -	denali_hw_init();
> +	denali_hw_init(denali);
>  	return 0;
>  }
>  

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-22 19:12     ` Scott Wood
@ 2014-04-23  7:15       ` Chin Liang See
  2014-04-24 10:25         ` Masahiro Yamada
  0 siblings, 1 reply; 10+ messages in thread
From: Chin Liang See @ 2014-04-23  7:15 UTC (permalink / raw)
  To: u-boot

On Tue, 2014-04-22 at 14:12 -0500, Scott Wood wrote:
> On Tue, 2014-04-22 at 10:04 +0900, Masahiro Yamada wrote:
> > Hi Scott,
> > 
> > 
> > > > It is really really painful to wait more than 10 seconds just for bad block
> > > > scanning to boot Linux.
> > > 
> > > Making bad block scans faster is a good thing, but why do you need to
> > > scan them just to boot Linux?  Aren't you using an on-flash BBT?
> > 
> > I did not know that.
> > I thought all blocks must be scanned.
> > 
> > Could you teach me the better way?
> 
> If you use NAND_BBT_USE_FLASH, and NAND_BBT_CREATE is present in the bbt
> descriptor (this is true of the default descriptors), then the scanning
> should only need to happen on first use.  On subsequent boots only the
> bad block table should need to be read.

Yup, I agreed with this statement :) I believe this bad block table can
be used by kernel in later stage. Probably someone can comment if I am
wrong.

Thanks
Chin Liang

> 
> -Scott
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-23  6:50 ` Chin Liang See
@ 2014-04-24 10:20   ` Masahiro Yamada
  0 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2014-04-24 10:20 UTC (permalink / raw)
  To: u-boot

Hi Chin,



> > [1] Fix denali_write_oob() handler.
> > 
> > As for v7, "nand markbad" did not work at all.
> > 
> > With this patch, it works.
> 
> Actually this was tested before.
> Here is the test result (as this was written as SOCFPGA documentation)
> 
> SOCFPGA_CYCLONE5 # nand markbad 0
> Bad block table written to 0x00003ffe0000, version 0x02
> Bad block table written to 0x00003ffc0000, version 0x02
> block 0x00000000 successfully marked as bad
> SOCFPGA_CYCLONE5 # nand bad
> 
> Device 0 bad blocks:
> 00000000
> 3ff80000
> 3ffa0000
> 3ffc0000
> 3ffe0000
> SOCFPGA_CYCLONE5 #
> 

The sutuation is different on my board.

When I run "nand markbad" with version 7, timeout error occurrs.

This is the log.

  => nand markbad 0
  Denali timeout with interrupt status 00000800
  Denali timeout with interrupt status 00000800
  block 0x00000000 NOT marked as bad! ERROR 0


I guess you test it on v2013.01 code base
with NAND_BBT_USE_FLASH enabled.

In that case, as you mentioned below,
nand_write_oob() is never called.
(But it is called on the latest release)

I'm gessing that's why you did not notice this bug in
nand_write_oob() function.



> > 
> > [3] Remove false comment
> > 
> >  /* Writes OOB data to the device.
> >   * This code unused under normal U-Boot console as normally page write raw
> >   * to be used for write oob data with main data.
> >   */
> >   static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
> > 
> > This comment is telling a lie.
> > write_oob_data() is called from "nand markbad" command.
> > It must be deleted.
> 
> Actually this comment is correct during integration into 2013 release. I
> set the option to use NAND_USE_FLASH_BBT where it will not call
> nand_do_write_oob. Do note that we will always write bad block info into
> bad block table which are located at last 4 blocks of the NAND flash.

Yes, this comment _was_ correct on v2013.01 release
only when you set NAND_BBT_USE_FLASH.

But it isn't on the latest release. (Precisely, after the commit
dfe64e2c)

On the latest code, bad block info are written back into
both oob area of  the bad block and flash-based bad block table.
It means write_oob_data() is always used under normal U-Boot usage.


> In latest code, the function "nand_do_write_oob" will be called if
> write_oob is set (where NAND_BBT_NO_OOB_BBM is clear). From the
> description of NAND_BBT_NO_OOB_BBM, user need to set it if we don't want
> the bad block table located at OOB. 

Yes, exactly.

> For our case, it should be true as we are using HW ECC. Our data might
> span into OOB region. Definitely we don't want this bad block table
> overwritten our data then. Wonder is this match with your understanding.
> 
> In short, the function write_oob_data shouldn't be called for our case.

I know SOCFPGA git repository (git://rocketboards.org/u-boot-socfpga.git)
is based on v2013.01 release.

This often caused problems under our review process.
A big change happened to NAND infrastucture  after v2013.04 release.
Check  commit dfe64e2c (mtd: resync with Linux-3.7.1).
With this commit, many NAND-related files were updated.

I think you tested your code based on v2013.01 and posted it to the ML.
It  often does not work on the current u-boot/master.

The behavior of bad-block-marking is one difference.

Before, I also pointed out "nand->ecc.strength" was missing
from your code.


And I notice another bug.
denali.c  v7  adds NAND_BBT_USE_FLASH to nand->options.
But on the latest code, it must be added nand->bbt_options.
Otherwise, nand_scan_tail() fails.

I think BBT option was added to nand->options on v2013.01.
But it was moved to nand->bbt_options.


If you had tested it on the latest release, you could have noticed
the bugs soon.
I strongly hope that SOCFPGA repository would be rebased on the latest
release.


Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob
  2014-04-23  7:15       ` Chin Liang See
@ 2014-04-24 10:25         ` Masahiro Yamada
  0 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2014-04-24 10:25 UTC (permalink / raw)
  To: u-boot

Hi Scott, Chin,

On Wed, 23 Apr 2014 02:15:10 -0500
Chin Liang See <clsee@altera.com> wrote:

> On Tue, 2014-04-22 at 14:12 -0500, Scott Wood wrote:
> > On Tue, 2014-04-22 at 10:04 +0900, Masahiro Yamada wrote:
> > > Hi Scott,
> > > 
> > > 
> > > > > It is really really painful to wait more than 10 seconds just for bad block
> > > > > scanning to boot Linux.
> > > > 
> > > > Making bad block scans faster is a good thing, but why do you need to
> > > > scan them just to boot Linux?  Aren't you using an on-flash BBT?
> > > 
> > > I did not know that.
> > > I thought all blocks must be scanned.
> > > 
> > > Could you teach me the better way?
> > 
> > If you use NAND_BBT_USE_FLASH, and NAND_BBT_CREATE is present in the bbt
> > descriptor (this is true of the default descriptors), then the scanning
> > should only need to happen on first use.  On subsequent boots only the
> > bad block table should need to be read.
> 
> Yup, I agreed with this statement :) I believe this bad block table can
> be used by kernel in later stage. Probably someone can comment if I am
> wrong.

It worked for me and it improved bbm checking. Thanks!


BTW,  NAND_BBT_USE_FLASH in denali driver did not work at first.

Finally I figured out it.
denali.c v7 adds the option to nand->options
  nand->options |= NAND_BBT_USE_FLASH;

I had to fix it like this
  nand->bbt_options |= NAND_BBT_USE_FLASH;



Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-04-24 10:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-18 11:30 [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Masahiro Yamada
2014-04-18 11:30 ` [U-Boot] [PATCH 2/2] mtd: denali: recover the same function prototypes as Linux Kernel Masahiro Yamada
2014-04-23  7:02   ` Chin Liang See
2014-04-21 22:21 ` [U-Boot] [PATCH 1/2] mtd: denali: improve nand_read_oob and fix nand_write_oob Scott Wood
2014-04-22  1:04   ` Masahiro Yamada
2014-04-22 19:12     ` Scott Wood
2014-04-23  7:15       ` Chin Liang See
2014-04-24 10:25         ` Masahiro Yamada
2014-04-23  6:50 ` Chin Liang See
2014-04-24 10:20   ` Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.