mtd: atmel_nand: according to pmecc version to perform 0xff page correction

PATCH In review Josh Wu 4 participants 10 messages Jan 16, 2015 – Feb 10, 2015

Subsystems (3)

Memory technology devices (MTD) Arm/Microchip (AT91) SoC support NAND flash

lore

Share & tools

lore

Josh Wu

author

2015-01-16 03:54 UTC #0 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

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 <[email protected]>

---

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

drivers/mtd/nand/atmel_nand.c +9 -0

index 620b6e8..b16e3aa 100644
@@ -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;

drivers/mtd/nand/atmel_nand_ecc.h +20 -0

index eac860d..b2d2682 100644
@@ -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 {
 #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
+#define PMECC_VERSION_AT91SAM9X5		0x101
+
 /* Galois field dimension */
 #define PMECC_GF_DIMENSION_13			13
 #define PMECC_GF_DIMENSION_14			14

Signature · 1 line

1.9.1

Josh Wu

author

2015-01-16 06:50 UTC #2 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

Hi, Bo

Quoted 11 lines · Bo Shen

Hi Josh,
Quoted 8 lines · 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.

Quoted 5 lines

Quoted 1 line
Signed-off-by Josh Wu <[email protected]>
except the nitpick.
Acked-by Bo Shen <[email protected]>

Thanks for the quick review.

Quoted 26 lines

---

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

drivers/mtd/nand/atmel_nand.c +1 -0

index 620b6e8..b16e3aa 100644
@@ -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;

Quoted 5 lines

drivers/mtd/nand/atmel_nand.c static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf, · line 491
Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+    /* 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.

Quoted 28 lines

drivers/mtd/nand/atmel_nand.c +2 -0

+        goto normal_check;
+
      eccbytes = nand_chip->ecc.bytes;
      for (i = 0; i < eccbytes; i++)

          if (ecc[i] != 0xff)

drivers/mtd/nand/atmel_nand.c +4 -0

@@ -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;

drivers/mtd/nand/atmel_nand_ecc.h +0 -0

index eac860d..b2d2682 100644
@@ -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 */

Quoted 8 lines

drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 131
Quoted 6 lines
drivers/mtd/nand/atmel_nand_ecc.h +6 -0
+
+    /*
+     * 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.

Quoted 10 lines

drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 138
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h +7 -0
+    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.

Quoted 9 lines

drivers/mtd/nand/atmel_nand_ecc.h +1 -0

+    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)

Quoted 6 lines

drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 157
Quoted 4 lines
drivers/mtd/nand/atmel_nand_ecc.h +4 -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.

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

Quoted 5 lines

drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 158
Quoted 2 lines
drivers/mtd/nand/atmel_nand_ecc.h +1 -0
+#define PMECC_VERSION_AT91SAM9X5 0x101
If hard coded, we can drop it also.

Quoted 8 lines

drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 162
Quoted 5 lines
drivers/mtd/nand/atmel_nand_ecc.h +1 -0
+
  /* Galois field dimension */
  #define PMECC_GF_DIMENSION_13            13
  #define PMECC_GF_DIMENSION_14            14
Best Regards,
Bo Shen

Thanks.
Best Regards,
Josh Wu

Andreas Bießmann

2015-01-16 08:24 UTC #3 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

Hi Bo, Josh,

Quoted 19 lines · Josh Wu

Hi, Bo
Quoted 16 lines · Bo Shen
Hi Josh,
Quoted 9 lines · 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 <[email protected]>
except the nitpick.
Acked-by Bo Shen <[email protected]>

Acked-by Andreas Bie?mann <[email protected]>

Quoted 29 lines

Thanks for the quick review.

Quoted 26 lines

---

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

drivers/mtd/nand/atmel_nand.c +1 -0

index 620b6e8..b16e3aa 100644
@@ -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;

Quoted 6 lines

Quoted 5 lines
drivers/mtd/nand/atmel_nand.c static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf, · line 491
Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+    /* 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!

Quoted 37 lines

Quoted 29 lines

drivers/mtd/nand/atmel_nand.c +2 -0

+        goto normal_check;
+
      eccbytes = nand_chip->ecc.bytes;
      for (i = 0; i < eccbytes; i++)

          if (ecc[i] != 0xff)

drivers/mtd/nand/atmel_nand.c +4 -0

@@ -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;

drivers/mtd/nand/atmel_nand_ecc.h +0 -0

index eac860d..b2d2682 100644
@@ -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 */

Quoted 8 lines

drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 131
Quoted 6 lines
drivers/mtd/nand/atmel_nand_ecc.h +6 -0
+
+    /*
+     * 0x100-0x1F8:
+     *   Reserved for AT91SAM9X5, AT91SAM9N12.
+     *   HSMC registers for SAMA5D3, SAMA5D4.
+     */
I think no need to add this.

Quoted 8 lines

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.

Quoted 22 lines

Quoted 11 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 138
Quoted 9 lines
drivers/mtd/nand/atmel_nand_ecc.h +7 -0
+    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.
Quoted 9 lines
Quoted 9 lines
drivers/mtd/nand/atmel_nand_ecc.h +1 -0
+    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)
 

Quoted 8 lines

Quoted 6 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 157
Quoted 4 lines
drivers/mtd/nand/atmel_nand_ecc.h +4 -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.
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

Josh Wu

author

2015-01-16 08:51 UTC #4 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

Hi, Andreas

Quoted 17 lines · Andreas Bie?mann

Hi Bo, Josh,
Quoted 14 lines · Josh Wu
Hi, Bo
Quoted 11 lines · Bo Shen
Hi Josh,
Quoted 8 lines · 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.

Quoted 5 lines

Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Josh Wu <[email protected]>
except the nitpick.
Acked-by Bo Shen <[email protected]>
Acked-by Andreas Bie?mann <[email protected]>

Thanks.

Quoted 96 lines

Quoted 27 lines
Thanks for the quick review.
Quoted 24 lines
Quoted 24 lines
---
   drivers/mtd/nand/atmel_nand.c     |  9 +++++++++
   drivers/mtd/nand/atmel_nand_ecc.h | 20 ++++++++++++++++++++
   2 files changed, 29 insertions(+)
drivers/mtd/nand/atmel_nand.c +1 -0
index 620b6e8..b16e3aa 100644
@@ -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;
 
Quoted 5 lines
Quoted 4 lines
drivers/mtd/nand/atmel_nand.c static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf, · line 491
Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+    /* 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!
Quoted 34 lines
Quoted 26 lines
Quoted 26 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+        goto normal_check;
+
       eccbytes = nand_chip->ecc.bytes;
       for (i = 0; i < eccbytes; i++)
           if (ecc[i] != 0xff)
drivers/mtd/nand/atmel_nand.c +4 -0
@@ -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;
drivers/mtd/nand/atmel_nand_ecc.h +0 -0
index eac860d..b2d2682 100644
@@ -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 */
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 131
Quoted 7 lines
drivers/mtd/nand/atmel_nand_ecc.h +6 -0
       u32 reserved1[5];    /* 0xEC-0xFC Reserved */
+
+    /*
+     * 0x100-0x1F8:
+     *   Reserved for AT91SAM9X5, AT91SAM9N12.
+     *   HSMC registers for SAMA5D3, SAMA5D4.
+     */
I think no need to add this.
Quoted 8 lines
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.
Quoted 18 lines
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 138
Quoted 7 lines
drivers/mtd/nand/atmel_nand_ecc.h +7 -0
+    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.
Quoted 8 lines
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h +1 -0
+    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)
 

Quoted 8 lines

Quoted 7 lines
Quoted 5 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 157
Quoted 4 lines
drivers/mtd/nand/atmel_nand_ecc.h +4 -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.
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.

Quoted 2 lines

If not please
let this information there.

Okay. let's keep this as it is.

Best Regards,
Josh Wu

Quoted 4 lines

Best regards
Andreas Bie?mann

Josh Wu

author

2015-02-02 06:08 UTC #5 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

In reply to Andreas Bießmann

Hi, Scott

Quoted 17 lines · Andreas Bie?mann

Hi Bo, Josh,
Quoted 14 lines · Josh Wu
Hi, Bo
Quoted 11 lines · Bo Shen
Hi Josh,
Quoted 8 lines · 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.

Quoted 5 lines

Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Josh Wu <[email protected]>
except the nitpick.
Acked-by Bo Shen <[email protected]>
Acked-by Andreas Bie?mann <[email protected]>

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

Best Regards,
Josh Wu

Quoted 110 lines

Quoted 27 lines
Thanks for the quick review.
Quoted 24 lines
Quoted 24 lines
---
   drivers/mtd/nand/atmel_nand.c     |  9 +++++++++
   drivers/mtd/nand/atmel_nand_ecc.h | 20 ++++++++++++++++++++
   2 files changed, 29 insertions(+)
drivers/mtd/nand/atmel_nand.c +1 -0
index 620b6e8..b16e3aa 100644
@@ -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;
 
Quoted 5 lines
Quoted 4 lines
drivers/mtd/nand/atmel_nand.c static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf, · line 491
Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+    /* 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!
Quoted 34 lines
Quoted 26 lines
Quoted 26 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+        goto normal_check;
+
       eccbytes = nand_chip->ecc.bytes;
       for (i = 0; i < eccbytes; i++)
           if (ecc[i] != 0xff)
drivers/mtd/nand/atmel_nand.c +4 -0
@@ -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;
drivers/mtd/nand/atmel_nand_ecc.h +0 -0
index eac860d..b2d2682 100644
@@ -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 */
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 131
Quoted 7 lines
drivers/mtd/nand/atmel_nand_ecc.h +6 -0
       u32 reserved1[5];    /* 0xEC-0xFC Reserved */
+
+    /*
+     * 0x100-0x1F8:
+     *   Reserved for AT91SAM9X5, AT91SAM9N12.
+     *   HSMC registers for SAMA5D3, SAMA5D4.
+     */
I think no need to add this.
Quoted 8 lines
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.
Quoted 18 lines
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 138
Quoted 7 lines
drivers/mtd/nand/atmel_nand_ecc.h +7 -0
+    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.
Quoted 8 lines
Quoted 8 lines
drivers/mtd/nand/atmel_nand_ecc.h +1 -0
+    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)
 
Quoted 7 lines
Quoted 5 lines
drivers/mtd/nand/atmel_nand_ecc.h struct pmecc_errloc_regs { · line 157
Quoted 4 lines
drivers/mtd/nand/atmel_nand_ecc.h +4 -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.
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

Scott Wood

2015-02-09 17:04 UTC #6 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

Quoted 20 lines · Josh Wu

Hi, Scott
Quoted 17 lines · Andreas Bie?mann
Hi Bo, Josh,
Quoted 14 lines · Josh Wu
Hi, Bo
Quoted 11 lines · Bo Shen
Hi Josh,
Quoted 8 lines · 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.

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Josh Wu <[email protected]>
except the nitpick.
Acked-by Bo Shen <[email protected]>
Acked-by Andreas Bie?mann <[email protected]>
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

Andreas Bießmann

2015-02-09 21:51 UTC #7 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

Hi Scott,

Quoted 25 lines · Scott Wood

Quoted 24 lines · Josh Wu
Hi, Scott
Quoted 21 lines · Andreas Bie?mann
Hi Bo, Josh,
Quoted 18 lines · Josh Wu
Hi, Bo
Quoted 15 lines · Bo Shen
Hi Josh,
Quoted 9 lines · 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 <[email protected]>
except the nitpick.
Acked-by Bo Shen <[email protected]>

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Acked-by Andreas Bie?mann <[email protected]>
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

Josh Wu

author

2015-02-10 01:44 UTC #8 lore raw

PATCH 1/1 mtd: atmel_nand: according to pmecc version to perform 0xff page correction

Hi, Andreas

Quoted 27 lines · Andreas Bie?mann

Hi Scott,
Quoted 24 lines · Scott Wood
Quoted 23 lines · Josh Wu
Hi, Scott
Quoted 20 lines · Andreas Bie?mann
Hi Bo, Josh,
Quoted 17 lines · Josh Wu
Hi, Bo
Quoted 14 lines · Bo Shen
Hi Josh,
Quoted 9 lines · 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 <[email protected]>
except the nitpick.

Quoted 8 lines

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Quoted 1 line
Quoted 1 line
Acked-by Bo Shen <[email protected]>
Acked-by Andreas Bie?mann <[email protected]>
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.

Quoted 4 lines

Best regards
Andreas Bie?mann

Best Regards,
Josh Wu

Andreas Bießmann

2015-02-07 22:46 UTC #9 lore raw

In reply to Josh Wu

Dear Josh Wu,

Quoted 6 lines · 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.

Quoted 8 lines

Signed-off-by Josh Wu <[email protected]>
Acked-by Bo Shen <[email protected]>
Acked-by Andreas Bie?mann <[email protected]>
---
drivers/mtd/nand/atmel_nand.c | 9 +++++++++
drivers/mtd/nand/atmel_nand_ecc.h | 20 ++++++++++++++++++++
2 files changed, 29 insertions(+)

applied to u-boot-atmel/master, thanks!

Best regards,
Andreas Bie?mann