[PATCH 1/7] mtd: Blackfin NFC: fix nand busy detection

[PATCH 1/7] mtd: Blackfin NFC: fix nand busy detection

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

The IRQSTAT register is a W1C register used by the interrupt handler and
may have its BUSY bit changed.  This makes it somewhat unreliable for the
polling devready function.  So switch it over to use the BUSY bit in the
STAT register that always reflects the current state of the hardware.

This fixes driver hangs seen when the NAND flash is under heavy system
load (like I/O benchmarks).

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/nand/bf5xx_nand.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index 2974995..f3e3e74 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -218,9 +218,9 @@ static void bf5xx_nand_hwcontrol(struct mtd_info *mtd, int cmd,
  */
 static int bf5xx_nand_devready(struct mtd_info *mtd)
 {
-	unsigned short val = bfin_read_NFC_IRQSTAT();
+	unsigned short val = bfin_read_NFC_STAT();
 
-	if ((val & NBUSYIRQ) == NBUSYIRQ)
+	if ((val & NBUSY) == NBUSY)
 		return 1;
 	else
 		return 0;
-- 
1.7.2

[PATCH 2/7] mtd: Blackfin NFC: delete useless comment about jffs2

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

The low level NAND driver doesn't care about filesystems, so punt the
local comment indicating otherwise.

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/nand/bf5xx_nand.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index f3e3e74..81a05cd 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -20,9 +20,6 @@
  *		- DMA supported in ECC_HW
  *		- YAFFS tested as rootfs in both ECC_HW and ECC_SW
  *
- * TODO:
- * 	Enable JFFS2 over NAND as rootfs
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
-- 
1.7.2

[PATCH 3/7] mtd: Blackfin NFC: fix typo for read/write delay setup

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

We used the platform rd_dly field when we meant to use the wr_dly field.

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/nand/bf5xx_nand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index 81a05cd..b553705 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -632,7 +632,7 @@ static int bf5xx_nand_hw_init(struct bf5xx_nand_info *info)
 	val = (plat->page_size << NFC_PG_SIZE_OFFSET) |
 		(plat->data_width << NFC_NWIDTH_OFFSET) |
 		(plat->rd_dly << NFC_RDDLY_OFFSET) |
-		(plat->rd_dly << NFC_WRDLY_OFFSET);
+		(plat->wr_dly << NFC_WRDLY_OFFSET);
 	dev_dbg(info->device, "NFC_CTL is 0x%04x\n", val);
 
 	bfin_write_NFC_CTL(val);
-- 
1.7.2

[PATCH 4/7] mtd: Blackfin NFC: wait for the ECC reset to finish

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

When resetting the ECC registers/counters, the bit will automatically
clear itself once the reset has actually finished.  So make sure we
wait for that to occur before doing anything else rather than assuming
everything is peachy and proceeding with stale ECC values.

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/nand/bf5xx_nand.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index b553705..949f656 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -507,6 +507,8 @@ static void bf5xx_nand_dma_rw(struct mtd_info *mtd,
 	 */
 	bfin_write_NFC_RST(ECC_RST);
 	SSYNC();
+	while (bfin_read_NFC_RST() & ECC_RST)
+		cpu_relax();
 
 	disable_dma(CH_NFC);
 	clear_dma_irqstat(CH_NFC);
-- 
1.7.2

Re: [PATCH 4/7] mtd: Blackfin NFC: wait for the ECC reset to finish

[David Woodhouse]

On Thu, 2010-08-05 at 11:07 -0400, Mike Frysinger wrote:
> 
> +       while (bfin_read_NFC_RST() & ECC_RST)
> +               cpu_relax(); 

All applied, but I'd be happier if this one had some kind of timeout and
didn't have the potential to loop forever if the hardware misbehaves.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [Uclinux-dist-devel] [PATCH 4/7] mtd: Blackfin NFC: wait for the ECC reset to finish

[Mike Frysinger]

On Thu, Aug 5, 2010 at 11:16, David Woodhouse wrote:
> On Thu, 2010-08-05 at 11:07 -0400, Mike Frysinger wrote:
>>
>> +       while (bfin_read_NFC_RST() & ECC_RST)
>> +               cpu_relax();
>
> All applied, but I'd be happier if this one had some kind of timeout and
> didn't have the potential to loop forever if the hardware misbehaves.

ive opened a tracker item for it
-mike

Re: [Uclinux-dist-devel] [PATCH 4/7] mtd: Blackfin NFC: wait for the ECC reset to finish

[Barry Song]

On Thu, Aug 5, 2010 at 11:16 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2010-08-05 at 11:07 -0400, Mike Frysinger wrote:
>>
>> +       while (bfin_read_NFC_RST() & ECC_RST)
>> +               cpu_relax();
>
> All applied, but I'd be happier if this one had some kind of timeout and
> didn't have the potential to loop forever if the hardware misbehaves.
If this loop forever, it means a chip bug and really has no any way to
continue to run the system. So the point is "bfin_read_NFC_RST() &
ECC_RST" will must be real for the right chip, if the chip has bug,
there should be workaround but not a timeout.

>
> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
>
>
> _______________________________________________
> Uclinux-dist-devel mailing list
> Uclinux-dist-devel@blackfin.uclinux.org
> https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>

Re: [Uclinux-dist-devel] [PATCH 4/7] mtd: Blackfin NFC: wait for the ECC reset to finish

[Mike Frysinger]

On Sun, Aug 15, 2010 at 22:54, Barry Song wrote:
> On Thu, Aug 5, 2010 at 11:16 PM, David Woodhouse wrote:
>> On Thu, 2010-08-05 at 11:07 -0400, Mike Frysinger wrote:
>>>
>>> +       while (bfin_read_NFC_RST() & ECC_RST)
>>> +               cpu_relax();
>>
>> All applied, but I'd be happier if this one had some kind of timeout and
>> didn't have the potential to loop forever if the hardware misbehaves.
>
> If this loop forever, it means a chip bug and really has no any way to
> continue to run the system. So the point is "bfin_read_NFC_RST() &
> ECC_RST" will must be real for the right chip, if the chip has bug,
> there should be workaround but not a timeout.

i think the presumption on the linux/driver side is that hardware has
the tendency to suck, so rather than hang the whole system because the
NFC flakes out, it should recover gracefully.  so spitting out NAND
I/O errors all over the console is preferable to a system that goes
nowhere.

personally, i think it depends on the driver/situation, and in the
embedded world, i tend to agree with you Barry in that a dead subpiece
is the same as the whole system being dead.  but my opinion doesnt
matter much if the subsystem maintainers reject the patches ;).
-mike

[PATCH 5/7] mtd: Blackfin NFC: make sure to check NAND_ALE in cmd_ctrl

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

The NAND base may send some controls which are neither CLE nor ALE, so
we need to explicitly check both instead of assuming things are always
one or the other.  Otherwise, we sometimes send out illegal addresses
to the NAND device.

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/nand/bf5xx_nand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index 949f656..2f90cac 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -203,7 +203,7 @@ static void bf5xx_nand_hwcontrol(struct mtd_info *mtd, int cmd,
 
 	if (ctrl & NAND_CLE)
 		bfin_write_NFC_CMD(cmd);
-	else
+	else if (ctrl & NAND_ALE)
 		bfin_write_NFC_ADDR(cmd);
 	SSYNC();
 }
-- 
1.7.2

[PATCH 6/7] mtd: Blackfin NFC: fix handling of page sizes

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

Rather than forcing the platform resources to declare the desired page
size, simply use the existing information passed down to us by the higher
layers.  This way we work out of the box with all flash chips that the
kernel knows about.

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/include/asm/nand.h |    3 -
 drivers/mtd/nand/bf5xx_nand.c    |   86 +++++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/arch/blackfin/include/asm/nand.h b/arch/blackfin/include/asm/nand.h
index 3a1e79d..256c50d 100644
--- a/arch/blackfin/include/asm/nand.h
+++ b/arch/blackfin/include/asm/nand.h
@@ -15,8 +15,6 @@
  * partitions	 = mtd partition list
  */
 
-#define NFC_PG_SIZE_256		0
-#define NFC_PG_SIZE_512		1
 #define NFC_PG_SIZE_OFFSET	9
 
 #define NFC_NWIDTH_8		0
@@ -30,7 +28,6 @@
 
 struct bf5xx_nand_platform {
 	/* NAND chip information */
-	unsigned short		page_size;
 	unsigned short		data_width;
 
 	/* RD/WR strobe delay timing information, all times in SCLK cycles */
diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index 2f90cac..b54f222 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -314,18 +314,16 @@ static int bf5xx_nand_correct_data_256(struct mtd_info *mtd, u_char *dat,
 static int bf5xx_nand_correct_data(struct mtd_info *mtd, u_char *dat,
 					u_char *read_ecc, u_char *calc_ecc)
 {
-	struct bf5xx_nand_info *info = mtd_to_nand_info(mtd);
-	struct bf5xx_nand_platform *plat = info->platform;
-	unsigned short page_size = (plat->page_size ? 512 : 256);
+	struct nand_chip *chip = mtd->priv;
 	int ret;
 
 	ret = bf5xx_nand_correct_data_256(mtd, dat, read_ecc, calc_ecc);
 
-	/* If page size is 512, correct second 256 bytes */
-	if (page_size == 512) {
+	/* If ecc size is 512, correct second 256 bytes */
+	if (chip->ecc.size == 512) {
 		dat += 256;
-		read_ecc += 8;
-		calc_ecc += 8;
+		read_ecc += 3;
+		calc_ecc += 3;
 		ret |= bf5xx_nand_correct_data_256(mtd, dat, read_ecc, calc_ecc);
 	}
 
@@ -341,13 +339,12 @@ static int bf5xx_nand_calculate_ecc(struct mtd_info *mtd,
 		const u_char *dat, u_char *ecc_code)
 {
 	struct bf5xx_nand_info *info = mtd_to_nand_info(mtd);
-	struct bf5xx_nand_platform *plat = info->platform;
-	u16 page_size = (plat->page_size ? 512 : 256);
+	struct nand_chip *chip = mtd->priv;
 	u16 ecc0, ecc1;
 	u32 code[2];
 	u8 *p;
 
-	/* first 4 bytes ECC code for 256 page size */
+	/* first 3 bytes ECC code for 256 page size */
 	ecc0 = bfin_read_NFC_ECC0();
 	ecc1 = bfin_read_NFC_ECC1();
 
@@ -355,12 +352,11 @@ static int bf5xx_nand_calculate_ecc(struct mtd_info *mtd,
 
 	dev_dbg(info->device, "returning ecc 0x%08x\n", code[0]);
 
-	/* first 3 bytes in ecc_code for 256 page size */
 	p = (u8 *) code;
 	memcpy(ecc_code, p, 3);
 
-	/* second 4 bytes ECC code for 512 page size */
-	if (page_size == 512) {
+	/* second 3 bytes ECC code for 512 ecc size */
+	if (chip->ecc.size == 512) {
 		ecc0 = bfin_read_NFC_ECC2();
 		ecc1 = bfin_read_NFC_ECC3();
 		code[1] = (ecc0 & 0x7ff) | ((ecc1 & 0x7ff) << 11);
@@ -480,8 +476,7 @@ static void bf5xx_nand_dma_rw(struct mtd_info *mtd,
 				uint8_t *buf, int is_read)
 {
 	struct bf5xx_nand_info *info = mtd_to_nand_info(mtd);
-	struct bf5xx_nand_platform *plat = info->platform;
-	unsigned short page_size = (plat->page_size ? 512 : 256);
+	struct nand_chip *chip = mtd->priv;
 	unsigned short val;
 
 	dev_dbg(info->device, " mtd->%p, buf->%p, is_read %d\n",
@@ -495,10 +490,10 @@ static void bf5xx_nand_dma_rw(struct mtd_info *mtd,
 	 */
 	if (is_read)
 		invalidate_dcache_range((unsigned int)buf,
-				(unsigned int)(buf + page_size));
+				(unsigned int)(buf + chip->ecc.size));
 	else
 		flush_dcache_range((unsigned int)buf,
-				(unsigned int)(buf + page_size));
+				(unsigned int)(buf + chip->ecc.size));
 
 	/*
 	 * This register must be written before each page is
@@ -519,13 +514,13 @@ static void bf5xx_nand_dma_rw(struct mtd_info *mtd,
 
 	/* The DMAs have different size on BF52x and BF54x */
 #ifdef CONFIG_BF52x
-	set_dma_x_count(CH_NFC, (page_size >> 1));
+	set_dma_x_count(CH_NFC, (chip->ecc.size >> 1));
 	set_dma_x_modify(CH_NFC, 2);
 	val = DI_EN | WDSIZE_16;
 #endif
 
 #ifdef CONFIG_BF54x
-	set_dma_x_count(CH_NFC, (page_size >> 2));
+	set_dma_x_count(CH_NFC, (chip->ecc.size >> 2));
 	set_dma_x_modify(CH_NFC, 4);
 	val = DI_EN | WDSIZE_32;
 #endif
@@ -547,12 +542,11 @@ static void bf5xx_nand_dma_read_buf(struct mtd_info *mtd,
 					uint8_t *buf, int len)
 {
 	struct bf5xx_nand_info *info = mtd_to_nand_info(mtd);
-	struct bf5xx_nand_platform *plat = info->platform;
-	unsigned short page_size = (plat->page_size ? 512 : 256);
+	struct nand_chip *chip = mtd->priv;
 
 	dev_dbg(info->device, "mtd->%p, buf->%p, int %d\n", mtd, buf, len);
 
-	if (len == page_size)
+	if (len == chip->ecc.size)
 		bf5xx_nand_dma_rw(mtd, buf, 1);
 	else
 		bf5xx_nand_read_buf(mtd, buf, len);
@@ -562,12 +556,11 @@ static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
 				const uint8_t *buf, int len)
 {
 	struct bf5xx_nand_info *info = mtd_to_nand_info(mtd);
-	struct bf5xx_nand_platform *plat = info->platform;
-	unsigned short page_size = (plat->page_size ? 512 : 256);
+	struct nand_chip *chip = mtd->priv;
 
 	dev_dbg(info->device, "mtd->%p, buf->%p, len %d\n", mtd, buf, len);
 
-	if (len == page_size)
+	if (len == chip->ecc.size)
 		bf5xx_nand_dma_rw(mtd, (uint8_t *)buf, 0);
 	else
 		bf5xx_nand_write_buf(mtd, buf, len);
@@ -626,12 +619,11 @@ static int bf5xx_nand_hw_init(struct bf5xx_nand_info *info)
 
 	/* setup NFC_CTL register */
 	dev_info(info->device,
-		"page_size=%d, data_width=%d, wr_dly=%d, rd_dly=%d\n",
-		(plat->page_size ? 512 : 256),
+		"data_width=%d, wr_dly=%d, rd_dly=%d\n",
 		(plat->data_width ? 16 : 8),
 		plat->wr_dly, plat->rd_dly);
 
-	val = (plat->page_size << NFC_PG_SIZE_OFFSET) |
+	val = (1 << NFC_PG_SIZE_OFFSET) |
 		(plat->data_width << NFC_NWIDTH_OFFSET) |
 		(plat->rd_dly << NFC_RDDLY_OFFSET) |
 		(plat->wr_dly << NFC_WRDLY_OFFSET);
@@ -697,6 +689,33 @@ static int __devexit bf5xx_nand_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int bf5xx_nand_scan(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	int ret;
+
+	ret = nand_scan_ident(mtd, 1);
+	if (ret)
+		return ret;
+
+	if (hardware_ecc) {
+		/*
+		 * for nand with page size > 512B, think it as several sections with 512B
+		 */
+		if (likely(mtd->writesize >= 512)) {
+			chip->ecc.size = 512;
+			chip->ecc.bytes = 6;
+		} else {
+			chip->ecc.size = 256;
+			chip->ecc.bytes = 3;
+			bfin_write_NFC_CTL(bfin_read_NFC_CTL() & ~(1 << NFC_PG_SIZE_OFFSET));
+			SSYNC();
+		}
+	}
+
+	return	nand_scan_tail(mtd);
+}
+
 /*
  * bf5xx_nand_probe
  *
@@ -782,15 +801,6 @@ static int __devinit bf5xx_nand_probe(struct platform_device *pdev)
 		chip->badblock_pattern = &bootrom_bbt;
 		chip->ecc.layout = &bootrom_ecclayout;
 #endif
-
-		if (plat->page_size == NFC_PG_SIZE_256) {
-			chip->ecc.bytes = 3;
-			chip->ecc.size = 256;
-		} else if (plat->page_size == NFC_PG_SIZE_512) {
-			chip->ecc.bytes = 6;
-			chip->ecc.size = 512;
-		}
-
 		chip->read_buf      = bf5xx_nand_dma_read_buf;
 		chip->write_buf     = bf5xx_nand_dma_write_buf;
 		chip->ecc.calculate = bf5xx_nand_calculate_ecc;
@@ -802,7 +812,7 @@ static int __devinit bf5xx_nand_probe(struct platform_device *pdev)
 	}
 
 	/* scan hardware nand chip and setup mtd info data struct */
-	if (nand_scan(mtd, 1)) {
+	if (bf5xx_nand_scan(mtd)) {
 		err = -ENXIO;
 		goto out_err_nand_scan;
 	}
-- 
1.7.2

[PATCH 7/7] mtd: Blackfin NFC: fix raw page write/read handling

[Mike Frysinger]

From: Barry Song <barry.song@analog.com>

Our write_buf/read_buf funcs always do ECC in HW ECC mode.  That is not
needed for raw funcs.  In fact, write_buf/read_buf should be a pure func
for data input/output while chip->ecc.hwctl controls ECC.  Unfortunately,
we can't separate ECC from normal data input/output in our NFC, so our DMA
write_buf/read_buf entries are coupled with ECC operations closely.

Thus we need to provide dedicated read_page_raw/write_page_raw funcs where
we do non-DMA transactions so as to avoid automatic ECC.

Signed-off-by: Barry Song <barry.song@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mtd/nand/bf5xx_nand.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index b54f222..a382e3d 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -566,6 +566,22 @@ static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
 		bf5xx_nand_write_buf(mtd, buf, len);
 }
 
+static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+		uint8_t *buf, int page)
+{
+	bf5xx_nand_read_buf(mtd, buf, mtd->writesize);
+	bf5xx_nand_read_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	return 0;
+}
+
+static void bf5xx_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+		const uint8_t *buf)
+{
+	bf5xx_nand_write_buf(mtd, buf, mtd->writesize);
+	bf5xx_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
+}
+
 /*
  * System initialization functions
  */
@@ -807,6 +823,8 @@ static int __devinit bf5xx_nand_probe(struct platform_device *pdev)
 		chip->ecc.correct   = bf5xx_nand_correct_data;
 		chip->ecc.mode	    = NAND_ECC_HW;
 		chip->ecc.hwctl	    = bf5xx_nand_enable_hwecc;
+		chip->ecc.read_page_raw = bf5xx_nand_read_page_raw;
+		chip->ecc.write_page_raw = bf5xx_nand_write_page_raw;
 	} else {
 		chip->ecc.mode	    = NAND_ECC_SOFT;
 	}
-- 
1.7.2