Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] mtd: rawnand: vf610_nfc: two fixes for chips with large OOB and for erased pages
@ 2026-08-18 11:42 Mehmet Fide
  2026-08-18 11:42 ` [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB Mehmet Fide
  2026-08-18 11:42 ` [PATCH 2/2] mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages Mehmet Fide
  0 siblings, 2 replies; 4+ messages in thread
From: Mehmet Fide @ 2026-08-18 11:42 UTC (permalink / raw)
  To: Stefan Agner, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: Mehmet Fide, Boris Brezillon, Frieder Schrempf, Edward Karpicz,
	linux-mtd, linux-kernel

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

Two independent fixes for the Vybrid NAND flash controller, both found
while bringing a Colibri VF61 up on mainline and both verified on
hardware.

Patch 1 is a regression. The driver clamps mtd->oobsize to the 64 bytes
its ECC layout uses, but since commit a7ab085d7c16 ("mtd: rawnand:
Initialize the nand_device object") nand_scan_tail() restores the value
from the memory organization right after ->attach_chip(), so the clamp
is silently lost. On a chip with more than 64 bytes of OOB every
ECC-protected read then fails and the board does not boot. Clamping the
memory organization as well is what the rest of the tree does when a
driver has to change the OOB size (nand_samsung.c, nand_onfi.c,
nand_jedec.c and denali.c all write memorg->oobsize); mpc5121_nfc.c
touches mtd->oobsize directly but does so before nand_scan(), so it is
not affected.

Doing this in ->attach_chip() is deliberate: the chip really does have
112 spare bytes, it is this controller that can only use 64, and
->attach_chip() is where a controller adapts to the detected chip. The
patch also touches the memory organization because nanddev_init(), which
runs later in nand_scan_tail(), re-derives mtd->{erasesize, writesize,
writebufsize, oobsize, size} from it, so an adjustment made only in mtd
does not survive. If you would rather have the core preserve what
->attach_chip() set up, or a dedicated way for a driver to declare that
it uses fewer OOB bytes than the chip provides, I am happy to respin; I
went with the minimal form because this is a regression fix that should
be backportable.

Patch 2 makes the erased-page check look at the flash instead of at the
controller buffer. When the ECC engine fails to decode a page it leaves
a bogus single-bit "correction" in that buffer, which the check counts
as a real bitflip; on the 60-byte ECC mode every erased page is
reported with one corrected bitflip, which renders the MTD statistics
useless for flash health monitoring.

Testing, all booting from NAND on an Iris carrier with Linux 6.18.44 and
U-Boot 2026.07:

  0010 Colibri VF50 128MB V1.2A     Macronix MX30LF1G08AA, 64-byte OOB
  0013 Colibri VF50 128MB IT V1.2B  Macronix MX30LF1G18AC, 64-byte OOB
  0012 Colibri VF61 256MB IT V1.2B  Macronix MX30LF4G28AC, 112-byte OOB

Without patch 1 the VF61 cannot attach UBI at all. With the series all
three boot, mtd oobsize stays 64, the bad block table written by an
older kernel reads back without ECC errors and the corrected-bitflip
counter stays at zero over a full-partition nanddump. The clamp only
triggers on the VF61 chip, so both patches are a no-op on the two VF50
revisions, which is what I wanted to confirm before sending them.

The credit for spotting both problems goes to Edward Karpicz, who
reported them on the Toradex community forum.

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 | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)


base-commit: 15a3cbce32994141252bb4ecfe3ff3a5d22d0b4f
-- 
2.54.0


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB
  2026-08-18 11:42 [PATCH 0/2] mtd: rawnand: vf610_nfc: two fixes for chips with large OOB and for erased pages Mehmet Fide
@ 2026-08-18 11:42 ` Mehmet Fide
  2026-08-25  9:52   ` Miquel Raynal
  2026-08-18 11:42 ` [PATCH 2/2] mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages Mehmet Fide
  1 sibling, 1 reply; 4+ messages in thread
From: Mehmet Fide @ 2026-08-18 11:42 UTC (permalink / raw)
  To: Stefan Agner, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: Mehmet Fide, Boris Brezillon, Frieder Schrempf, Edward Karpicz,
	linux-mtd, linux-kernel, stable

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

The driver only implements the 64-byte OOB 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 therefore transfers writesize + the chip's full OOB size, so
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.

Clamp the memory organization as well so the driver's 64-byte layout
stays in effect, and log the truncation once, as it decides which
on-flash layout the system uses.

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>
---
 drivers/mtd/nand/raw/vf610_nfc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index 9940681810cf..f27ef2b0884d 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -771,8 +771,14 @@ static int vf610_nfc_attach_chip(struct nand_chip *chip)
 	}
 
 	/* Only 64 byte ECC layouts known */
-	if (mtd->oobsize > 64)
+	if (mtd->oobsize > 64) {
+		dev_info(nfc->dev,
+			 "using 64 of %d OOB bytes, ECC layout is unchanged\n",
+			 mtd->oobsize);
 		mtd->oobsize = 64;
+		/* nand_scan_tail() restores mtd->oobsize from the memorg */
+		nanddev_get_memorg(&chip->base)->oobsize = 64;
+	}
 
 	/* Use default large page ECC layout defined in NAND core */
 	mtd_set_ooblayout(mtd, nand_get_large_page_ooblayout());
-- 
2.54.0


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages
  2026-08-18 11:42 [PATCH 0/2] mtd: rawnand: vf610_nfc: two fixes for chips with large OOB and for erased pages Mehmet Fide
  2026-08-18 11:42 ` [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB Mehmet Fide
@ 2026-08-18 11:42 ` Mehmet Fide
  1 sibling, 0 replies; 4+ messages in thread
From: Mehmet Fide @ 2026-08-18 11:42 UTC (permalink / raw)
  To: Stefan Agner, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: Mehmet Fide, Boris Brezillon, Frieder Schrempf, Edward Karpicz,
	linux-mtd, linux-kernel

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>
---
 drivers/mtd/nand/raw/vf610_nfc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index f27ef2b0884d..d41750a4352c 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -514,6 +514,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;
@@ -521,9 +522,17 @@ 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 the SRAM buffer,
+	 * so re-read the data without ECC too, as already done for the OOB.
+	 */
 	nfc->data_access = true;
-	nand_read_oob_op(&nfc->chip, page, 0, oob, mtd->oobsize);
+	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, mtd->oobsize);
 	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/

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB
  2026-08-18 11:42 ` [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB Mehmet Fide
@ 2026-08-25  9:52   ` Miquel Raynal
  0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2026-08-25  9:52 UTC (permalink / raw)
  To: Mehmet Fide
  Cc: Stefan Agner, Richard Weinberger, Vignesh Raghavendra,
	Mehmet Fide, Boris Brezillon, Frieder Schrempf, Edward Karpicz,
	linux-mtd, linux-kernel, stable

Hi Mehmet,

On 18/08/2026 at 13:42:07 +02, Mehmet Fide <mehmet.fide@gmail.com> wrote:

> From: Mehmet Fide <mehmet.fide@screeningeagle.com>
>
> The driver only implements the 64-byte OOB layout, so attach_chip()
> shrinks mtd->oobsize when the chip provides more. 

Ehr...

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

I guess that's expected.

> The driver therefore transfers writesize + the chip's full OOB size, so
> 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.

This is clearly the correct™ way :)

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

This commit is not incorrect, what the driver does is incorrect, and
is now unfortunately visible.

> Clamp the memory organization as well so the driver's 64-byte layout
> stays in effect, and log the truncation once, as it decides which
> on-flash layout the system uses.

It fixes the problem, but honestly modifying the memorg is not
correct. Broadcom and mpc5121 controller drivers do it, but that's
likely wrong.

What you report here is a U-Boot issue. The chips has more than 64 OOB
bytes, U-boot clamps that value, Linux does not. U-boot is wrong. But I
guess it's now too late and you'll tell me many devices in the field
already use this broken layout?

In this case, maybe you should just change the layout, instead of
forcing an obviously incorrect memory layout. In this driver the large
page generic OOB layout (which puts the ECC bytes at the end) is used. I
guess a better approach could be to make your own 64-byte clamped OOB
layout for backward compatibility. You should drop the oobsize
modification as well.

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

There is at least one legitimate Sashiko warning on this patch, can you
please also check it?

Thanks!
Miquèl

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-25  9:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 11:42 [PATCH 0/2] mtd: rawnand: vf610_nfc: two fixes for chips with large OOB and for erased pages Mehmet Fide
2026-08-18 11:42 ` [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB Mehmet Fide
2026-08-25  9:52   ` Miquel Raynal
2026-08-18 11:42 ` [PATCH 2/2] mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages Mehmet Fide

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox