[PATCH V3 25/63 resend] Newly erased page read workaround

[PATCH V3 25/63 resend] Newly erased page read workaround

[Viresh Kumar]

From: Vipin Kumar <vipin.kumar@st.com>

A newly erased page contains ff in data as well as spare area. While reading an
erased page, the read out ecc from spare area does not match the ecc generated
by fsmc ecc hardware accelarator. This is because ecc of data ff ff is not ff
ff. This leads to errors when jffs2 fs erases and reads back the pages to
ensure consistency.

This patch adds a software workaround to ensure that the ecc check is not
performed for erased pages. An erased page is checked by checking data as ff ff.

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/mtd/nand/fsmc_nand.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 02edfba..d3f0d8d 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -385,7 +385,7 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 	struct fsmc_nand_data *host = container_of(mtd,
 					struct fsmc_nand_data, mtd);
 	struct fsmc_eccplace *ecc_place = host->ecc_place;
-	int i, j, s, stat, eccsize = chip->ecc.size;
+	int i, j, k, s, stat, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
 	int eccsteps = chip->ecc.steps;
 	uint8_t *p = buf;
@@ -425,11 +425,27 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 		memcpy(&ecc_code[i], oob, 13);
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
 
-		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
-		if (stat < 0)
-			mtd->ecc_stats.failed++;
-		else
-			mtd->ecc_stats.corrected += stat;
+		/*
+		 * This is a temporary erase check. A newly erased page read
+		 * would result in an ecc error because the oob data is also
+		 * erased to FF and the calculated ecc for an FF data is not
+		 * FF..FF.
+		 * This is a workaround to skip performing correction in case
+		 * data is FF..FF
+		 */
+		for (k = 0; k < eccsize; k++) {
+			if (*(p + k) != 0xff)
+				break;
+		}
+
+		if (k < eccsize) {
+			stat = chip->ecc.correct(mtd, p, &ecc_code[i],
+					&ecc_calc[i]);
+			if (stat < 0)
+				mtd->ecc_stats.failed++;
+			else
+				mtd->ecc_stats.corrected += stat;
+		}
 	}
 
 	return 0;
-- 
1.7.2.2

Re: [PATCH V3 36/63] ST SPEAr : FSMC (Flexible Static Memory Controller) NOR interface driver

[Linus Walleij]

2010/12/20 Viresh Kumar <viresh.kumar@st.com>:

> SPEAr1300 SoC supports FSMC to interface with various memories
> (NOR/NAND/SRAM). This patch adds the support for FSMC over NOR interface.
>
> The driver being used is driver/mtd/maps/physmap.c

OK..

(...)
> diff --git a/arch/arm/mach-spear13xx/fsmc-nor.c b/arch/arm/mach-spear13xx/fsmc-nor.c
> new file mode 100644
> index 0000000..03234b6
> --- /dev/null
> +++ b/arch/arm/mach-spear13xx/fsmc-nor.c

What is this file doing in mach-spear13xx? Several other platforms
like U300 and Nomadik use the same NOR controller.

This should be in drivers/mtd/* somewhere I believe?

> @@ -0,0 +1,85 @@
> +/*
> + * arch/arm/mach-spear13xx/fsmc-nor.c
> + *
> + * FSMC (Flexible Static Memory Controller) interface for NOR
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Vipin Kumar<vipin.kumar@st.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <plat/fsmc.h>

Use <linux/mtd/fsmc.h> as include file, get rid of plat/fsmc.h.
Extend that .h file with the stuff you need.

We need to keep definitions for MTD NOR, NAND and OneNAND using FSMC
in this file so as to keep things simple. One header file is enough, and it's
very generic too, FSMC is not for plat-spear/* but also U300, Nomadik and
Ux500.

Yours,
Linus Walleij

Re: [PATCH V3 36/63] ST SPEAr : FSMC (Flexible Static Memory Controller) NOR interface driver

[viresh kumar]

Linus,

On 12/26/2010 09:08 PM, Linus Walleij wrote:
> 2010/12/20 Viresh Kumar <viresh.kumar@st.com>:
> 

(...)

>> diff --git a/arch/arm/mach-spear13xx/fsmc-nor.c b/arch/arm/mach-spear13xx/fsmc-nor.c
>> new file mode 100644
>> index 0000000..03234b6
>> --- /dev/null
>> +++ b/arch/arm/mach-spear13xx/fsmc-nor.c
> 
> What is this file doing in mach-spear13xx? Several other platforms
> like U300 and Nomadik use the same NOR controller.
> 
> This should be in drivers/mtd/* somewhere I believe?
> 
>> @@ -0,0 +1,85 @@
>> +/*
>> + * arch/arm/mach-spear13xx/fsmc-nor.c
>> + *
>> + * FSMC (Flexible Static Memory Controller) interface for NOR
>> + *
>> + * Copyright (C) 2010 ST Microelectronics
>> + * Vipin Kumar<vipin.kumar@st.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <plat/fsmc.h>
> 
> Use <linux/mtd/fsmc.h> as include file, get rid of plat/fsmc.h.
> Extend that .h file with the stuff you need.
> 
> We need to keep definitions for MTD NOR, NAND and OneNAND using FSMC
> in this file so as to keep things simple. One header file is enough, and it's
> very generic too, FSMC is not for plat-spear/* but also U300, Nomadik and
> Ux500.
> 

Yes i agree. I will remove patches related to fsmc from this patch series and will
send them separately.

-- 
viresh

Re: [PATCH V3 25/63 resend] Newly erased page read workaround

[viresh kumar]

On 12/20/2010 02:49 PM, Viresh KUMAR wrote:
> From: Vipin Kumar <vipin.kumar@st.com>
> 
> A newly erased page contains ff in data as well as spare area. While reading an
> erased page, the read out ecc from spare area does not match the ecc generated
> by fsmc ecc hardware accelarator. This is because ecc of data ff ff is not ff
> ff. This leads to errors when jffs2 fs erases and reads back the pages to
> ensure consistency.
> 
> This patch adds a software workaround to ensure that the ecc check is not
> performed for erased pages. An erased page is checked by checking data as ff ff.

David,

Does this patch looks fine to you??

--
viresh