Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-02-09 Thread Josh Wu 

Hi, Andreas

On 2/10/2015 5:51 AM, Andreas Bießmann wrote:

Hi Scott,

On 09.02.15 18:04, Scott Wood wrote:

On Mon, 2015-02-02 at 14:08 +0800, Josh Wu wrote:

Hi, Scott

On 1/16/2015 4:24 PM, Andreas Bießmann wrote:

Hi Bo, Josh,

On 01/16/2015 07:50 AM, Josh Wu wrote:

Hi, Bo

On 1/16/2015 1:27 PM, Bo Shen wrote:

Hi Josh,

On 01/16/2015 11:54 AM, Josh Wu wrote:

As the PMECC hardware has different version. In SAMA5D4 chip, the
PMECC ip
can generate 0xff pmecc ECC value for all 0xff sector.

According to this, add PMECC version check, if it's SAMA5D4 then we
always
let PMECC hardware to correct it.

Signed-off-by: Josh Wu josh...@atmel.com

except the nitpick.

Acked-by: Bo Shen voice.s...@atmel.com

Acked-by: Andreas Bießmann andreas.de...@googlemail.com

Would you pick this patch into your tree with Bo and Andreas's Acked-by?
Thanks.

I won't be able to apply anything for at least a week and a half...
I'll look at it then if it hasn't been applied yet, but I'm fine with
the custodian for the relevant hardware taking it in the meantime.

I picked it up already


Great, Thanks.



Best regards

Andreas Bießmann

Best Regards,
Josh Wu
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-02-09 Thread Scott Wood 
On Mon, 2015-02-02 at 14:08 +0800, Josh Wu wrote:
 Hi, Scott
 
 On 1/16/2015 4:24 PM, Andreas Bießmann wrote:
  Hi Bo, Josh,
 
  On 01/16/2015 07:50 AM, Josh Wu wrote:
  Hi, Bo
 
  On 1/16/2015 1:27 PM, Bo Shen wrote:
  Hi Josh,
 
  On 01/16/2015 11:54 AM, Josh Wu wrote:
  As the PMECC hardware has different version. In SAMA5D4 chip, the
  PMECC ip
  can generate 0xff pmecc ECC value for all 0xff sector.
 
  According to this, add PMECC version check, if it's SAMA5D4 then we
  always
  let PMECC hardware to correct it.
 
  Signed-off-by: Josh Wu josh...@atmel.com
  except the nitpick.
 
  Acked-by: Bo Shen voice.s...@atmel.com
  Acked-by: Andreas Bießmann andreas.de...@googlemail.com
 
 Would you pick this patch into your tree with Bo and Andreas's Acked-by? 
 Thanks.

I won't be able to apply anything for at least a week and a half...
I'll look at it then if it hasn't been applied yet, but I'm fine with
the custodian for the relevant hardware taking it in the meantime.

-Scott


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-02-09 Thread Andreas Bießmann 
Hi Scott,

On 09.02.15 18:04, Scott Wood wrote:
 On Mon, 2015-02-02 at 14:08 +0800, Josh Wu wrote:
 Hi, Scott

 On 1/16/2015 4:24 PM, Andreas Bießmann wrote:
 Hi Bo, Josh,

 On 01/16/2015 07:50 AM, Josh Wu wrote:
 Hi, Bo

 On 1/16/2015 1:27 PM, Bo Shen wrote:
 Hi Josh,

 On 01/16/2015 11:54 AM, Josh Wu wrote:
 As the PMECC hardware has different version. In SAMA5D4 chip, the
 PMECC ip
 can generate 0xff pmecc ECC value for all 0xff sector.

 According to this, add PMECC version check, if it's SAMA5D4 then we
 always
 let PMECC hardware to correct it.

 Signed-off-by: Josh Wu josh...@atmel.com
 except the nitpick.

 Acked-by: Bo Shen voice.s...@atmel.com
 Acked-by: Andreas Bießmann andreas.de...@googlemail.com

 Would you pick this patch into your tree with Bo and Andreas's Acked-by? 
 Thanks.
 
 I won't be able to apply anything for at least a week and a half...
 I'll look at it then if it hasn't been applied yet, but I'm fine with
 the custodian for the relevant hardware taking it in the meantime.

I picked it up already

Best regards

Andreas Bießmann
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-02-01 Thread Josh Wu 

Hi, Scott

On 1/16/2015 4:24 PM, Andreas Bießmann wrote:

Hi Bo, Josh,

On 01/16/2015 07:50 AM, Josh Wu wrote:

Hi, Bo

On 1/16/2015 1:27 PM, Bo Shen wrote:

Hi Josh,

On 01/16/2015 11:54 AM, Josh Wu wrote:

As the PMECC hardware has different version. In SAMA5D4 chip, the
PMECC ip
can generate 0xff pmecc ECC value for all 0xff sector.

According to this, add PMECC version check, if it's SAMA5D4 then we
always
let PMECC hardware to correct it.

Signed-off-by: Josh Wu josh...@atmel.com

except the nitpick.

Acked-by: Bo Shen voice.s...@atmel.com

Acked-by: Andreas Bießmann andreas.de...@googlemail.com


Would you pick this patch into your tree with Bo and Andreas's Acked-by? 
Thanks.


Best Regards,
Josh Wu




Thanks for the quick review.


---

   drivers/mtd/nand/atmel_nand.c |  9 +
   drivers/mtd/nand/atmel_nand_ecc.h | 20 
   2 files changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/atmel_nand.c
b/drivers/mtd/nand/atmel_nand.c
index 620b6e8..b16e3aa 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -44,6 +44,7 @@ struct atmel_nand_host {
   u8pmecc_corr_cap;
   u16pmecc_sector_size;
   u32pmecc_index_table_offset;
+u32pmecc_version;

   intpmecc_bytes_per_sector;
   intpmecc_sector_number;
@@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info
*mtd, u32 pmecc_stat, uint8_t *buf,
   int i, err_nbr, eccbytes;
   uint8_t *buf_pos;

+/* SAMA5D4 PMECC IP can correct errors for all 0xff page */
+if (host-pmecc_version = PMECC_VERSION_SAMA5D4)

I think we can hard coded here, then we can drop the definition in
header file.

I don't like hard coded magic number in personly.

me too!


+goto normal_check;
+
   eccbytes = nand_chip-ecc.bytes;
   for (i = 0; i  eccbytes; i++)
   if (ecc[i] != 0xff)
@@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct
nand_chip *nand,
   nand-ecc.write_page = atmel_nand_pmecc_write_page;
   nand-ecc.strength = cap;

+/* Check the PMECC ip version */
+host-pmecc_version = pmecc_readl(host-pmerrloc, version);
+dev_dbg(host-dev, PMECC IP version is: %x\n,
host-pmecc_version);
+
   atmel_pmecc_core_init(mtd);

   return 0;
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h
b/drivers/mtd/nand/atmel_nand_ecc.h
index eac860d..b2d2682 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -123,6 +123,20 @@ struct pmecc_errloc_regs {
   u32 sigma[25];/* 0x28-0x88 Error Location Sigma Registers */
   u32 el[24];/* 0x8C-0xE8 Error Location Registers */
   u32 reserved1[5];/* 0xEC-0xFC Reserved */
+
+/*
+ * 0x100-0x1F8:
+ *   Reserved for AT91SAM9X5, AT91SAM9N12.
+ *   HSMC registers for SAMA5D3, SAMA5D4.
+ */

I think no need to add this.

actually, I would like to keep those comments.
As in the datasheet, sama5d3 and sama5d4's PMECC ERRLOC only have the
register range: 0x0~0xFF.
and the range 0x100-0x1F8 is for the HSMC.

So people will be confused if they find HSMC definitions in the
PMECC_ERRLOC structure.
I think list this comment would be cleaner.

This is reasonable, please let the comment in.


+u32 reserved2[63];
+
+/*
+ * 0x1FC:
+ *   PMECC version for AT91SAM9X5, AT91SAM9N12.
+ *   HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version.
+ */

ditto.

Like mentioned above, I want avoid the confusion about PMECC_ERRLOC/HSMC.


+u32 version;
   };

   /* For Error Location Configuration Register */
@@ -137,6 +151,12 @@ struct pmecc_errloc_regs {
   #definePMERRLOC_ERR_NUM_MASK(0x1f  8)
   #definePMERRLOC_CALC_DONE(1  0)

+/* PMECC IP version */
+#define PMECC_VERSION_SAMA5D40x113
+#define PMECC_VERSION_SAMA5D30x112
+#define PMECC_VERSION_AT91SAM9N120x102

No where will use the upper three definitions, we can drop them.

I don't have strong option about this. Just think the version
information is useful for other to reference.

Can someone else get this information from the datasheets? If not please
let this information there.


Best regards

Andreas Bießmann


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-01-16 Thread Andreas Bießmann 
Hi Bo, Josh,

On 01/16/2015 07:50 AM, Josh Wu wrote:
 Hi, Bo
 
 On 1/16/2015 1:27 PM, Bo Shen wrote:
 Hi Josh,

 On 01/16/2015 11:54 AM, Josh Wu wrote:
 As the PMECC hardware has different version. In SAMA5D4 chip, the
 PMECC ip
 can generate 0xff pmecc ECC value for all 0xff sector.

 According to this, add PMECC version check, if it's SAMA5D4 then we
 always
 let PMECC hardware to correct it.

 Signed-off-by: Josh Wu josh...@atmel.com

 except the nitpick.

 Acked-by: Bo Shen voice.s...@atmel.com

Acked-by: Andreas Bießmann andreas.de...@googlemail.com

 
 Thanks for the quick review.
 


 ---

   drivers/mtd/nand/atmel_nand.c |  9 +
   drivers/mtd/nand/atmel_nand_ecc.h | 20 
   2 files changed, 29 insertions(+)

 diff --git a/drivers/mtd/nand/atmel_nand.c
 b/drivers/mtd/nand/atmel_nand.c
 index 620b6e8..b16e3aa 100644
 --- a/drivers/mtd/nand/atmel_nand.c
 +++ b/drivers/mtd/nand/atmel_nand.c
 @@ -44,6 +44,7 @@ struct atmel_nand_host {
   u8pmecc_corr_cap;
   u16pmecc_sector_size;
   u32pmecc_index_table_offset;
 +u32pmecc_version;

   intpmecc_bytes_per_sector;
   intpmecc_sector_number;
 @@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info
 *mtd, u32 pmecc_stat, uint8_t *buf,
   int i, err_nbr, eccbytes;
   uint8_t *buf_pos;

 +/* SAMA5D4 PMECC IP can correct errors for all 0xff page */
 +if (host-pmecc_version = PMECC_VERSION_SAMA5D4)

 I think we can hard coded here, then we can drop the definition in
 header file.
 I don't like hard coded magic number in personly.

me too!

 

 +goto normal_check;
 +
   eccbytes = nand_chip-ecc.bytes;
   for (i = 0; i  eccbytes; i++)
   if (ecc[i] != 0xff)
 @@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct
 nand_chip *nand,
   nand-ecc.write_page = atmel_nand_pmecc_write_page;
   nand-ecc.strength = cap;

 +/* Check the PMECC ip version */
 +host-pmecc_version = pmecc_readl(host-pmerrloc, version);
 +dev_dbg(host-dev, PMECC IP version is: %x\n,
 host-pmecc_version);
 +
   atmel_pmecc_core_init(mtd);

   return 0;
 diff --git a/drivers/mtd/nand/atmel_nand_ecc.h
 b/drivers/mtd/nand/atmel_nand_ecc.h
 index eac860d..b2d2682 100644
 --- a/drivers/mtd/nand/atmel_nand_ecc.h
 +++ b/drivers/mtd/nand/atmel_nand_ecc.h
 @@ -123,6 +123,20 @@ struct pmecc_errloc_regs {
   u32 sigma[25];/* 0x28-0x88 Error Location Sigma Registers */
   u32 el[24];/* 0x8C-0xE8 Error Location Registers */
   u32 reserved1[5];/* 0xEC-0xFC Reserved */
 +
 +/*
 + * 0x100-0x1F8:
 + *   Reserved for AT91SAM9X5, AT91SAM9N12.
 + *   HSMC registers for SAMA5D3, SAMA5D4.
 + */

 I think no need to add this.
 actually, I would like to keep those comments.
 As in the datasheet, sama5d3 and sama5d4's PMECC ERRLOC only have the
 register range: 0x0~0xFF.
 and the range 0x100-0x1F8 is for the HSMC.
 
 So people will be confused if they find HSMC definitions in the
 PMECC_ERRLOC structure.
 I think list this comment would be cleaner.

This is reasonable, please let the comment in.

 

 +u32 reserved2[63];
 +
 +/*
 + * 0x1FC:
 + *   PMECC version for AT91SAM9X5, AT91SAM9N12.
 + *   HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version.
 + */

 ditto.
 Like mentioned above, I want avoid the confusion about PMECC_ERRLOC/HSMC.
 

 +u32 version;
   };

   /* For Error Location Configuration Register */
 @@ -137,6 +151,12 @@ struct pmecc_errloc_regs {
   #definePMERRLOC_ERR_NUM_MASK(0x1f  8)
   #definePMERRLOC_CALC_DONE(1  0)

 +/* PMECC IP version */
 +#define PMECC_VERSION_SAMA5D40x113
 +#define PMECC_VERSION_SAMA5D30x112
 +#define PMECC_VERSION_AT91SAM9N120x102

 No where will use the upper three definitions, we can drop them.
 I don't have strong option about this. Just think the version
 information is useful for other to reference.

Can someone else get this information from the datasheets? If not please
let this information there.


Best regards

Andreas Bießmann
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-01-16 Thread Josh Wu 

Hi, Andreas

On 1/16/2015 4:24 PM, Andreas Bießmann wrote:

Hi Bo, Josh,

On 01/16/2015 07:50 AM, Josh Wu wrote:

Hi, Bo

On 1/16/2015 1:27 PM, Bo Shen wrote:

Hi Josh,

On 01/16/2015 11:54 AM, Josh Wu wrote:

As the PMECC hardware has different version. In SAMA5D4 chip, the
PMECC ip
can generate 0xff pmecc ECC value for all 0xff sector.

According to this, add PMECC version check, if it's SAMA5D4 then we
always
let PMECC hardware to correct it.

Signed-off-by: Josh Wu josh...@atmel.com

except the nitpick.

Acked-by: Bo Shen voice.s...@atmel.com

Acked-by: Andreas Bießmann andreas.de...@googlemail.com


Thanks.




Thanks for the quick review.


---

   drivers/mtd/nand/atmel_nand.c |  9 +
   drivers/mtd/nand/atmel_nand_ecc.h | 20 
   2 files changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/atmel_nand.c
b/drivers/mtd/nand/atmel_nand.c
index 620b6e8..b16e3aa 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -44,6 +44,7 @@ struct atmel_nand_host {
   u8pmecc_corr_cap;
   u16pmecc_sector_size;
   u32pmecc_index_table_offset;
+u32pmecc_version;

   intpmecc_bytes_per_sector;
   intpmecc_sector_number;
@@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info
*mtd, u32 pmecc_stat, uint8_t *buf,
   int i, err_nbr, eccbytes;
   uint8_t *buf_pos;

+/* SAMA5D4 PMECC IP can correct errors for all 0xff page */
+if (host-pmecc_version = PMECC_VERSION_SAMA5D4)

I think we can hard coded here, then we can drop the definition in
header file.

I don't like hard coded magic number in personly.

me too!


+goto normal_check;
+
   eccbytes = nand_chip-ecc.bytes;
   for (i = 0; i  eccbytes; i++)
   if (ecc[i] != 0xff)
@@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct
nand_chip *nand,
   nand-ecc.write_page = atmel_nand_pmecc_write_page;
   nand-ecc.strength = cap;

+/* Check the PMECC ip version */
+host-pmecc_version = pmecc_readl(host-pmerrloc, version);
+dev_dbg(host-dev, PMECC IP version is: %x\n,
host-pmecc_version);
+
   atmel_pmecc_core_init(mtd);

   return 0;
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h
b/drivers/mtd/nand/atmel_nand_ecc.h
index eac860d..b2d2682 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -123,6 +123,20 @@ struct pmecc_errloc_regs {
   u32 sigma[25];/* 0x28-0x88 Error Location Sigma Registers */
   u32 el[24];/* 0x8C-0xE8 Error Location Registers */
   u32 reserved1[5];/* 0xEC-0xFC Reserved */
+
+/*
+ * 0x100-0x1F8:
+ *   Reserved for AT91SAM9X5, AT91SAM9N12.
+ *   HSMC registers for SAMA5D3, SAMA5D4.
+ */

I think no need to add this.

actually, I would like to keep those comments.
As in the datasheet, sama5d3 and sama5d4's PMECC ERRLOC only have the
register range: 0x0~0xFF.
and the range 0x100-0x1F8 is for the HSMC.

So people will be confused if they find HSMC definitions in the
PMECC_ERRLOC structure.
I think list this comment would be cleaner.

This is reasonable, please let the comment in.


+u32 reserved2[63];
+
+/*
+ * 0x1FC:
+ *   PMECC version for AT91SAM9X5, AT91SAM9N12.
+ *   HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version.
+ */

ditto.

Like mentioned above, I want avoid the confusion about PMECC_ERRLOC/HSMC.


+u32 version;
   };

   /* For Error Location Configuration Register */
@@ -137,6 +151,12 @@ struct pmecc_errloc_regs {
   #definePMERRLOC_ERR_NUM_MASK(0x1f  8)
   #definePMERRLOC_CALC_DONE(1  0)

+/* PMECC IP version */
+#define PMECC_VERSION_SAMA5D40x113
+#define PMECC_VERSION_SAMA5D30x112
+#define PMECC_VERSION_AT91SAM9N120x102

No where will use the upper three definitions, we can drop them.

I don't have strong option about this. Just think the version
information is useful for other to reference.

Can someone else get this information from the datasheets?

No, This information are not in datasheet.


If not please
let this information there.

Okay. let's keep this as it is.

Best Regards,
Josh Wu



Best regards

Andreas Bießmann


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-01-15 Thread Bo Shen 

Hi Josh,

On 01/16/2015 11:54 AM, Josh Wu wrote:

As the PMECC hardware has different version. In SAMA5D4 chip, the PMECC ip
can generate 0xff pmecc ECC value for all 0xff sector.

According to this, add PMECC version check, if it's SAMA5D4 then we always
let PMECC hardware to correct it.

Signed-off-by: Josh Wu josh...@atmel.com


except the nitpick.

Acked-by: Bo Shen voice.s...@atmel.com



---

  drivers/mtd/nand/atmel_nand.c |  9 +
  drivers/mtd/nand/atmel_nand_ecc.h | 20 
  2 files changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 620b6e8..b16e3aa 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -44,6 +44,7 @@ struct atmel_nand_host {
u8  pmecc_corr_cap;
u16 pmecc_sector_size;
u32 pmecc_index_table_offset;
+   u32 pmecc_version;

int pmecc_bytes_per_sector;
int pmecc_sector_number;
@@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info *mtd, u32 
pmecc_stat, uint8_t *buf,
int i, err_nbr, eccbytes;
uint8_t *buf_pos;

+   /* SAMA5D4 PMECC IP can correct errors for all 0xff page */
+   if (host-pmecc_version = PMECC_VERSION_SAMA5D4)


I think we can hard coded here, then we can drop the definition in 
header file.



+   goto normal_check;
+
eccbytes = nand_chip-ecc.bytes;
for (i = 0; i  eccbytes; i++)
if (ecc[i] != 0xff)
@@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct nand_chip 
*nand,
nand-ecc.write_page = atmel_nand_pmecc_write_page;
nand-ecc.strength = cap;

+   /* Check the PMECC ip version */
+   host-pmecc_version = pmecc_readl(host-pmerrloc, version);
+   dev_dbg(host-dev, PMECC IP version is: %x\n, host-pmecc_version);
+
atmel_pmecc_core_init(mtd);

return 0;
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h 
b/drivers/mtd/nand/atmel_nand_ecc.h
index eac860d..b2d2682 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -123,6 +123,20 @@ struct pmecc_errloc_regs {
u32 sigma[25];  /* 0x28-0x88 Error Location Sigma Registers */
u32 el[24]; /* 0x8C-0xE8 Error Location Registers */
u32 reserved1[5];   /* 0xEC-0xFC Reserved */
+
+   /*
+* 0x100-0x1F8:
+*   Reserved for AT91SAM9X5, AT91SAM9N12.
+*   HSMC registers for SAMA5D3, SAMA5D4.
+*/


I think no need to add this.


+   u32 reserved2[63];
+
+   /*
+* 0x1FC:
+*   PMECC version for AT91SAM9X5, AT91SAM9N12.
+*   HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version.
+*/


ditto.


+   u32 version;
  };

  /* For Error Location Configuration Register */
@@ -137,6 +151,12 @@ struct pmecc_errloc_regs {
  #define   PMERRLOC_ERR_NUM_MASK   (0x1f  8)
  #define   PMERRLOC_CALC_DONE  (1  0)

+/* PMECC IP version */
+#define PMECC_VERSION_SAMA5D4  0x113
+#define PMECC_VERSION_SAMA5D3  0x112
+#define PMECC_VERSION_AT91SAM9N12  0x102


No where will use the upper three definitions, we can drop them.


+#define PMECC_VERSION_AT91SAM9X5   0x101


If hard coded, we can drop it also.


+
  /* Galois field dimension */
  #define PMECC_GF_DIMENSION_13 13
  #define PMECC_GF_DIMENSION_14 14



Best Regards,
Bo Shen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-01-15 Thread Josh Wu 

Hi, Bo

On 1/16/2015 1:27 PM, Bo Shen wrote:

Hi Josh,

On 01/16/2015 11:54 AM, Josh Wu wrote:
As the PMECC hardware has different version. In SAMA5D4 chip, the 
PMECC ip

can generate 0xff pmecc ECC value for all 0xff sector.

According to this, add PMECC version check, if it's SAMA5D4 then we 
always

let PMECC hardware to correct it.

Signed-off-by: Josh Wu josh...@atmel.com


except the nitpick.

Acked-by: Bo Shen voice.s...@atmel.com


Thanks for the quick review.





---

  drivers/mtd/nand/atmel_nand.c |  9 +
  drivers/mtd/nand/atmel_nand_ecc.h | 20 
  2 files changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/atmel_nand.c 
b/drivers/mtd/nand/atmel_nand.c

index 620b6e8..b16e3aa 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -44,6 +44,7 @@ struct atmel_nand_host {
  u8pmecc_corr_cap;
  u16pmecc_sector_size;
  u32pmecc_index_table_offset;
+u32pmecc_version;

  intpmecc_bytes_per_sector;
  intpmecc_sector_number;
@@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info 
*mtd, u32 pmecc_stat, uint8_t *buf,

  int i, err_nbr, eccbytes;
  uint8_t *buf_pos;

+/* SAMA5D4 PMECC IP can correct errors for all 0xff page */
+if (host-pmecc_version = PMECC_VERSION_SAMA5D4)


I think we can hard coded here, then we can drop the definition in 
header file.

I don't like hard coded magic number in personly.




+goto normal_check;
+
  eccbytes = nand_chip-ecc.bytes;
  for (i = 0; i  eccbytes; i++)
  if (ecc[i] != 0xff)
@@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct 
nand_chip *nand,

  nand-ecc.write_page = atmel_nand_pmecc_write_page;
  nand-ecc.strength = cap;

+/* Check the PMECC ip version */
+host-pmecc_version = pmecc_readl(host-pmerrloc, version);
+dev_dbg(host-dev, PMECC IP version is: %x\n, 
host-pmecc_version);

+
  atmel_pmecc_core_init(mtd);

  return 0;
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h 
b/drivers/mtd/nand/atmel_nand_ecc.h

index eac860d..b2d2682 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -123,6 +123,20 @@ struct pmecc_errloc_regs {
  u32 sigma[25];/* 0x28-0x88 Error Location Sigma Registers */
  u32 el[24];/* 0x8C-0xE8 Error Location Registers */
  u32 reserved1[5];/* 0xEC-0xFC Reserved */
+
+/*
+ * 0x100-0x1F8:
+ *   Reserved for AT91SAM9X5, AT91SAM9N12.
+ *   HSMC registers for SAMA5D3, SAMA5D4.
+ */


I think no need to add this.

actually, I would like to keep those comments.
As in the datasheet, sama5d3 and sama5d4's PMECC ERRLOC only have the 
register range: 0x0~0xFF.

and the range 0x100-0x1F8 is for the HSMC.

So people will be confused if they find HSMC definitions in the 
PMECC_ERRLOC structure.

I think list this comment would be cleaner.




+u32 reserved2[63];
+
+/*
+ * 0x1FC:
+ *   PMECC version for AT91SAM9X5, AT91SAM9N12.
+ *   HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version.
+ */


ditto.

Like mentioned above, I want avoid the confusion about PMECC_ERRLOC/HSMC.




+u32 version;
  };

  /* For Error Location Configuration Register */
@@ -137,6 +151,12 @@ struct pmecc_errloc_regs {
  #definePMERRLOC_ERR_NUM_MASK(0x1f  8)
  #definePMERRLOC_CALC_DONE(1  0)

+/* PMECC IP version */
+#define PMECC_VERSION_SAMA5D40x113
+#define PMECC_VERSION_SAMA5D30x112
+#define PMECC_VERSION_AT91SAM9N120x102


No where will use the upper three definitions, we can drop them.
I don't have strong option about this. Just think the version 
information is useful for other to reference.





+#define PMECC_VERSION_AT91SAM9X5 0x101


If hard coded, we can drop it also.


+
  /* Galois field dimension */
  #define PMECC_GF_DIMENSION_1313
  #define PMECC_GF_DIMENSION_1414



Best Regards,
Bo Shen


Thanks.
Best Regards,
Josh Wu

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction

2015-01-15 Thread Josh Wu 
As the PMECC hardware has different version. In SAMA5D4 chip, the PMECC ip
can generate 0xff pmecc ECC value for all 0xff sector.

According to this, add PMECC version check, if it's SAMA5D4 then we always
let PMECC hardware to correct it.

Signed-off-by: Josh Wu josh...@atmel.com

---

 drivers/mtd/nand/atmel_nand.c |  9 +
 drivers/mtd/nand/atmel_nand_ecc.h | 20 
 2 files changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 620b6e8..b16e3aa 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -44,6 +44,7 @@ struct atmel_nand_host {
u8  pmecc_corr_cap;
u16 pmecc_sector_size;
u32 pmecc_index_table_offset;
+   u32 pmecc_version;
 
int pmecc_bytes_per_sector;
int pmecc_sector_number;
@@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info *mtd, u32 
pmecc_stat, uint8_t *buf,
int i, err_nbr, eccbytes;
uint8_t *buf_pos;
 
+   /* SAMA5D4 PMECC IP can correct errors for all 0xff page */
+   if (host-pmecc_version = PMECC_VERSION_SAMA5D4)
+   goto normal_check;
+
eccbytes = nand_chip-ecc.bytes;
for (i = 0; i  eccbytes; i++)
if (ecc[i] != 0xff)
@@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct nand_chip 
*nand,
nand-ecc.write_page = atmel_nand_pmecc_write_page;
nand-ecc.strength = cap;
 
+   /* Check the PMECC ip version */
+   host-pmecc_version = pmecc_readl(host-pmerrloc, version);
+   dev_dbg(host-dev, PMECC IP version is: %x\n, host-pmecc_version);
+
atmel_pmecc_core_init(mtd);
 
return 0;
diff --git a/drivers/mtd/nand/atmel_nand_ecc.h 
b/drivers/mtd/nand/atmel_nand_ecc.h
index eac860d..b2d2682 100644
--- a/drivers/mtd/nand/atmel_nand_ecc.h
+++ b/drivers/mtd/nand/atmel_nand_ecc.h
@@ -123,6 +123,20 @@ struct pmecc_errloc_regs {
u32 sigma[25];  /* 0x28-0x88 Error Location Sigma Registers */
u32 el[24]; /* 0x8C-0xE8 Error Location Registers */
u32 reserved1[5];   /* 0xEC-0xFC Reserved */
+
+   /*
+* 0x100-0x1F8:
+*   Reserved for AT91SAM9X5, AT91SAM9N12.
+*   HSMC registers for SAMA5D3, SAMA5D4.
+*/
+   u32 reserved2[63];
+
+   /*
+* 0x1FC:
+*   PMECC version for AT91SAM9X5, AT91SAM9N12.
+*   HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version.
+*/
+   u32 version;
 };
 
 /* For Error Location Configuration Register */
@@ -137,6 +151,12 @@ struct pmecc_errloc_regs {
 #definePMERRLOC_ERR_NUM_MASK   (0x1f  8)
 #definePMERRLOC_CALC_DONE  (1  0)
 
+/* PMECC IP version */
+#define PMECC_VERSION_SAMA5D4  0x113
+#define PMECC_VERSION_SAMA5D3  0x112
+#define PMECC_VERSION_AT91SAM9N12  0x102
+#define PMECC_VERSION_AT91SAM9X5   0x101
+
 /* Galois field dimension */
 #define PMECC_GF_DIMENSION_13  13
 #define PMECC_GF_DIMENSION_14  14
-- 
1.9.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot