Re: [PATCH v2 2/2] mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages

[Miquel Raynal]

Hi Mehmet,

On 28/08/2026 at 10:53:37 +02, Mehmet Fide <mehmet.fide@xxxxxxxxx> wrote:

> From: Mehmet Fide <mehmet.fide@xxxxxxxxxxxxxxxxxx>
>
> When the ECC engine fails to decode a page, the driver re-reads the OOB
> area with the engine bypassed, but runs the erased-page check for the
> data area on the buffer left in the controller SRAM by the failed
> transfer.
>
> That buffer does not hold what is on the flash: the failing engine
> writes a bogus single-bit "correction" into it. In the 60-byte ECC mode
> the all-0xff content of an erased page always decodes to the same error
> location, so every erased page shows one stale zero bit at data offset
> 0x5FD, which the erased-page check then reports as a corrected bitflip.
>
> Edward Karpicz discovered this behaviour and identified the offset on a
> Colibri VF61; the analysis and the fix build on his finding. Measured
> with an instrumented driver on a Colibri VF50 (MX30LF1G18AC, 32-bit
> ECC): reading a 126 MiB partition with nanddump increased the corrected
> counter by 18035, exactly one per erased page, while raw reads of the
> same pages return clean 0xff. A v4.4 kernel on the VF61 (MX30LF4G28AC)
> accumulates the same false counts, so the behaviour follows the
> controller rather than the chip or the driver generation. Neither the
> Vybrid reference manual nor the published mask set errata (VFXXX_2N02G)
> document it. The 45-byte ECC mode is not affected.
>
> Restoring the known byte is not enough: on pages that fail to decode
> with content other than all-0xff the engine writes its correction
> wherever the syndrome points (measured at a different offset on such a
> page), so the check has to run on what the flash holds. Re-read the data
> area with the ECC engine bypassed, exactly as already done for the OOB
> area. The corrected counter then stays at zero on both boards.
>
> Reported-by: Edward Karpicz <webmaster@xxxxxxxxxxx>
> Link: https://community.toradex.com/t/colibri-vf50-vf61-on-the-current-bsp-mainline-u-boot-v2026-07-and-linux-6-18-lts/30735
> Signed-off-by: Mehmet Fide <mehmet.fide@xxxxxxxxxxxxxxxxxx>
> ---
> v2:
>  - the no-ECC re-read and the erased-page check use the clamped spare
>    size instead of mtd->oobsize
>  - condense the re-read comment to one line
>  - Reported-by/Link trailer order fixed (checkpatch)
>
>  drivers/mtd/nand/raw/vf610_nfc.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
> index 9104db19dd29..ffcf66f96c7f 100644
> --- a/drivers/mtd/nand/raw/vf610_nfc.c
> +++ b/drivers/mtd/nand/raw/vf610_nfc.c
> @@ -520,6 +520,7 @@ static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
>  	u8 ecc_status;
>  	u8 ecc_count;
>  	int flips_threshold = nfc->chip.ecc.strength / 2;
> +	int ret;
>  
>  	ecc_status = vf610_nfc_read(nfc, ecc_status_off) & 0xff;
>  	ecc_count = ecc_status & ECC_STATUS_ERR_COUNT;
> @@ -527,9 +528,15 @@ static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
>  	if (!(ecc_status & ECC_STATUS_MASK))
>  		return ecc_count;
>  
> +	/* The failed decode leaves a bogus correction in SRAM; re-read without ECC */
>  	nfc->data_access = true;
> -	nand_read_oob_op(&nfc->chip, page, 0, oob, vf610_nfc_spare_size(mtd));
> +	ret = nand_read_page_op(&nfc->chip, page, 0, dat,
> nfc->chip.ecc.size);

chip.ecc.size is not covering the whole data buffer. You should be
reading mtd->writesize + mtd->oobsize, no? Otherwise you only overwrite
the first ECC step (out of 2/4/8 depending on the configuration of
the ECC engine).

> +	if (!ret)
> +		ret = nand_read_oob_op(&nfc->chip, page, 0, oob,
> +				       vf610_nfc_spare_size(mtd));
>  	nfc->data_access = false;
> +	if (ret)
> +		return ret;
>  
>  	/*
>  	 * On an erased page, bit count (including OOB) should be zero
>  	or

Unrelated, but this comment is wrong, we accept up to <strength>
bitflips. Not a big deal though, the impact is very limited.

Thanks,
Miquèl