Re: [PATCH v4 2/5] mtd: rawnand: meson: move OOB to non-protected ECC area

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Arseniy Krasnov <avkrasnov@sberdevices.ru>
Cc: Liang Yang <liang.yang@amlogic.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Yixun Lan <yixun.lan@amlogic.com>,
	Jianxin Pan <jianxin.pan@amlogic.com>, <oxffffaa@gmail.com>,
	<kernel@sberdevices.ru>, <linux-mtd@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/5] mtd: rawnand: meson: move OOB to non-protected ECC area
Date: Tue, 30 May 2023 09:44:20 +0200	[thread overview]
Message-ID: <20230530094420.06281ab5@xps-13> (raw)
In-Reply-To: <b9f0a38a-0d50-23f0-4509-c38362d05f12@sberdevices.ru>

Hi Arseniy,

> >>>> -static void meson_nfc_get_user_byte(struct nand_chip *nand, u8 *oob_buf)
> >>>> -{
> >>>> -	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> >>>> -	__le64 *info;
> >>>> -	int i, count;
> >>>> +	int i;
> >>>>  
> >>>> -	for (i = 0, count = 0; i < nand->ecc.steps; i++, count += 2) {
> >>>> +	for (i = 0; i < nand->ecc.steps; i++) {
> >>>>  		info = &meson_chip->info_buf[i];
> >>>> -		oob_buf[count] = *info;
> >>>> -		oob_buf[count + 1] = *info >> 8;
> >>>> +		/* Always ignore user bytes programming. */    
> >>>
> >>> Why?    
> >>
> >> I think comment message is wrong a little bit. Here "user bytes" are
> >> user bytes protected by ECC (e.g. location of these bytes differs from new
> >> OOB layout introduced by this patch). During page write this hardware
> >> always writes these bytes along with data. But, new OOB layout always ignores
> >> these 4 bytes, so set them to 0xFF always.  
> > 
> > When performing page reads/writes, you need to take the data as it's
> > been provided. You may move the data around in the buffer provided to
> > the controller, so that it get the ECC data at the right location, and
> > you need of course to reorganize the data when reading as well, so that
> > the user sees XkiB of data + YB of OOB. That's all you need to do in
> > these helpers.
> >   
> 
> I think there is some misunderstanding about these "user bytes" above: there are 4
> bytes which this NAND controller always writes to page in ECC mode - it was free OOB
> bytes covered by ECC. Controller grabs values from DMA buffer (second DMA buffer which
> doesn't contains page data) and writes it along with data and ECC codes. Idea of this
> change is to always suppress this write by setting them to 0xFF (may be there is some
> command option to not write it, but I don't have doc), because all of them (4 bytes)
> become unavailable to reader/writer.

At the NAND controller level, I would rather avoid doing things like
that.

I believe you can just update the ooblayout so that protected OOB bytes
are not exposed to the user as free bytes. Then your buffers should
already contain 0xffffff at the problematic location.

Thanks,
Miquèl

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic