[PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Feng Kan]

Fix ECC Correction bug where the byte offset location were double
fliped causing correction routine to toggle the wrong byte location
in the ECC segment. The ndfc_calculate_ecc routine change the order
of getting the ECC code.
        /* The NDFC uses Smart Media (SMC) bytes order */
        ecc_code[0] = p[2];
        ecc_code[1] = p[1];
        ecc_code[2] = p[3];
But in the Correction algorithm when calculating the byte offset
location, the b1 is used as the upper part of the address. Which
again reverse the order making the final byte offset address 
location incorrect.
	byte_addr = (addressbits[b1] << 4) + addressbits[b0];
The order is change to read it in straight and let the correction
function to revert it to SMC order.

Signed-off-by: Feng Kan <fkan@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
---
 drivers/mtd/nand/ndfc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 5906c40..d9d3e6e 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -101,8 +101,8 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 	wmb();
 	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
 	/* The NDFC uses Smart Media (SMC) bytes order */
-	ecc_code[0] = p[2];
-	ecc_code[1] = p[1];
+	ecc_code[0] = p[1];
+	ecc_code[1] = p[2];
 	ecc_code[2] = p[3];
 
 	return 0;
-- 
1.5.5

RE: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Victor Gallardo]

Hi Artem and David,

Can you guys double check this patch. We need to monitor it until it gets accepted.

http://lists.infradead.org/pipermail/linux-mtd/2009-August/027054.html

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

Best Regards,

Victor Gallardo

[PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Feng Kan]

Fix ECC Correction bug where the byte offset location were double
fliped causing correction routine to toggle the wrong byte location
in the ECC segment. The ndfc_calculate_ecc routine change the order
of getting the ECC code.
        /* The NDFC uses Smart Media (SMC) bytes order */
        ecc_code[0] = p[2];
        ecc_code[1] = p[1];
        ecc_code[2] = p[3];
But in the Correction algorithm when calculating the byte offset
location, the b1 is used as the upper part of the address. Which
again reverse the order making the final byte offset address 
location incorrect.
	byte_addr = (addressbits[b1] << 4) + addressbits[b0];
The order is change to read it in straight and let the correction
function to revert it to SMC order.

Signed-off-by: Feng Kan <fkan@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
---
 drivers/mtd/nand/ndfc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 5906c40..d9d3e6e 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -101,8 +101,8 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 	wmb();
 	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
 	/* The NDFC uses Smart Media (SMC) bytes order */
-	ecc_code[0] = p[2];
-	ecc_code[1] = p[1];
+	ecc_code[0] = p[1];
+	ecc_code[1] = p[2];
 	ecc_code[2] = p[3];
 
 	return 0;
-- 
1.5.5

[PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Feng Kan]

Fix ECC Correction bug where the byte offset location were double
fliped causing correction routine to toggle the wrong byte location
in the ECC segment. The ndfc_calculate_ecc routine change the order
of getting the ECC code.
        /* The NDFC uses Smart Media (SMC) bytes order */
        ecc_code[0] = p[2];
        ecc_code[1] = p[1];
        ecc_code[2] = p[3];
But in the Correction algorithm when calculating the byte offset
location, the b1 is used as the upper part of the address. Which
again reverse the order making the final byte offset address 
location incorrect.
	byte_addr = (addressbits[b1] << 4) + addressbits[b0];
The order is change to read it in straight and let the correction
function to revert it to SMC order.

Signed-off-by: Feng Kan <fkan@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
---
 drivers/mtd/nand/ndfc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 5906c40..d9d3e6e 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -101,8 +101,8 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 	wmb();
 	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
 	/* The NDFC uses Smart Media (SMC) bytes order */
-	ecc_code[0] = p[2];
-	ecc_code[1] = p[1];
+	ecc_code[0] = p[1];
+	ecc_code[1] = p[2];
 	ecc_code[2] = p[3];
 
 	return 0;
-- 
1.5.5

Re: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Victor Gallardo]

Hi Sean=0A=0A> =0A> It looks like another fix for this bug is to leave the =
current code=0A> alone and turn off CONFIG_MTD_NAND_ECC_SMC.=0A=0AThis=A0wo=
uld fix the problem by hiding the issue. Fengs patch is=A0the correct way t=
o go.=0A=0ABest Regards,=0A=0AVictor Gallardo

Re: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Victor Gallardo]

Hi Sean

> 
> It looks like another fix for this bug is to leave the current code
> alone and turn off CONFIG_MTD_NAND_ECC_SMC.

This would fix the problem by hiding the issue. Fengs patch is the correct way to go.

Best Regards,

Victor Gallardo

RE: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Feng Kan]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Yes, I have considered that. However, it would make the #define rather confusing
for the rest.

Cheers,
Feng


-----Original Message-----
From: Sean MacLennan [mailto:smaclennan@pikatech.com]
Sent: Fri 8/21/2009 11:55 AM
To: Feng Kan
Cc: linuxppc-dev@ozlabs.org; linux-mtd@lists.infradead.org; Feng Kan
Subject: Re: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.
 
On Thu, 20 Aug 2009 17:19:17 -0700
Feng Kan <fkan@amcc.com> wrote:

> Fix ECC Correction bug where the byte offset location were double
> fliped causing correction routine to toggle the wrong byte location
> in the ECC segment. The ndfc_calculate_ecc routine change the order
> of getting the ECC code.

It looks like another fix for this bug is to leave the current code
alone and turn off CONFIG_MTD_NAND_ECC_SMC.

This could be a better fix if this is the way u-boot currently works.
Has anybody verified if the current u-boot has the ECC problem?

Cheers,
   Sean


[-- Attachment #2: Type: text/html, Size: 1598 bytes --]

Re: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Sean MacLennan]

On Thu, 20 Aug 2009 17:19:17 -0700
Feng Kan <fkan@amcc.com> wrote:

> Fix ECC Correction bug where the byte offset location were double
> fliped causing correction routine to toggle the wrong byte location
> in the ECC segment. The ndfc_calculate_ecc routine change the order
> of getting the ECC code.

It looks like another fix for this bug is to leave the current code
alone and turn off CONFIG_MTD_NAND_ECC_SMC.

This could be a better fix if this is the way u-boot currently works.
Has anybody verified if the current u-boot has the ECC problem?

Cheers,
   Sean

Re: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Stefan Roese]

On Friday 21 August 2009 02:19:17 Feng Kan wrote:
> Fix ECC Correction bug where the byte offset location were double
> fliped causing correction routine to toggle the wrong byte location
> in the ECC segment. The ndfc_calculate_ecc routine change the order
> of getting the ECC code.
>         /* The NDFC uses Smart Media (SMC) bytes order */
>         ecc_code[0] = p[2];
>         ecc_code[1] = p[1];
>         ecc_code[2] = p[3];
> But in the Correction algorithm when calculating the byte offset
> location, the b1 is used as the upper part of the address. Which
> again reverse the order making the final byte offset address
> location incorrect.
> 	byte_addr = (addressbits[b1] << 4) + addressbits[b0];
> The order is change to read it in straight and let the correction
> function to revert it to SMC order.
>
> Signed-off-by: Feng Kan <fkan@amcc.com>
> Acked-by: Victor Gallardo <vgallardo@amcc.com>
> Acked-by: Prodyut Hazarika <phazarika@amcc.com>

Acked-by: Stefan Roese <sr@denx.de>

Would be great if we could get this fix into 2.6.31.

Cheers,
Stefan

Re: [PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Sean MacLennan]

On Thu, 20 Aug 2009 17:19:17 -0700
Feng Kan <fkan@amcc.com> wrote:

> Fix ECC Correction bug where the byte offset location were double
> fliped causing correction routine to toggle the wrong byte location
> in the ECC segment. The ndfc_calculate_ecc routine change the order
> of getting the ECC code.
>         /* The NDFC uses Smart Media (SMC) bytes order */
>         ecc_code[0] = p[2];
>         ecc_code[1] = p[1];
>         ecc_code[2] = p[3];
> But in the Correction algorithm when calculating the byte offset
> location, the b1 is used as the upper part of the address. Which
> again reverse the order making the final byte offset address 
> location incorrect.
> 	byte_addr = (addressbits[b1] << 4) + addressbits[b0];
> The order is change to read it in straight and let the correction
> function to revert it to SMC order.
> 
> Signed-off-by: Feng Kan <fkan@amcc.com>
> Acked-by: Victor Gallardo <vgallardo@amcc.com>
> Acked-by: Prodyut Hazarika <phazarika@amcc.com>
> ---
>  drivers/mtd/nand/ndfc.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
> index 5906c40..d9d3e6e 100644
> --- a/drivers/mtd/nand/ndfc.c
> +++ b/drivers/mtd/nand/ndfc.c
> @@ -101,8 +101,8 @@ static int ndfc_calculate_ecc(struct mtd_info
> *mtd, wmb();
>  	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
>  	/* The NDFC uses Smart Media (SMC) bytes order */
> -	ecc_code[0] = p[2];
> -	ecc_code[1] = p[1];
> +	ecc_code[0] = p[1];
> +	ecc_code[1] = p[2];
>  	ecc_code[2] = p[3];
>  
>  	return 0;

Acked-by: Sean MacLennan <smaclennan@pikatech.com>

Cheers,
   Sean

[PATCH 1/1] Fix ECC Correction bug for SMC ordering for NDFC driver.

[Feng Kan]

Fix ECC Correction bug where the byte offset location were double
fliped causing correction routine to toggle the wrong byte location
in the ECC segment. The ndfc_calculate_ecc routine change the order
of getting the ECC code.
        /* The NDFC uses Smart Media (SMC) bytes order */
        ecc_code[0] = p[2];
        ecc_code[1] = p[1];
        ecc_code[2] = p[3];
But in the Correction algorithm when calculating the byte offset
location, the b1 is used as the upper part of the address. Which
again reverse the order making the final byte offset address 
location incorrect.
	byte_addr = (addressbits[b1] << 4) + addressbits[b0];
The order is change to read it in straight and let the correction
function to revert it to SMC order.

Signed-off-by: Feng Kan <fkan@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
---
 drivers/mtd/nand/ndfc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 5906c40..d9d3e6e 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -101,8 +101,8 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 	wmb();
 	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
 	/* The NDFC uses Smart Media (SMC) bytes order */
-	ecc_code[0] = p[2];
-	ecc_code[1] = p[1];
+	ecc_code[0] = p[1];
+	ecc_code[1] = p[2];
 	ecc_code[2] = p[3];
 
 	return 0;
-- 
1.5.5