Received-SPF: pass (google.com: domain of linux-kernel+bounces-149815-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 18 Apr 2024 11:32:44 +0200
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>, Vignesh Raghavendra
 <vigneshr@ti.com>, linux-mtd@lists.infradead.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] mtd: nand: mxc_nand: disable subpage reads
Message-ID: <20240418113244.6e535d3f@xps-13>
In-Reply-To: <ZiDCKGlG4MZ23Tqo@pengutronix.de>
References: <20240417-mtd-nand-mxc-nand-exec-op-v1-0-d12564fe54e9@pengutronix.de>
	<20240417-mtd-nand-mxc-nand-exec-op-v1-4-d12564fe54e9@pengutronix.de>
	<ZiDCKGlG4MZ23Tqo@pengutronix.de>
Organization: Bootlin
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi Sascha,

s.hauer@pengutronix.de wrote on Thu, 18 Apr 2024 08:48:08 +0200:

> On Wed, Apr 17, 2024 at 09:13:31AM +0200, Sascha Hauer wrote:
> > The NAND core enabled subpage reads when a largepage NAND is used with
> > SOFT_ECC. The i.MX NAND controller doesn't support subpage reads, so
> > clear the flag again.
> >=20
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mtd/nand/raw/mxc_nand.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >=20
> > diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc=
_nand.c
> > index f44c130dca18d..19b46210bd194 100644
> > --- a/drivers/mtd/nand/raw/mxc_nand.c
> > +++ b/drivers/mtd/nand/raw/mxc_nand.c
> > @@ -1667,6 +1667,8 @@ static int mxcnd_probe(struct platform_device *pd=
ev)
> >  	if (err)
> >  		goto escan;
> > =20
> > +	this->options &=3D ~NAND_SUBPAGE_READ;
> > + =20
>=20
> Nah, it doesn't work like this. It turns out the BBT is read using
> subpage reads before we can disable them here.
>
> This is the code in nand_scan_tail() we stumble upon:
>=20
> 	/* Large page NAND with SOFT_ECC should support subpage reads */
> 	switch (ecc->engine_type) {
> 	case NAND_ECC_ENGINE_TYPE_SOFT:
> 		if (chip->page_shift > 9)
> 			chip->options |=3D NAND_SUBPAGE_READ;
> 		break;
>=20
> 	default:
> 		break;
> 	}
>=20
> So the code assumes subpage reads are ok when SOFT_ECC is in use, which
> in my case is not true. I guess some drivers depend on the
> NAND_SUBPAGE_READ bit magically be set, so simply removing this code is
> likely not an option.  Any ideas what to do?

Can you elaborate why subpage reads are not an option in your
situation? While subpage writes depend on chip capabilities, reads
however should always work: it's just the controller selecting the
column where to start and then reading less data than it could from the
NAND cache. It's a very basic NAND controller feature, and I remember
this was working on eg. an i.MX27.

Thanks,
Miqu=C3=A8l