NAND: davinci: choose correct 1-bit h/w ECC reg

PATCH New Laurence Withers 3 participants 7 messages Sep 26, 2011 – Oct 10, 2011

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Laurence Withers

author

2011-09-26 16:02 UTC #0 lore raw

PATCH 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

In nand_davinci_readecc(), select the correct NANDF<n>ECC register based on CONFIG_SYS_NAND_CS rather than hardcoding the choice of NANDF1ECC. This allows 1-bit hardware ECC to work with chip select other than CS2.

Note this now matches the usage in nand_davinci_enable_hwecc(), which already had the correct handling, and allows refactoring to a single function encapsulating the register read.

Without this fix, writing NAND pages to a chip not wired to CS2 would result in in the ECC calculation always returning FFFFFF for each 512-byte segment, and reading back a correctly written page (one with ECC intact) would always fail. With this fix, the ECC is written and verified correctly.

---
 drivers/mtd/nand/davinci_nand.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

drivers/mtd/nand/davinci_nand.c +13 ~~-13~~

index d41579c..e8506dd 100644
@@ -176,12 +176,22 @@ static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd,
 
 #ifdef CONFIG_SYS_NAND_HW_ECC
 
+static u_int32_t nand_davinci_readecc(struct mtd_info *mtd)
+{
+	u_int32_t	ecc = 0;
+
+	ecc = __raw_readl(&(davinci_emif_regs->nandfecc[
+				CONFIG_SYS_NAND_CS - 2]));
+
+	return ecc;
+}
+
 static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 {
 	u_int32_t	val;
 
-	(void)__raw_readl(&(davinci_emif_regs->nandfecc[
-				CONFIG_SYS_NAND_CS - 2]));
+	/* reading the ECC result register resets the ECC calculation */
+	nand_davinci_readecc(mtd);
 
 	val = __raw_readl(&davinci_emif_regs->nandfcr);
 	val |= DAVINCI_NANDFCR_NAND_ENABLE(CONFIG_SYS_NAND_CS);

@@ -189,22 +199,12 @@ static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 	__raw_writel(val, &davinci_emif_regs->nandfcr);
 }
 
-static u_int32_t nand_davinci_readecc(struct mtd_info *mtd, u_int32_t region)
-{
-	u_int32_t	ecc = 0;
-
-	ecc = __raw_readl(&(davinci_emif_regs->nandfecc[region - 1]));
-
-	return ecc;
-}
-
 static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 		u_char *ecc_code)
 {
 	u_int32_t		tmp;
-	const int region = 1;
 
-	tmp = nand_davinci_readecc(mtd, region);
+	tmp = nand_davinci_readecc(mtd);
 
 	/* Squeeze 4 bytes ECC into 3 bytes by removing RESERVED bits
 	 * and shifting. RESERVED bits are 31 to 28 and 15 to 12. */

Signature · 1 line

1.7.2.5

Laurence Withers

author

2011-09-26 16:02 UTC #1 lore raw

PATCH 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

Note this now matches the usage in nand_davinci_enable_hwecc(), which already had the correct handling, and allows refactoring to a single function encapsulating the register read.

Signed-off-by Laurence Withers <[email protected]>

---
Changes for v2:
Add Signed-off-by to commit message.

---
 drivers/mtd/nand/davinci_nand.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

drivers/mtd/nand/davinci_nand.c +13 ~~-13~~

index d41579c..e8506dd 100644
@@ -176,12 +176,22 @@ static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd,
 
 #ifdef CONFIG_SYS_NAND_HW_ECC
 
+static u_int32_t nand_davinci_readecc(struct mtd_info *mtd)
+{
+	u_int32_t	ecc = 0;
+
+	ecc = __raw_readl(&(davinci_emif_regs->nandfecc[
+				CONFIG_SYS_NAND_CS - 2]));
+
+	return ecc;
+}
+
 static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 {
 	u_int32_t	val;
 
-	(void)__raw_readl(&(davinci_emif_regs->nandfecc[
-				CONFIG_SYS_NAND_CS - 2]));
+	/* reading the ECC result register resets the ECC calculation */
+	nand_davinci_readecc(mtd);
 
 	val = __raw_readl(&davinci_emif_regs->nandfcr);
 	val |= DAVINCI_NANDFCR_NAND_ENABLE(CONFIG_SYS_NAND_CS);

@@ -189,22 +199,12 @@ static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 	__raw_writel(val, &davinci_emif_regs->nandfcr);
 }
 
-static u_int32_t nand_davinci_readecc(struct mtd_info *mtd, u_int32_t region)
-{
-	u_int32_t	ecc = 0;
-
-	ecc = __raw_readl(&(davinci_emif_regs->nandfecc[region - 1]));
-
-	return ecc;
-}
-
 static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 		u_char *ecc_code)
 {
 	u_int32_t		tmp;
-	const int region = 1;
 
-	tmp = nand_davinci_readecc(mtd, region);
+	tmp = nand_davinci_readecc(mtd);
 
 	/* Squeeze 4 bytes ECC into 3 bytes by removing RESERVED bits
 	 * and shifting. RESERVED bits are 31 to 28 and 15 to 12. */

Signature · 1 line

1.7.2.5

Laurence Withers

author

2011-09-26 16:02 UTC #2 lore raw

PATCH v2 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

In reply to Laurence Withers

Note this now matches the usage in nand_davinci_enable_hwecc(), which already had the correct handling, and allows refactoring to a single function encapsulating the register read.

Signed-off-by Laurence Withers <[email protected]>

---
Changes for v2:
Add Signed-off-by to commit message.

---
 drivers/mtd/nand/davinci_nand.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

drivers/mtd/nand/davinci_nand.c +13 ~~-13~~

index d41579c..e8506dd 100644
@@ -176,12 +176,22 @@ static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd,
 
 #ifdef CONFIG_SYS_NAND_HW_ECC
 
+static u_int32_t nand_davinci_readecc(struct mtd_info *mtd)
+{
+	u_int32_t	ecc = 0;
+
+	ecc = __raw_readl(&(davinci_emif_regs->nandfecc[
+				CONFIG_SYS_NAND_CS - 2]));
+
+	return ecc;
+}
+
 static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 {
 	u_int32_t	val;
 
-	(void)__raw_readl(&(davinci_emif_regs->nandfecc[
-				CONFIG_SYS_NAND_CS - 2]));
+	/* reading the ECC result register resets the ECC calculation */
+	nand_davinci_readecc(mtd);
 
 	val = __raw_readl(&davinci_emif_regs->nandfcr);
 	val |= DAVINCI_NANDFCR_NAND_ENABLE(CONFIG_SYS_NAND_CS);

@@ -189,22 +199,12 @@ static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode)
 	__raw_writel(val, &davinci_emif_regs->nandfcr);
 }
 
-static u_int32_t nand_davinci_readecc(struct mtd_info *mtd, u_int32_t region)
-{
-	u_int32_t	ecc = 0;
-
-	ecc = __raw_readl(&(davinci_emif_regs->nandfecc[region - 1]));
-
-	return ecc;
-}
-
 static int nand_davinci_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 		u_char *ecc_code)
 {
 	u_int32_t		tmp;
-	const int region = 1;
 
-	tmp = nand_davinci_readecc(mtd, region);
+	tmp = nand_davinci_readecc(mtd);
 
 	/* Squeeze 4 bytes ECC into 3 bytes by removing RESERVED bits
 	 * and shifting. RESERVED bits are 31 to 28 and 15 to 12. */

Signature · 1 line

1.7.2.5

Laurence Withers

author

2011-10-10 13:45 UTC #3 lore raw

PATCH v2 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

Quoted 8 lines · Laurence Withers

In nand_davinci_readecc(), select the correct NANDF<n>ECC register based on CONFIG_SYS_NAND_CS rather than hardcoding the choice of NANDF1ECC. This allows 1-bit hardware ECC to work with chip select other than CS2.
Note this now matches the usage in nand_davinci_enable_hwecc(), which already had the correct handling, and allows refactoring to a single function encapsulating the register read.

Quoted 7 lines

Without this fix, writing NAND pages to a chip not wired to CS2 would result in in the ECC calculation always returning FFFFFF for each 512-byte segment, and reading back a correctly written page (one with ECC intact) would always fail. With this fix, the ECC is written and verified correctly.
Signed-off-by Laurence Withers <[email protected]>

Does anybody have any comments on this bugfix? If not, can it be accepted?

Many thanks, and bye for now,

Signature · 3 lines

Laurence Withers, <[email protected]>                http://www.guralp.com/
Direct tel:+447753988197 or tel:+443333408643               Software Engineer
General support queries: <[email protected]>         CMG-DCM CMG-EAM CMG-NAM

Scott Wood

2011-10-10 20:29 UTC #4 lore raw

PATCH v2 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

In reply to Laurence Withers

Quoted 14 lines · Laurence Withers

In nand_davinci_readecc(), select the correct NANDF<n>ECC register based on CONFIG_SYS_NAND_CS rather than hardcoding the choice of NANDF1ECC. This allows 1-bit hardware ECC to work with chip select other than CS2.
Note this now matches the usage in nand_davinci_enable_hwecc(), which already had the correct handling, and allows refactoring to a single function encapsulating the register read.
Without this fix, writing NAND pages to a chip not wired to CS2 would result in in the ECC calculation always returning FFFFFF for each 512-byte segment, and reading back a correctly written page (one with ECC intact) would always fail. With this fix, the ECC is written and verified correctly.

Quoted 7 lines

Signed-off-by Laurence Withers <[email protected]>
---
Changes for v2:
Add Signed-off-by to commit message.
---
 drivers/mtd/nand/davinci_nand.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

Applied to u-boot-nand-flash

-Scott

Sergei Shtylyov

2011-09-28 08:58 UTC #5 lore raw

PATCH 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

In reply to Laurence Withers

Hello.

Quoted 8 lines · Laurence Withers

In nand_davinci_readecc(), select the correct NANDF<n>ECC register based on CONFIG_SYS_NAND_CS rather than hardcoding the choice of NANDF1ECC. This allows 1-bit hardware ECC to work with chip select other than CS2.
Note this now matches the usage in nand_davinci_enable_hwecc(), which already had the correct handling, and allows refactoring to a single function encapsulating the register read.

Quoted 5 lines

Without this fix, writing NAND pages to a chip not wired to CS2 would result in in the ECC calculation always returning FFFFFF for each 512-byte segment, and reading back a correctly written page (one with ECC intact) would always fail. With this fix, the ECC is written and verified correctly.

You need to sign off your patch. Add this line to the changelog:

Signed-off-by Laurence Withers <[email protected]>

WBR, Sergei

Laurence Withers

author

2011-09-29 10:18 UTC #6 lore raw

PATCH 1/1 NAND: davinci: choose correct 1-bit h/w ECC reg

Quoted 3 lines · Sergei Shtylyov

You need to sign off your patch. Add this line to the changelog:
Signed-off-by Laurence Withers <[email protected]>

Thanks, I have reposted v2. Please ignore the posting with the unaltered subject line; I only realised after sending that I had forgotten it.

Bye for now,

Signature · 3 lines

Laurence Withers, <[email protected]>                http://www.guralp.com/
Direct tel:+447753988197 or tel:+443333408643               Software Engineer
General support queries: <[email protected]>         CMG-DCM CMG-EAM CMG-NAM