Re: [PATCH 2/2] mtd: rawnand: Bypass a couple of sanity checks during NAND identification

From: Steven Seeger <steven.seeger@flightsystems.net>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <tudor.ambarus@linaro.org>,
	Pratyush Yadav <pratyush@kernel.org>,
	Michael Walle <michael@walle.cc>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	Alexander Dahl <ada@thorsis.com>
Subject: Re: [PATCH 2/2] mtd: rawnand: Bypass a couple of sanity checks during NAND identification
Date: Tue, 14 May 2024 17:57:46 +0000	[thread overview]
Message-ID: <DM6PR05MB450689909396CB80A56B8732F7E32@DM6PR05MB4506.namprd05.prod.outlook.com> (raw)
In-Reply-To: <20240507160546.130255-3-miquel.raynal@bootlin.com>

On Tuesday, May 7, 2024, Miquel Raynal wrote:

>So, if the fields are empty, especially mtd->writesize which is *always*
>set quite rapidly after identification, let's skip the sanity checks.

I noticed this when first looking at my board with the bitflip in a NAND chip's parameter page. I just assumed that since I was setting it up to do column change operations, I needed to add this in at the time. Looking at it now, since this information is being supplied by me before the scan, it's wrong.  So I agree it's a bug, but I didn't think about it again since I was tackling the bug of trying to read additional parameter page copies further down the page due to the bitflip.

I don't have access to the board right now, but when I get to it again I can try this patch. I will need to remove what I already added in to check and reply back. It may be a few weeks, though.

On another note, I think that this entire API would benefit from discouraging hybrid approaches. I implement function overloads for things like ecc.read_page, write_page, read_page_raw, etc, but also use the exec function for things like erase, read id, read parameter page, etc. I maybe did it "wrong" but it works. Past drivers I've done use the legacy cmdfunc, so this was my first attempt at using the command parser. I suspect there are a lot of people like me writing drivers for proprietary hardware that uses FPGAs to do some of the NAND interaction, rather than direct chip access as the API was originally designed for.

So, to explain further, read_page triggers my addr/chip select, read page command, and retrieving the buffer. Read parameter page goes through the command parser, as does the column change op, with some state variables to keep track of where in the read cycle we are so that each copy of the parameter page data can be accessed in the buffer. I lament the lack of consistency here. But, it works and the customer is unlikely to want to change anything at this point. :)

Steven

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