[PATCH] mtd: rawnand: arasan: Check for bitflips in page data & its ECC data

[PATCH] mtd: rawnand: arasan: Check for bitflips in page data & its ECC data

[Amit Kumar Mahapatra]

During page read in HW-ECC mode the driver checks for bitflips to confirm
that the page is erased one or not, currently this check is only for page
data and not for ECC data of that page. Due to this, in case of ECC
corruption, the driver considers some of the page data as erased page data
and overwrites the valid data with 0xFF. Update the driver to include the
ECC bitflips check along with page data while checking for erased ECC data
chunk.

Fixes: 88ffef1b65cf ("mtd: rawnand: arasan: Support the hardware BCH ECC engine")
Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
---
BRANCH: nand/next
---
 drivers/mtd/nand/raw/arasan-nand-controller.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c
index 4621ec549cc7..547cc82b5d80 100644
--- a/drivers/mtd/nand/raw/arasan-nand-controller.c
+++ b/drivers/mtd/nand/raw/arasan-nand-controller.c
@@ -481,7 +481,7 @@ static int anfc_read_page_hw_ecc(struct nand_chip *chip, u8 *buf,
 		}
 
 		bf = nand_check_erased_ecc_chunk(raw_buf, chip->ecc.size,
-						 NULL, 0, NULL, 0,
+						 anand->hw_ecc, chip->ecc.bytes, NULL, 0,
 						 chip->ecc.strength);
 		if (bf > 0) {
 			mtd->ecc_stats.corrected += bf;
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: rawnand: arasan: Check for bitflips in page data & its ECC data

[Miquel Raynal]

Hi Amit,

amit.kumar-mahapatra@amd.com wrote on Tue, 26 Sep 2023 17:43:52 +0530:

> During page read in HW-ECC mode the driver checks for bitflips to confirm
> that the page is erased one or not, currently this check is only for page
> data and not for ECC data of that page. Due to this, in case of ECC
> corruption, the driver considers some of the page data as erased page data
> and overwrites the valid data with 0xFF. Update the driver to include the
> ECC bitflips check along with page data while checking for erased ECC data
> chunk.

The diff below is fine but the commit log is wrong. I would propose:

"
Following an ECC failure condition upon page reads, we shall distinguish
between a real ECC failure and an empty page. This is handled with a
call to nand_check_erased_ecc_chunk() which looks at the data and
counts the number of bits which are not 'ones'. If we get less zeros
than the ECC strength, we assume the page was erased and we are in the
presence of natural bitflips. Otherwise, if we are above, we assume
some data was written and the ECC engine could not recover it all, so we
report an ECC failure.

In order for this logic to be as close as the reality as we can (this
is already a simplified condition but we can hardly be more precise), we
should check all the data that is covered by the ECC step not only the
in-band data, so we should also include the ECC syndrome in the check.
"

In practice it's unlikely that this will make a real difference unless
the NAND chip is old/weared, but it's definitely worth adding.

> Fixes: 88ffef1b65cf ("mtd: rawnand: arasan: Support the hardware BCH ECC engine")
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> ---
> BRANCH: nand/next
> ---
>  drivers/mtd/nand/raw/arasan-nand-controller.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/arasan-nand-controller.c b/drivers/mtd/nand/raw/arasan-nand-controller.c
> index 4621ec549cc7..547cc82b5d80 100644
> --- a/drivers/mtd/nand/raw/arasan-nand-controller.c
> +++ b/drivers/mtd/nand/raw/arasan-nand-controller.c
> @@ -481,7 +481,7 @@ static int anfc_read_page_hw_ecc(struct nand_chip *chip, u8 *buf,
>  		}
>  
>  		bf = nand_check_erased_ecc_chunk(raw_buf, chip->ecc.size,
> -						 NULL, 0, NULL, 0,
> +						 anand->hw_ecc, chip->ecc.bytes, NULL, 0,
>  						 chip->ecc.strength);
>  		if (bf > 0) {
>  			mtd->ecc_stats.corrected += bf;


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/