[PATCH v2 0/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[PATCH v2 0/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

The driver only implements the 64-byte OOB format the controller
transfers, so chips with a larger OOB (the Colibri VF61's MX30LF4G28AC
has 112 bytes) stopped working when nanddev_init() began restoring
mtd->oobsize after ->attach_chip(): the parity moved and every
ECC-protected read failed, including the BBT and everything UBI needs.

v2 takes the approach Miquel suggested instead of clamping the memory
organization: the detected OOB size stays, the driver gets its own
mtd_ooblayout_ops computed on the first 64 OOB bytes, and the data
paths keep transferring exactly those 64 spare bytes, so the on-flash
format stays identical to U-Boot and to the kernels that clamped.

Tested on a Colibri VF61 (112-byte OOB): mtd->oobsize now reads 112,
the flash-based BBT is found and read without errors, UBIFS written by
a clamping kernel mounts read-write, and an 8 MiB write/read-back
returns identical data with zero corrected bits. Regression-tested on a
Colibri VF50 (64-byte OOB chip): unchanged layout, oobavail and clean
ECC counters.

The two pre-existing issues the Sashiko report flagged on v1 are sent
as a separate series, as they are independent of this fix.

v1: https://lore.kernel.org/linux-mtd/20260818114208.2780311-1-mehmet.fide@gmail.com/


Mehmet Fide (2):
  mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of
    OOB
  mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages

 drivers/mtd/nand/raw/vf610_nfc.c | 66 ++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 11 deletions(-)

-- 
2.54.0


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

[PATCH v2 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

The controller transfers 64 spare bytes per page and the driver only
implements the matching 64-byte ECC layout, so attach_chip() shrinks
mtd->oobsize when the chip provides more. That clamp does not survive:
nand_scan_tail() runs nanddev_init() after ->attach_chip(), and it
restores mtd->oobsize from the memory organization, which still holds
the value detected from the chip. The driver then transfers writesize
plus the chip's full OOB size, the hardware ECC parity ends up at a
different offset than the layout the controller was set up for, and
every ECC-protected read fails with -EBADMSG.

Measured on a Colibri VF61 (MX30LF4G28AC, 2048-byte pages, 112 bytes of
OOB): with the clamp lost, UBI cannot read the erase counter headers of
the pages U-Boot has just written, and the on-flash bad block table
written by an older kernel reads back with ECC errors, so the board
does not boot. Kernels before commit a7ab085d7c16 ("mtd: rawnand:
Initialize the nand_device object") are not affected because nothing
overwrote the clamp there, which is why the same chip works with a v4.4
kernel and with U-Boot, whose copy of this driver has no memory
organization to restore the value from. Edward Karpicz reported that
the clamp no longer takes effect on this chip; see the link below.

Instead of modifying the memory organization, keep the detected OOB
size and give the driver its own mtd_ooblayout_ops: the same layout the
NAND core uses for large pages, but computed on the first 64 OOB bytes
instead of the whole OOB, so the ECC bytes stay where U-Boot and the
old kernels put them. The data paths transfer writesize plus those 64
bytes, as the controller always has.

Reported-by: Edward Karpicz <webmaster@toradex.com>
Link: https://community.toradex.com/t/colibri-vf50-vf61-on-the-current-bsp-mainline-u-boot-v2026-07-and-linux-6-18-lts/30735
Fixes: a7ab085d7c16 ("mtd: rawnand: Initialize the nand_device object")
Cc: stable@vger.kernel.org
Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
v2:
 - keep the detected OOB size and add driver ooblayout_ops computed on
   the first 64 OOB bytes instead of clamping the memory organization
   (Miquel)
 - clamp the spare transfer size in the data paths so the controller
   keeps reading and writing 64 spare bytes
 - drop the truncation dev_info() and with it the %d format for a u32
   (Sashiko report)

 drivers/mtd/nand/raw/vf610_nfc.c | 59 ++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index 9940681810cf..9104db19dd29 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -505,6 +505,12 @@ static int vf610_nfc_exec_op(struct nand_chip *chip,
 				      check_only);
 }
 
+/* The controller transfers 64 spare bytes; larger OOBs keep using the first 64 */
+static inline unsigned int vf610_nfc_spare_size(struct mtd_info *mtd)
+{
+	return min_t(unsigned int, mtd->oobsize, 64);
+}
+
 static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
 					 uint8_t *oob, int page)
 {
@@ -522,7 +528,7 @@ static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
 		return ecc_count;
 
 	nfc->data_access = true;
-	nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize);
+	nand_read_oob_op(&nfc->chip, page, 0, oob, vf610_nfc_spare_size(mtd));
 	nfc->data_access = false;
 
 	/*
@@ -530,7 +536,7 @@ static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
 	 * at least less then half of the ECC strength.
 	 */
 	return nand_check_erased_ecc_chunk(dat, nfc->chip.ecc.size, oob,
-					   mtd->oobsize, NULL, 0,
+					   vf610_nfc_spare_size(mtd), NULL, 0,
 					   flips_threshold);
 }
 
@@ -551,7 +557,7 @@ static int vf610_nfc_read_page(struct nand_chip *chip, uint8_t *buf,
 {
 	struct vf610_nfc *nfc = chip_to_nfc(chip);
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	int trfr_sz = mtd->writesize + mtd->oobsize;
+	int trfr_sz = mtd->writesize + vf610_nfc_spare_size(mtd);
 	u32 row = 0, cmd1 = 0, cmd2 = 0, code = 0;
 	int stat;
 
@@ -581,7 +587,7 @@ static int vf610_nfc_read_page(struct nand_chip *chip, uint8_t *buf,
 		vf610_nfc_rd_from_sram(chip->oob_poi,
 				       nfc->regs + NFC_MAIN_AREA(0) +
 						   mtd->writesize,
-				       mtd->oobsize, false);
+				       vf610_nfc_spare_size(mtd), false);
 
 	stat = vf610_nfc_correct_data(chip, buf, chip->oob_poi, page);
 
@@ -599,7 +605,7 @@ static int vf610_nfc_write_page(struct nand_chip *chip, const uint8_t *buf,
 {
 	struct vf610_nfc *nfc = chip_to_nfc(chip);
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	int trfr_sz = mtd->writesize + mtd->oobsize;
+	int trfr_sz = mtd->writesize + vf610_nfc_spare_size(mtd);
 	u32 row = 0, cmd1 = 0, cmd2 = 0, code = 0;
 	u8 status;
 	int ret;
@@ -740,6 +746,42 @@ static void vf610_nfc_init_controller(struct vf610_nfc *nfc)
 	}
 }
 
+/* The default large page layout, clamped to the 64 transferred bytes */
+static int vf610_nfc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+
+	if (section || !total_ecc_bytes)
+		return -ERANGE;
+
+	oobregion->length = total_ecc_bytes;
+	oobregion->offset = vf610_nfc_spare_size(mtd) - oobregion->length;
+
+	return 0;
+}
+
+static int vf610_nfc_ooblayout_free(struct mtd_info *mtd, int section,
+				    struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
+
+	if (section)
+		return -ERANGE;
+
+	oobregion->length = vf610_nfc_spare_size(mtd) - total_ecc_bytes - 2;
+	oobregion->offset = 2;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops vf610_nfc_ooblayout_ops = {
+	.ecc = vf610_nfc_ooblayout_ecc,
+	.free = vf610_nfc_ooblayout_free,
+};
+
 static int vf610_nfc_attach_chip(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
@@ -770,12 +812,7 @@ static int vf610_nfc_attach_chip(struct nand_chip *chip)
 		return -ENXIO;
 	}
 
-	/* Only 64 byte ECC layouts known */
-	if (mtd->oobsize > 64)
-		mtd->oobsize = 64;
-
-	/* Use default large page ECC layout defined in NAND core */
-	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
+	mtd_set_ooblayout(mtd, &vf610_nfc_ooblayout_ops);
 	if (chip->ecc.strength == 32) {
 		nfc->ecc_mode = ECC_60_BYTE;
 		chip->ecc.bytes = 60;
-- 
2.54.0


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

Re: [PATCH v2 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Miquel Raynal]

Hi Mehmet,

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

> From: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> The controller transfers 64 spare bytes per page and the driver only
> implements the matching 64-byte ECC layout, so attach_chip() shrinks
> mtd->oobsize when the chip provides more. That clamp does not survive:
> nand_scan_tail() runs nanddev_init() after ->attach_chip(), and it
> restores mtd->oobsize from the memory organization, which still holds
> the value detected from the chip. The driver then transfers writesize
> plus the chip's full OOB size, the hardware ECC parity ends up at a
> different offset than the layout the controller was set up for, and
> every ECC-protected read fails with -EBADMSG.
>
> Measured on a Colibri VF61 (MX30LF4G28AC, 2048-byte pages, 112 bytes of
> OOB): with the clamp lost, UBI cannot read the erase counter headers of
> the pages U-Boot has just written, and the on-flash bad block table
> written by an older kernel reads back with ECC errors, so the board
> does not boot. Kernels before commit a7ab085d7c16 ("mtd: rawnand:
> Initialize the nand_device object") are not affected because nothing
> overwrote the clamp there, which is why the same chip works with a v4.4
> kernel and with U-Boot, whose copy of this driver has no memory
> organization to restore the value from. Edward Karpicz reported that
> the clamp no longer takes effect on this chip; see the link below.
>
> Instead of modifying the memory organization, keep the detected OOB
> size and give the driver its own mtd_ooblayout_ops: the same layout the
> NAND core uses for large pages, but computed on the first 64 OOB bytes
> instead of the whole OOB, so the ECC bytes stay where U-Boot and the
> old kernels put them. The data paths transfer writesize plus those 64
> bytes, as the controller always has.
>
> Reported-by: Edward Karpicz <webmaster@toradex.com>
> Link: https://community.toradex.com/t/colibri-vf50-vf61-on-the-current-bsp-mainline-u-boot-v2026-07-and-linux-6-18-lts/30735
> Fixes: a7ab085d7c16 ("mtd: rawnand: Initialize the nand_device object")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
> ---
> v2:
>  - keep the detected OOB size and add driver ooblayout_ops computed on
>    the first 64 OOB bytes instead of clamping the memory organization
>    (Miquel)

Since it is a total rewrite of the former approach, this is probably a
good candidate for a Suggested-by.

>  - clamp the spare transfer size in the data paths so the controller
>    keeps reading and writing 64 spare bytes
>  - drop the truncation dev_info() and with it the %d format for a u32
>    (Sashiko report)
>
>  drivers/mtd/nand/raw/vf610_nfc.c | 59 ++++++++++++++++++++++++++------
>  1 file changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
> index 9940681810cf..9104db19dd29 100644
> --- a/drivers/mtd/nand/raw/vf610_nfc.c
> +++ b/drivers/mtd/nand/raw/vf610_nfc.c
> @@ -505,6 +505,12 @@ static int vf610_nfc_exec_op(struct nand_chip *chip,
>  				      check_only);
>  }
>  
> +/* The controller transfers 64 spare bytes; larger OOBs keep using
> the first 64 */

Is it a real controller constraint? Or is this a compatibility fix only?
If this is a real constraint, you can keep the comment, otherwise I
would drop it.

> +static inline unsigned int vf610_nfc_spare_size(struct mtd_info *mtd)

No explicit inline please.

> +{
> +	return min_t(unsigned int, mtd->oobsize, 64);
> +}
> +
>  static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
>  					 uint8_t *oob, int page)
>  {
> @@ -522,7 +528,7 @@ static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
>  		return ecc_count;
>  
>  	nfc->data_access = true;
> -	nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize);
> +	nand_read_oob_op(&nfc->chip, page, 0, oob, vf610_nfc_spare_size(mtd));
>  	nfc->data_access = false;
>  
>  	/*
> @@ -530,7 +536,7 @@ static inline int vf610_nfc_correct_data(struct nand_chip *chip, uint8_t *dat,
>  	 * at least less then half of the ECC strength.
>  	 */
>  	return nand_check_erased_ecc_chunk(dat, nfc->chip.ecc.size, oob,
> -					   mtd->oobsize, NULL, 0,
> +					   vf610_nfc_spare_size(mtd), NULL, 0,
>  					   flips_threshold);
>  }
>  
> @@ -551,7 +557,7 @@ static int vf610_nfc_read_page(struct nand_chip *chip, uint8_t *buf,
>  {
>  	struct vf610_nfc *nfc = chip_to_nfc(chip);
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> -	int trfr_sz = mtd->writesize + mtd->oobsize;
> +	int trfr_sz = mtd->writesize + vf610_nfc_spare_size(mtd);
>  	u32 row = 0, cmd1 = 0, cmd2 = 0, code = 0;
>  	int stat;
>  
> @@ -581,7 +587,7 @@ static int vf610_nfc_read_page(struct nand_chip *chip, uint8_t *buf,
>  		vf610_nfc_rd_from_sram(chip->oob_poi,
>  				       nfc->regs + NFC_MAIN_AREA(0) +
>  						   mtd->writesize,
> -				       mtd->oobsize, false);
> +				       vf610_nfc_spare_size(mtd), false);
>  
>  	stat = vf610_nfc_correct_data(chip, buf, chip->oob_poi, page);
>  
> @@ -599,7 +605,7 @@ static int vf610_nfc_write_page(struct nand_chip *chip, const uint8_t *buf,
>  {
>  	struct vf610_nfc *nfc = chip_to_nfc(chip);
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> -	int trfr_sz = mtd->writesize + mtd->oobsize;
> +	int trfr_sz = mtd->writesize + vf610_nfc_spare_size(mtd);
>  	u32 row = 0, cmd1 = 0, cmd2 = 0, code = 0;
>  	u8 status;
>  	int ret;
> @@ -740,6 +746,42 @@ static void vf610_nfc_init_controller(struct vf610_nfc *nfc)
>  	}
>  }
>  
> +/* The default large page layout, clamped to the 64 transferred bytes */
> +static int vf610_nfc_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				   struct mtd_oob_region *oobregion)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
> +
> +	if (section || !total_ecc_bytes)
> +		return -ERANGE;
> +
> +	oobregion->length = total_ecc_bytes;
> +	oobregion->offset = vf610_nfc_spare_size(mtd) - oobregion->length;
> +
> +	return 0;
> +}
> +
> +static int vf610_nfc_ooblayout_free(struct mtd_info *mtd, int section,
> +				    struct mtd_oob_region *oobregion)
> +{
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	unsigned int total_ecc_bytes = nand->ecc.ctx.total;
> +
> +	if (section)
> +		return -ERANGE;
> +
> +	oobregion->length = vf610_nfc_spare_size(mtd) - total_ecc_bytes - 2;
> +	oobregion->offset = 2;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops vf610_nfc_ooblayout_ops = {
> +	.ecc = vf610_nfc_ooblayout_ecc,
> +	.free = vf610_nfc_ooblayout_free,
> +};
> +
>  static int vf610_nfc_attach_chip(struct nand_chip *chip)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> @@ -770,12 +812,7 @@ static int vf610_nfc_attach_chip(struct nand_chip *chip)
>  		return -ENXIO;
>  	}
>  
> -	/* Only 64 byte ECC layouts known */
> -	if (mtd->oobsize > 64)
> -		mtd->oobsize = 64;
> -
> -	/* Use default large page ECC layout defined in NAND core */

Please modify this comment to express why we use our own layout here.

> -	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
> +	mtd_set_ooblayout(mtd, &vf610_nfc_ooblayout_ops);
>  	if (chip->ecc.strength == 32) {
>  		nfc->ecc_mode = ECC_60_BYTE;
>  		chip->ecc.bytes = 60;

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

Re: [PATCH v2 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Mehmet Fide]

Hi Miquel,

thanks for the review.

> Since it is a total rewrite of the former approach, this is probably a
> good candidate for a Suggested-by.

Of course, I will add your Suggested-by in v3.

> > +/* The controller transfers 64 spare bytes; larger OOBs keep using
> > the first 64 */
>
> Is it a real controller constraint? Or is this a compatibility fix only?
> If this is a real constraint, you can keep the comment, otherwise I
> would drop it.

Compatibility only, so I will drop it. The SRAM row buffer takes up to
248 spare bytes and the ECC engine computes parity for whatever length
is transferred - that is exactly how the bug bites, the parity moves
with the transfer size. The 64 is the on-flash format that U-Boot's
copy of this driver and the kernels before a7ab085d7c16 wrote, and the
explanation belongs at the ooblayout, which brings us to your last
point.

> No explicit inline please.

Dropped.

> Please modify this comment to express why we use our own layout here.

Will do. Something along the lines of: the core's large page layout,
computed over the first 64 spare bytes instead of the whole OOB, so the
ECC bytes stay at the offsets the established on-flash format uses,
while mtd->oobsize keeps reporting the chip's real spare size.

Thanks,
Mehmet

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

Re: [PATCH v2 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Miquel Raynal]

On 31/08/2026 at 13:39:59 +02, Mehmet Fide <mehmet.fide@gmail.com> wrote:

> Hi Miquel,
>
> thanks for the review.
>
>> Since it is a total rewrite of the former approach, this is probably a
>> good candidate for a Suggested-by.
>
> Of course, I will add your Suggested-by in v3.
>
>> > +/* The controller transfers 64 spare bytes; larger OOBs keep using
>> > the first 64 */
>>
>> Is it a real controller constraint? Or is this a compatibility fix only?
>> If this is a real constraint, you can keep the comment, otherwise I
>> would drop it.
>
> Compatibility only, so I will drop it. The SRAM row buffer takes up to
> 248 spare bytes and the ECC engine computes parity for whatever length
> is transferred - that is exactly how the bug bites, the parity moves
> with the transfer size. The 64 is the on-flash format that U-Boot's
> copy of this driver and the kernels before a7ab085d7c16 wrote, and the
> explanation belongs at the ooblayout, which brings us to your last
> point.
>
>> No explicit inline please.
>
> Dropped.
>
>> Please modify this comment to express why we use our own layout here.
>
> Will do. Something along the lines of: the core's large page layout,
> computed over the first 64 spare bytes instead of the whole OOB, so the
> ECC bytes stay at the offsets the established on-flash format uses,
> while mtd->oobsize keeps reporting the chip's real spare size.

I would instead talk about the fact that with 64B OOB chips, the core's
large page layout was matching U-Boot, but U-Boot and older kernel where
clamping the OOB size to 64 if that size was bigger. Modifying the OOB
size is no longer possible (we must respect the actual chip geometry)
but to avoid breaking existing setups, we need our own layout which
only exposes 64 bytes.

Thanks,
Miquèl

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

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

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

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@toradex.com>
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@screeningeagle.com>
---
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);
+	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
-- 
2.54.0


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

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@gmail.com> wrote:

> From: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> 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@toradex.com>
> 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@screeningeagle.com>
> ---
> 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

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

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

[Mehmet Fide]

Hi Miquel,

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

On this controller there is only ever one step: attach_chip() rejects
any hwecc setup where ecc.size differs from the page size ("Step size
needs to be page size") and the engine protects the whole page in one
go, so ecc.size always equals the full data area here. But you are
right that mtd->writesize says that much more clearly, so I will use it
in v3.

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

I will reword that comment in v3 to match what the code actually passes
(the driver's historic threshold of half the strength). If you would
rather see the threshold itself aligned to <strength>, I can send that
as a separate cleanup so it does not hide in a fix.

Thanks,
Mehmet

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

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

[Miquel Raynal]

On 31/08/2026 at 13:40:00 +02, Mehmet Fide <mehmet.fide@gmail.com> wrote:

> Hi Miquel,
>
>> 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).
>
> On this controller there is only ever one step: attach_chip() rejects
> any hwecc setup where ecc.size differs from the page size ("Step size
> needs to be page size") and the engine protects the whole page in one
> go, so ecc.size always equals the full data area here. But you are
> right that mtd->writesize says that much more clearly, so I will use it
> in v3.

Ah ok, then yes that would be nice.

>> Unrelated, but this comment is wrong, we accept up to <strength>
>> bitflips. Not a big deal though, the impact is very limited.
>
> I will reword that comment in v3 to match what the code actually passes
> (the driver's historic threshold of half the strength). If you would
> rather see the threshold itself aligned to <strength>, I can send that
> as a separate cleanup so it does not hide in a fix.

Do not hide it indeed, let the threshold as it is for this series. If
you want, you may send a patch to increase that threshold.

Thanks,
Miquèl

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

Re: [PATCH v2 0/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Miquel Raynal]

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

> From: Mehmet Fide <mehmet.fide@screeningeagle.com>

Sashiko says:

> New issues:
> - [High] Kernel heap memory is leaked to userspace during out-of-band
> (OOB) reads when the NAND chip's OOB size is larger than 64 bytes.

Probably right, to be checked.

> - [Medium] Integer underflows occur in OOB layout functions when the
> flash chip's spare size is smaller than the required ECC bytes + 2,
> leading to an inflated `mtd->oobavail` and potential heap buffer
> overflow.

Cannot happen.

> Pre-existing issues:
> - [High] `vf610_nfc_write_page()` completely ignores the `oob_required`
> parameter and fails to copy the caller's OOB data into the controller's
> SRAM, leading to stale data written to the flash.

Probably true.

Cheers,
Miquèl

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

Re: [PATCH v2 0/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB

[Mehmet Fide]

Hi Miquel,

> Sashiko says:
>
> > New issues:
> > - [High] Kernel heap memory is leaked to userspace during out-of-band
> > (OOB) reads when the NAND chip's OOB size is larger than 64 bytes.
>
> Probably right, to be checked.

Checked, and Sashiko is right. vf610_nfc_read_page() fills only the
first 64 bytes of oob_poi while the core is free to copy the full
mtd->oobsize from it on an MTD_OPS_PLACE_OOB read, so the remaining
bytes expose whatever the buffer held before. The raw paths are fine,
they bypass the engine and transfer the chip's real spare area. v3 will
fill the tail of oob_poi with 0xff after the copy, which also matches
what raw reads see on flash, since the write path only ever programs
the first 64 spare bytes.

> > - [Medium] Integer underflows occur in OOB layout functions when the
> > flash chip's spare size is smaller than the required ECC bytes + 2,
> > leading to an inflated `mtd->oobavail` and potential heap buffer
> > overflow.
>
> Cannot happen.

Agreed: the layout is only installed in the hwecc path, where
attach_chip() rejects chips with less than 64 bytes of OOB, and the
largest ECC mode uses 60 bytes + 2, which still fits.

> > Pre-existing issues:
> > - [High] `vf610_nfc_write_page()` completely ignores the `oob_required`
> > parameter and fails to copy the caller's OOB data into the controller's
> > SRAM, leading to stale data written to the flash.
>
> Probably true.

It is true, and it is exactly what the first patch of the other series
I posted the same day fixes:

https://lore.kernel.org/linux-mtd/20260828085340.3916239-2-mehmet.fide@gmail.com/

One correction to that series' cover letter while we are here: it calls
the two fixes independent of this one, but its first patch uses the
vf610_nfc_spare_size() helper this series introduces, so it only builds
on top of it. Apply order is this series first.

Thanks,
Mehmet

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