Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Mehmet Fide <mehmet.fide@gmail.com>
Cc: Stefan Agner <stefan@agner.ch>,
	 Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	 Mehmet Fide <mehmet.fide@screeningeagle.com>,
	 Boris Brezillon <bbrezillon@kernel.org>,
	 Frieder Schrempf <frieder.schrempf@kontron.de>,
	Edward Karpicz <webmaster@toradex.com>,
	 linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	 stable@vger.kernel.org
Subject: Re: [PATCH 1/2] mtd: rawnand: vf610_nfc: fix reads on chips with more than 64 bytes of OOB
Date: Tue, 25 Aug 2026 11:52:27 +0200	[thread overview]
Message-ID: <87mruail2c.fsf@bootlin.com> (raw)
In-Reply-To: <20260818114208.2780311-2-mehmet.fide@gmail.com> (Mehmet Fide's message of "Tue, 18 Aug 2026 13:42:07 +0200")

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/

  reply	other threads:[~2026-08-25  9:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-08-18 11:42 ` [PATCH 2/2] mtd: rawnand: vf610_nfc: fix false bitflips on reads of erased pages Mehmet Fide

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mruail2c.fsf@bootlin.com \
    --to=miquel.raynal@bootlin.com \
    --cc=bbrezillon@kernel.org \
    --cc=frieder.schrempf@kontron.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mehmet.fide@gmail.com \
    --cc=mehmet.fide@screeningeagle.com \
    --cc=richard@nod.at \
    --cc=stable@vger.kernel.org \
    --cc=stefan@agner.ch \
    --cc=vigneshr@ti.com \
    --cc=webmaster@toradex.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox