Skip to contentSkip to conversation

MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

PATCH Superseded Josh Wu 3 participants 6 messages Nov 27, 2012 – Dec 3, 2012

Subsystems (3)
Memory technology devices (MTD) Arm/Microchip (AT91) SoC support NAND flash
lore
Share & tools
lore

Josh Wu

author
#0 lore raw

PATCH v2 1/1 MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable.

In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), The ecc.read_page() is changed to return the maximum number of bitflips.
And when meet uncorrectable bitflips it needs to return 0.

See the comment in nand.h:
* @read_page: function to read a page according to the ECC generator
* requirements; returns maximum number of bitflips corrected in
* any single ECC step, 0 if bitflips uncorrectable, -EIO hw error

Signed-off-by Josh Wu <[email protected]>

---
change since v1:
1. add detail commit message for the fix.
2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion.

 drivers/mtd/nand/atmel_nand.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/mtd/nand/atmel_nand.c +11 -5
index c918386..1669d27 100644
@@ -723,6 +723,7 @@ static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
 	struct atmel_nand_host *host = nand_chip->priv;
 	int i, err_nbr, eccbytes;
 	uint8_t *buf_pos;
+	int total_err = 0;
 
 	eccbytes = nand_chip->ecc.bytes;
 	for (i = 0; i < eccbytes; i++)

@@ -750,12 +751,13 @@ normal_check:
 				pmecc_correct_data(mtd, buf_pos, ecc, i,
 					host->pmecc_bytes_per_sector, err_nbr);
 				mtd->ecc_stats.corrected += err_nbr;
+				total_err += err_nbr;
 			}
 		}
 		pmecc_stat >>= 1;
 	}
 
-	return 0;
+	return total_err;
 }
 
 static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,

@@ -767,6 +769,7 @@ static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 	uint32_t stat;
 	unsigned long end_time;
+	int bitflips = 0;
 
 	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
 	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);

@@ -789,11 +792,14 @@ static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
 	}
 
 	stat = pmecc_readl_relaxed(host->ecc, ISR);
-	if (stat != 0)
-		if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0)
-			return -EIO;
+	if (stat != 0) {
+		bitflips = pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]);
+		if (bitflips < 0)
+			/* uncorrectable errors */
+			return 0;
+	}
 
-	return 0;
+	return bitflips;
 }
 
 static int atmel_nand_pmecc_write_page(struct mtd_info *mtd,
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH v2 1/1 MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

Quoted 11 lines · Josh Wu

This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable.

In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), The ecc.read_page() is changed to return the maximum number of bitflips.
And when meet uncorrectable bitflips it needs to return 0.

See the comment in nand.h:
* @read_page: function to read a page according to the ECC generator
* requirements; returns maximum number of bitflips corrected in
* any single ECC step, 0 if bitflips uncorrectable, -EIO hw error

Quoted 5 lines
Signed-off-by Josh Wu <[email protected]>

---
change since v1:
1. add detail commit message for the fix.
2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion.

Reviewed-by Mike Dunn <[email protected]>

I see now the pmecc controller patch in the git log. Nice work.

Mike

#2 lore raw

PATCH v2 1/1 MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

Quoted 19 lines · Mike Dunn
Quoted 11 lines · Josh Wu

This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable.

In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), The ecc.read_page() is changed to return the maximum number of bitflips.
And when meet uncorrectable bitflips it needs to return 0.

See the comment in nand.h:
* @read_page: function to read a page according to the ECC generator
* requirements; returns maximum number of bitflips corrected in
* any single ECC step, 0 if bitflips uncorrectable, -EIO hw error

Quoted 5 lines
Signed-off-by Josh Wu <[email protected]>

---
change since v1:
1. add detail commit message for the fix.
2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion.

Quoted 3 lines
Reviewed-by Mike Dunn <[email protected]>

I see now the pmecc controller patch in the git log. Nice work.

BTW, with such a wide range for ecc strength - up to 24 bits, according to the commit message for the pmecc patch - you may want to think about setting an appropriate bitflip_threshold in the driver. I'm not a nand expert, and I don't know much about the atmel_nand specifically, but I would think that if 23 bits are corrected on a page of size 2k (or less), maybe a return code of -EUCLEAN from mtd_read() might be appropriate.

Mike

author
#3 lore raw

PATCH v2 1/1 MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

Hi, Mike

Quoted 21 lines · Mike Dunn
Quoted 20 lines · Mike Dunn
Quoted 11 lines · Josh Wu

This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable.

In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), The ecc.read_page() is changed to return the maximum number of bitflips.
And when meet uncorrectable bitflips it needs to return 0.

See the comment in nand.h:
* @read_page: function to read a page according to the ECC generator
* requirements; returns maximum number of bitflips corrected in
* any single ECC step, 0 if bitflips uncorrectable, -EIO hw error

Quoted 5 lines
Signed-off-by Josh Wu <[email protected]>

---
change since v1:
1. add detail commit message for the fix.
2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion.

Reviewed-by Mike Dunn <[email protected]>
Quoted 8 lines
Quoted 1 line

I see now the pmecc controller patch in the git log. Nice work.

BTW, with such a wide range for ecc strength - up to 24 bits, according to the commit message for the pmecc patch - you may want to think about setting an appropriate bitflip_threshold in the driver. I'm not a nand expert, and I don't know much about the atmel_nand specifically, but I would think that if 23 bits are corrected on a page of size 2k (or less), maybe a return code of -EUCLEAN from mtd_read() might be appropriate.

After checking the nand_base.c, I saw it will set the mtd.bitflip_threshold to mtd->ecc.strength during nand_scan_tail(). in the atmel_nand code, the ecc strength will be set correctly, that means bitflip_threashold should be set up correctly by default. so I think I don't need set up the the bitflip_threshold anymore if I set ecc strength correctly. Am I missing any point here?

Thanks,
Josh Wu

Quoted 2 lines

Mike

#4 lore raw

PATCH v2 1/1 MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

Quoted 11 lines · Josh Wu

Hi, Mike

Quoted 7 lines · Mike Dunn

BTW, with such a wide range for ecc strength - up to 24 bits, according to the commit message for the pmecc patch - you may want to think about setting an appropriate bitflip_threshold in the driver. I'm not a nand expert, and I don't know much about the atmel_nand specifically, but I would think that if 23 bits are corrected on a page of size 2k (or less), maybe a return code of -EUCLEAN from mtd_read() might be appropriate.

Quoted 6 lines

After checking the nand_base.c, I saw it will set the mtd.bitflip_threshold to mtd->ecc.strength during nand_scan_tail().
in the atmel_nand code, the ecc strength will be set correctly, that means bitflip_threashold should be set up correctly by default.
so I think I don't need set up the the bitflip_threshold anymore if I set ecc strength correctly. Am I missing any point here?

No, you are correct; bitflip_threshold is set to ecc.strength by default if the driver has not assigned it a value already. I just wondered if the default value is too liberal given the possibility of such high ecc strength in your case. With bitflip_threshold == 24 (default for the greatest possible ecc.strength on atmel_nand with pmecc) mtd_read() will not return -EUCLEAN until 24 bitflips are corrected on a page that is no greater than 2k in size, and may be as small as 512 bytes (if I read the code correctly).

The -EUCLEAN return code is used by higher layers as an indication that a block may be going bad. I don't know whether or not a block bears scrutiny when 23 bitflips are corrected on a page. I guess it depends on the particular flash device. It was just a thought.

Hope this makes sense.
Mike

Applied
#5 lore raw

PATCH v2 1/1 MTD: at91: atmel_nand: return bit flips for the PMECC read_page()

In reply to Josh Wu
Quoted 6 lines · Josh Wu

This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable.

In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), The ecc.read_page() is changed to return the maximum number of bitflips.
And when meet uncorrectable bitflips it needs to return 0.

Quoted 6 lines

See the comment in nand.h:
* @read_page: function to read a page according to the ECC generator
* requirements; returns maximum number of bitflips corrected in
* any single ECC step, 0 if bitflips uncorrectable, -EIO hw error

Signed-off-by Josh Wu <[email protected]>

Pushed to l2-mtd.git, thanks!

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121203/3dc8bf5f/attachment.sig>