Re: [V2] mtd: spinand: Add support for Etron EM73D044VCx

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Bert Vermeulen <bert@biot.com>
Cc: linux-mtd@lists.infradead.org,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Christophe Kerello <christophe.kerello@foss.st.com>,
	Mark Brown <broonie@kernel.org>,
	Alexander Lobakin <alobakin@pm.me>
Subject: Re: [V2] mtd: spinand: Add support for Etron EM73D044VCx
Date: Sat, 7 Aug 2021 12:25:26 +0200	[thread overview]
Message-ID: <20210807122526.3f9fec0b@xps13> (raw)
In-Reply-To: <ba6637f8-6ed0-262e-0229-1b449334cdd2@biot.com>

Hi Bert,

Bert Vermeulen <bert@biot.com> wrote on Sat, 7 Aug 2021 09:31:55 +0200:

> On 8/6/21 9:34 PM, Miquel Raynal wrote:
> 
> Hi Miquel,
> 
> Thanks for reviewing. I'll send in a v2 with fixes. However...
> 
> > Bert Vermeulen <bert@biot.com> wrote on Thu, 29 Jul 2021 17:18:23 +0200:  
> >> +static int etron_ooblayout_ecc(struct mtd_info *mtd, int section,
> >> +					struct mtd_oob_region *oobregion)
> >> +{
> >> +	if (section)
> >> +		return -ERANGE;
> >> +
> >> +	oobregion->offset = 72;
> >> +	oobregion->length = 56;  
> > 
> > This looks highly dependent on a single chip, could you make these
> > values (at least the offset) dependent on the MTD device?
> >   
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int etron_ooblayout_free(struct mtd_info *mtd, int section,
> >> +			   struct mtd_oob_region *oobregion)
> >> +{
> >> +	if (section)
> >> +		return -ERANGE;
> >> +
> >> +	oobregion->offset = 1;
> >> +	oobregion->length = 71;  
> > 
> > Same here.  
> 
> I was rather hoping for some guidance on the _ecc and _free
> offset/length numbers. There's not much info around on what's best
> practice here, and I haven't played with NAND chips enough to actually
> test the in-kernel ECC stuff.
> 
> The _free offset of 1 is for the bad block byte, which I saw in another
> NAND chip driver, but I don't remember where I got the other numbers
> from, and really need somebody to double-check that.

These values should be in the datasheet somehow.

Depending on the algorithm (ex: BCH) and the strength (ex: 4b/512B) you
will need a certain number of ECC bytes per chunk. Then you multiply
that number by the number of chunks and you get the total number of ECC
bytes. Assuming you know an offset from the start or the end of the OOB
section, you might write something like that:

	length = mtd->writesize / chunk_size * nb_ecc_bytes/chunk;
	offset = mtd->oobsize - length;

Also please reserve two bytes for OOB (even if so far we encountered
only 1B markers).

Thanks,
Miquèl

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