mtd: gpmi: Always report ECC stats and return max_bitflips

PATCH In review Zach Sadecki 3 participants 17 messages Dec 3, 2012 – Jan 15, 2013

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Zach Sadecki

author

2012-12-03 19:48 UTC #0 lore raw

PATCH 1/1 mtd: gpmi: Always report ECC stats and return max_bitflips

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

---
  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 ++++++++-------------------
  1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +8 ~~-19~~

index d79696b..fe10c53 100644
@@ -914,8 +914,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
  	dma_addr_t    auxiliary_phys;
  	unsigned int  i;
  	unsigned char *status;
-	unsigned int  failed;
-	unsigned int  corrected;
+	unsigned int  max_bitflips = 0;
  	int           ret;
  
  	pr_debug("page number is : %d\n", page);

@@ -939,35 +938,25 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
  			payload_virt, payload_phys);
  	if (ret) {
  		pr_err("Error in ECC-based read: %d\n", ret);
-		goto exit_nfc;
+		return ret;
  	}
  
  	/* handle the block mark swapping */
  	block_mark_swapping(this, payload_virt, auxiliary_virt);
  
  	/* Loop over status bytes, accumulating ECC status. */
-	failed		= 0;
-	corrected	= 0;
-	status		= auxiliary_virt + nfc_geo->auxiliary_status_offset;
+	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
  
  	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
  		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
  			continue;
  
  		if (*status == STATUS_UNCORRECTABLE) {
-			failed++;
+			mtd->ecc_stats.failed++;
  			continue;
  		}
-		corrected += *status;
-	}
-
-	/*
-	 * Propagate ECC status to the owning MTD only when failed or
-	 * corrected times nearly reaches our ECC correction threshold.
-	 */
-	if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
-		mtd->ecc_stats.failed    += failed;
-		mtd->ecc_stats.corrected += corrected;
+		mtd->ecc_stats.corrected += *status;
+		max_bitflips = max_t(unsigned int, max_bitflips, *status);
  	}
  
  	if (oob_required) {

@@ -989,8 +978,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
  			this->payload_virt, this->payload_phys,
  			nfc_geo->payload_size,
  			payload_virt, payload_phys);
-exit_nfc:
-	return ret;
+
+	return max_bitflips;
  }
  
  static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

Signature · 1 line

1.7.9.5

Huang Shijie

2012-12-04 03:30 UTC #1 lore raw

Quoted 47 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++-------------------

1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +4 -9

index d79696b..fe10c53 100644
@@ -914,8 +914,7 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 dma_addr_t auxiliary_phys;
 unsigned int i;
 unsigned char *status;
- unsigned int failed;
- unsigned int corrected;
+ unsigned int max_bitflips = 0;
 int ret;
 pr_debug("page number is : %d\n", page);

@@ -939,35 +938,25 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 payload_virt, payload_phys);
 if (ret) {
 pr_err("Error in ECC-based read: %d\n", ret);
- goto exit_nfc;
+ return ret;
 }
 /* handle the block mark swapping */
 block_mark_swapping(this, payload_virt, auxiliary_virt);
 /* Loop over status bytes, accumulating ECC status. */
- failed = 0;
- corrected = 0;
- status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
+ status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
 if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
 continue;
 if (*status == STATUS_UNCORRECTABLE) {
- failed++;
+ mtd->ecc_stats.failed++;
 continue;
 }
- corrected += *status;
- }

drivers/mtd/nand/gpmi-nand/gpmi-nand.c static int gpmi_ecc_read_page(struct mtd_info · line 956

Quoted 10 lines

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +2 -8

-
- /*
- * Propagate ECC status to the owning MTD only when failed or
- * corrected times nearly reaches our ECC correction threshold.
- */
- if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
- mtd->ecc_stats.failed += failed;
- mtd->ecc_stats.corrected += corrected;
+ mtd->ecc_stats.corrected += *status;
+ max_bitflips = max_t(unsigned int, max_bitflips, *status);

Does the max_bitflips stand for the whole page's bitfips ? or just the bitfips of one part of the page? The gpmi driver splits a page into several parts when it does the ECC.

thanks
Huang Shijie

Quoted 13 lines

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +2 -2

 }
 if (oob_required) {

@@ -989,8 +978,8 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 this->payload_virt, this->payload_phys,
 nfc_geo->payload_size,
 payload_virt, payload_phys);
-exit_nfc:
- return ret;
+
+ return max_bitflips;
 }
 static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

Zach Sadecki

author

2012-12-04 14:40 UTC #2 lore raw

Quoted 62 lines · Huang Shijie

Quoted 47 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++-------------------

1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +4 -9

index d79696b..fe10c53 100644
@@ -914,8 +914,7 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 dma_addr_t auxiliary_phys;
 unsigned int i;
 unsigned char *status;
- unsigned int failed;
- unsigned int corrected;
+ unsigned int max_bitflips = 0;
 int ret;
 pr_debug("page number is : %d\n", page);

@@ -939,35 +938,25 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 payload_virt, payload_phys);
 if (ret) {
 pr_err("Error in ECC-based read: %d\n", ret);
- goto exit_nfc;
+ return ret;
 }
 /* handle the block mark swapping */
 block_mark_swapping(this, payload_virt, auxiliary_virt);
 /* Loop over status bytes, accumulating ECC status. */
- failed = 0;
- corrected = 0;
- status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
+ status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
 if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
 continue;
 if (*status == STATUS_UNCORRECTABLE) {
- failed++;
+ mtd->ecc_stats.failed++;
 continue;
 }
- corrected += *status;
- }

drivers/mtd/nand/gpmi-nand/gpmi-nand.c static int gpmi_ecc_read_page(struct mtd_info · line 956

Quoted 10 lines

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +2 -8

-
- /*
- * Propagate ECC status to the owning MTD only when failed or
- * corrected times nearly reaches our ECC correction threshold.
- */
- if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
- mtd->ecc_stats.failed += failed;
- mtd->ecc_stats.corrected += corrected;
+ mtd->ecc_stats.corrected += *status;
+ max_bitflips = max_t(unsigned int, max_bitflips, *status);

Does the max_bitflips stand for the whole page's bitfips ? or just the bitfips of one part of the page? The gpmi driver splits a page into several parts when it does the ECC.

Quoted 3 lines

thanks
Huang Shijie

max_bitflips represents the maximum number of bitflips per ECC corrected region in a page. The MTD layer needs it to compare to bitflip_threshold to know when to return -EUCLEAN to upper layers (like UBI) so a nearly corrupt page can be copied elsewhere. This is pretty much identical to the way other drivers do this as I copied it from those. Here's where it was added in the other drivers: http://git.infradead.org/linux-mtd.git/commit/3f91e94f7f511de74c0d2abe08672ccdbdd1961c

Zach

Quoted 16 lines

Quoted 14 lines

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +2 -2

 }
 if (oob_required) {

@@ -989,8 +978,8 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 this->payload_virt, this->payload_phys,
 nfc_geo->payload_size,
 payload_virt, payload_phys);
-exit_nfc:
- return ret;
+
+ return max_bitflips;
 }
 static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

Huang Shijie

Acked-by

2012-12-05 02:59 UTC #3 lore raw

Quoted 1 line · Zach Sadecki

max_bitflips represents the maximum number of bitflips per ECC corrected region in a page.

got it.

I also tested this patch in the mx6q-arm2 board. it runned well.

thanks.

Acked-by Huang Shijie <[email protected]>

Huang Shijie

Artem Bityutskiy

2012-12-10 13:43 UTC #4 lore raw

In reply to Zach Sadecki

Quoted 4 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.
Signed-off-by Zach Sadecki <[email protected]>

Could you please send a version which applies cleanly to the l2-mtd.git tree? Thanks!

git://git.infradead.org/users/dedekind/l2-mtd.git

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Zach Sadecki

author

2012-12-10 15:10 UTC #5 lore raw

PATCH 1/1 mtd: gpmi: Always report ECC stats and return max_bitflips

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

Acked-by Huang Shijie <[email protected]>

---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +8 ~~-19~~

index 5cd141f..1535bd5 100644
@@ -921,8 +921,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
     dma_addr_t    auxiliary_phys;
     unsigned int  i;
     unsigned char *status;
-    unsigned int  failed;
-    unsigned int  corrected;
+    unsigned int  max_bitflips = 0;
     int           ret;
 
     pr_debug("page number is : %d\n", page);

@@ -946,35 +945,25 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
             payload_virt, payload_phys);
     if (ret) {
         pr_err("Error in ECC-based read: %d\n", ret);
-        goto exit_nfc;
+        return ret;
     }
 
     /* handle the block mark swapping */
     block_mark_swapping(this, payload_virt, auxiliary_virt);
 
     /* Loop over status bytes, accumulating ECC status. */
-    failed        = 0;
-    corrected    = 0;
-    status        = auxiliary_virt + nfc_geo->auxiliary_status_offset;
+    status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
     for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
         if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
             continue;
 
         if (*status == STATUS_UNCORRECTABLE) {
-            failed++;
+            mtd->ecc_stats.failed++;
             continue;
         }
-        corrected += *status;
-    }
-
-    /*
-     * Propagate ECC status to the owning MTD only when failed or
-     * corrected times nearly reaches our ECC correction threshold.
-     */
-    if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
-        mtd->ecc_stats.failed    += failed;
-        mtd->ecc_stats.corrected += corrected;
+        mtd->ecc_stats.corrected += *status;
+        max_bitflips = max_t(unsigned int, max_bitflips, *status);
     }
 
     if (oob_required) {

@@ -996,8 +985,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
             this->payload_virt, this->payload_phys,
             nfc_geo->payload_size,
             payload_virt, payload_phys);
-exit_nfc:
-    return ret;
+
+    return max_bitflips;
 }
 
 static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip

*chip,

Signature · 1 line

1.7.9.5

Zach Sadecki

author

2012-12-10 15:18 UTC #6 lore raw

PATCH 1/1 mtd: gpmi: Always report ECC stats and return max_bitflips

In reply to Artem Bityutskiy

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

Acked-by Huang Shijie <b32955 at freescale.com>

---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +8 ~~-19~~

index 5cd141f..1535bd5 100644
@@ -921,8 +921,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_addr_t    auxiliary_phys;
 	unsigned int  i;
 	unsigned char *status;
-	unsigned int  failed;
-	unsigned int  corrected;
+	unsigned int  max_bitflips = 0;
 	int           ret;
 
 	pr_debug("page number is : %d\n", page);

@@ -946,35 +945,25 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			payload_virt, payload_phys);
 	if (ret) {
 		pr_err("Error in ECC-based read: %d\n", ret);
-		goto exit_nfc;
+		return ret;
 	}
 
 	/* handle the block mark swapping */
 	block_mark_swapping(this, payload_virt, auxiliary_virt);
 
 	/* Loop over status bytes, accumulating ECC status. */
-	failed		= 0;
-	corrected	= 0;
-	status		= auxiliary_virt + nfc_geo->auxiliary_status_offset;
+	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
 	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
 		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
 			continue;
 
 		if (*status == STATUS_UNCORRECTABLE) {
-			failed++;
+			mtd->ecc_stats.failed++;
 			continue;
 		}
-		corrected += *status;
-	}
-
-	/*
-	 * Propagate ECC status to the owning MTD only when failed or
-	 * corrected times nearly reaches our ECC correction threshold.
-	 */
-	if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
-		mtd->ecc_stats.failed    += failed;
-		mtd->ecc_stats.corrected += corrected;
+		mtd->ecc_stats.corrected += *status;
+		max_bitflips = max_t(unsigned int, max_bitflips, *status);
 	}
 
 	if (oob_required) {

@@ -996,8 +985,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			this->payload_virt, this->payload_phys,
 			nfc_geo->payload_size,
 			payload_virt, payload_phys);
-exit_nfc:
-	return ret;
+
+	return max_bitflips;
 }
 
 static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

Signature · 1 line

1.7.9.5

Artem Bityutskiy

2012-12-12 15:16 UTC #7 lore raw

Quoted 8 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.
Signed-off-by Zach Sadecki <[email protected]>
Acked-by Huang Shijie <b32955 at freescale.com>
---
  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 ++++++++-------------------
  1 file changed, 8 insertions(+), 19 deletions(-)

patching file drivers/mtd/nand/gpmi-nand/gpmi-nand.c
Hunk #1 FAILED at 921.
Hunk #2 FAILED at 946.
Hunk #3 FAILED at 996.
3 out of 3 hunks FAILED -- saving rejects to file drivers/mtd/nand/gpmi-nand/gpmi-nand.c.rej

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Zach Sadecki

author

2012-12-12 16:57 UTC #8 lore raw

Quoted 6 lines · Artem Bityutskiy

patching file drivers/mtd/nand/gpmi-nand/gpmi-nand.c
Hunk #1 FAILED at 921.
Hunk #2 FAILED at 946.
Hunk #3 FAILED at 996.
3 out of 3 hunks FAILED -- saving rejects to file drivers/mtd/nand/gpmi-nand/gpmi-nand.c.rej

Odd. I used git format-patch on a freshly cloned l2-mtd repo... I don't know what could be wrong. I did a fresh pull, and another git format-patch and it generated the exact same patch file. Any ideas?

Artem Bityutskiy

2012-12-13 11:26 UTC #9 lore raw

In reply to Zach Sadecki

Quoted 6 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.
Signed-off-by Zach Sadecki <[email protected]>
Acked-by Huang Shijie <b32955 at freescale.com>
---

Dunno, may be your patch is corrupted or something. Try to save it and apply. This is what I get:

$ git apply --check ~/tmp/zach.mbox
error: patch failed: drivers/mtd/nand/gpmi-nand/gpmi-nand.c:921
error: drivers/mtd/nand/gpmi-nand/gpmi-nand.c: patch does not apply

$ git log -1 --oneline
70df8b3 mtd: nand: gpmi: reset BCH earlier, too, to avoid NAND startup problems

I've attached the zach.mbox file for you.

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Zach Sadecki

author

2012-12-14 02:36 UTC #10 lore raw

PATCH 1/1 mtd: gpmi: Always report ECC stats and return max_bitflips

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

Acked-by Huang Shijie <b32955 at freescale.com>

---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +8 ~~-19~~

index 5cd141f..1535bd5 100644
@@ -921,8 +921,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_addr_t    auxiliary_phys;
 	unsigned int  i;
 	unsigned char *status;
-	unsigned int  failed;
-	unsigned int  corrected;
+	unsigned int  max_bitflips = 0;
 	int           ret;
 
 	pr_debug("page number is : %d\n", page);

@@ -946,35 +945,25 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			payload_virt, payload_phys);
 	if (ret) {
 		pr_err("Error in ECC-based read: %d\n", ret);
-		goto exit_nfc;
+		return ret;
 	}
 
 	/* handle the block mark swapping */
 	block_mark_swapping(this, payload_virt, auxiliary_virt);
 
 	/* Loop over status bytes, accumulating ECC status. */
-	failed		= 0;
-	corrected	= 0;
-	status		= auxiliary_virt + nfc_geo->auxiliary_status_offset;
+	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
 	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
 		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
 			continue;
 
 		if (*status == STATUS_UNCORRECTABLE) {
-			failed++;
+			mtd->ecc_stats.failed++;
 			continue;
 		}
-		corrected += *status;
-	}
-
-	/*
-	 * Propagate ECC status to the owning MTD only when failed or
-	 * corrected times nearly reaches our ECC correction threshold.
-	 */
-	if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
-		mtd->ecc_stats.failed    += failed;
-		mtd->ecc_stats.corrected += corrected;
+		mtd->ecc_stats.corrected += *status;
+		max_bitflips = max_t(unsigned int, max_bitflips, *status);
 	}
 
 	if (oob_required) {

@@ -996,8 +985,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			this->payload_virt, this->payload_phys,
 			nfc_geo->payload_size,
 			payload_virt, payload_phys);
-exit_nfc:
-	return ret;
+
+	return max_bitflips;
 }
 
 static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

Signature · 1 line

1.7.9.5

Zach Sadecki

author

2012-12-14 02:36 UTC #11 lore raw

PATCH 1/1 mtd: gpmi: Always report ECC stats and return max_bitflips

In reply to Artem Bityutskiy

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

Acked-by Huang Shijie <[email protected]>

---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +8 ~~-19~~

index 5cd141f..1535bd5 100644
@@ -921,8 +921,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_addr_t    auxiliary_phys;
 	unsigned int  i;
 	unsigned char *status;
-	unsigned int  failed;
-	unsigned int  corrected;
+	unsigned int  max_bitflips = 0;
 	int           ret;
 
 	pr_debug("page number is : %d\n", page);

@@ -946,35 +945,25 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			payload_virt, payload_phys);
 	if (ret) {
 		pr_err("Error in ECC-based read: %d\n", ret);
-		goto exit_nfc;
+		return ret;
 	}
 
 	/* handle the block mark swapping */
 	block_mark_swapping(this, payload_virt, auxiliary_virt);
 
 	/* Loop over status bytes, accumulating ECC status. */
-	failed		= 0;
-	corrected	= 0;
-	status		= auxiliary_virt + nfc_geo->auxiliary_status_offset;
+	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
 	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
 		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
 			continue;
 
 		if (*status == STATUS_UNCORRECTABLE) {
-			failed++;
+			mtd->ecc_stats.failed++;
 			continue;
 		}
-		corrected += *status;
-	}
-
-	/*
-	 * Propagate ECC status to the owning MTD only when failed or
-	 * corrected times nearly reaches our ECC correction threshold.
-	 */
-	if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
-		mtd->ecc_stats.failed    += failed;
-		mtd->ecc_stats.corrected += corrected;
+		mtd->ecc_stats.corrected += *status;
+		max_bitflips = max_t(unsigned int, max_bitflips, *status);
 	}
 
 	if (oob_required) {

@@ -996,8 +985,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			this->payload_virt, this->payload_phys,
 			nfc_geo->payload_size,
 			payload_virt, payload_phys);
-exit_nfc:
-	return ret;
+
+	return max_bitflips;
 }
 
 static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

Signature · 1 line

1.7.9.5

Huang Shijie

2013-01-10 03:02 UTC #12 lore raw

Quoted 77 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.

Signed-off-by Zach Sadecki <[email protected]>

Acked-by Huang Shijie <[email protected]>

---
drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++-------------------

1 file changed, 8 insertions(+), 19 deletions(-)

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +1 -2

index 5cd141f..1535bd5 100644
@@ -921,8 +921,7 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 dma_addr_t auxiliary_phys;
 unsigned int i;
 unsigned char *status;
- unsigned int failed;
- unsigned int corrected;
+ unsigned int max_bitflips = 0;
 int ret;

pr_debug("page number is : %d\n", page);

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +5 ~~-15~~

@@ -946,35 +945,25 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 payload_virt, payload_phys);
 if (ret) {
 pr_err("Error in ECC-based read: %d\n", ret);
- goto exit_nfc;
+ return ret;
 }
 
 /* handle the block mark swapping */
 block_mark_swapping(this, payload_virt, auxiliary_virt);
 
 /* Loop over status bytes, accumulating ECC status. */
- failed = 0;
- corrected = 0;
- status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
+ status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
 for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
 if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
 continue;
 
 if (*status == STATUS_UNCORRECTABLE) {
- failed++;
+ mtd->ecc_stats.failed++;
 continue;
 }
- corrected += *status;
- }
-
- /*
- * Propagate ECC status to the owning MTD only when failed or
- * corrected times nearly reaches our ECC correction threshold.
- */
- if (failed || corrected >= (nfc_geo->ecc_strength - 1)) {
- mtd->ecc_stats.failed += failed;
- mtd->ecc_stats.corrected += corrected;
+ mtd->ecc_stats.corrected += *status;
+ max_bitflips = max_t(unsigned int, max_bitflips, *status);
 }

if (oob_required) {

drivers/mtd/nand/gpmi-nand/gpmi-nand.c +2 -2

@@ -996,8 +985,8 @@ static int gpmi_ecc_read_page(struct mtd_info 
 *mtd, struct nand_chip *chip,
 this->payload_virt, this->payload_phys,
 nfc_geo->payload_size,
 payload_virt, payload_phys);
-exit_nfc:
- return ret;
+
+ return max_bitflips;
 }

Quoted 1 line

static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,

just a ping.

Artem Bityutskiy

Applied

2013-01-15 09:28 UTC #13 lore raw

In reply to Zach Sadecki

Quoted 5 lines · Zach Sadecki

Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.
Signed-off-by Zach Sadecki <[email protected]>
Acked-by Huang Shijie <[email protected]>

Pushed to l2-mtd.git, thanks!

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Zach Sadecki

author

2012-12-14 02:39 UTC #14 lore raw

In reply to Artem Bityutskiy

Quoted 10 lines · Artem Bityutskiy

Quoted 6 lines · Zach Sadecki
Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.
Signed-off-by Zach Sadecki <[email protected]>
Acked-by Huang Shijie <b32955 at freescale.com>
---
Dunno, may be your patch is corrupted or something. Try to save it and apply. This is what I get:

Quoted 9 lines

$ git apply --check ~/tmp/zach.mbox
error: patch failed: drivers/mtd/nand/gpmi-nand/gpmi-nand.c:921
error: drivers/mtd/nand/gpmi-nand/gpmi-nand.c: patch does not apply

$ git log -1 --oneline
70df8b3 mtd: nand: gpmi: reset BCH earlier, too, to avoid NAND startup problems

I've attached the zach.mbox file for you.

OK. I rebased the patch on the latest l2-mtd and I tested it with git apply, so this one should work...

Sent as a separate reply with only the patch in it.

Huang Shijie

2013-01-10 03:29 UTC #15 lore raw

Quoted 15 lines · Zach Sadecki

Quoted 14 lines · Artem Bityutskiy
Quoted 6 lines · Zach Sadecki
Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now.
Signed-off-by Zach Sadecki <[email protected]>
Acked-by Huang Shijie <b32955 at freescale.com>
---
Dunno, may be your patch is corrupted or something. Try to save it and apply. This is what I get:
$ git apply --check ~/tmp/zach.mbox
error: patch failed: drivers/mtd/nand/gpmi-nand/gpmi-nand.c:921
error: drivers/mtd/nand/gpmi-nand/gpmi-nand.c: patch does not apply

Quoted 8 lines

Quoted 5 lines
$ git log -1 --oneline
70df8b3 mtd: nand: gpmi: reset BCH earlier, too, to avoid NAND startup problems
I've attached the zach.mbox file for you.
OK. I rebased the patch on the latest l2-mtd and I tested it with git apply, so this one should work...
Sent as a separate reply with only the patch in it.

Dear Zach,
I think you'd better check the patch with /scripts/checkpatch.pl before you send out the patch. I saw many "trailing whitespace" in your patch.

thanks.

Huang Shijie

Zach Sadecki

author

2013-01-10 15:40 UTC #16 lore raw

Quoted 7 lines · Huang Shijie

Dear Zach,
I think you'd better check the patch with /scripts/checkpatch.pl before you send out the patch. I saw many "trailing whitespace" in your patch.
thanks.
Huang Shijie

I just checked it:

zach@zpc:~/git/l2-mtd$ scripts/checkpatch.pl 0001-mtd-gpmi-Always-report-ECC-stats-and-return-max_bitf.patch
total: 0 errors, 0 warnings, 59 lines checked

0001-mtd-gpmi-Always-report-ECC-stats-and-return-max_bitf.patch has no obvious style problems and is ready for submission.

---

The reply to the original thread dated "Thu Dec 13 21:36:43 EST 2012" should be the correct patch...

Zach