Re: [PATCH] mtd: spi-nor: micron-st: Add n25q064a WP support

From: "Michael Walle" <mwalle@kernel.org>
To: "Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Brian Norris" <computersforpeace@gmail.com>
Cc: <linux-mtd@lists.infradead.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: micron-st: Add n25q064a WP support
Date: Wed, 31 Jul 2024 11:05:28 +0200	[thread overview]
Message-ID: <D33LRTJ8BE0T.2EQJ1MGLG2NUS@kernel.org> (raw)
In-Reply-To: <0685ef1b-b0e1-4c53-94dc-4d5de5be8e94@linaro.org>

[-- Attachment #1.1: Type: text/plain, Size: 2227 bytes --]

> >> Also, if you care, would be good to extend the SPI NOR documentation on
> >> how one shall test the Block Protection.
> > 
> > That sounds tougher. I don't know that there's really a standard
> > toolset for handling protection -- I guess the 'flash_{un,}lock'
> > utilities in mtd-utils qualify, but they aren't even packaged by
> > relevant distros (e.g., OpenWrt; but I guess they're in Debian). I
> > typically use flashrom, since that's what ChromiumOS uses, and it's
> > available in OpenWrt -- would that be an OK basis for docs?
>
> yes, why not. At least for people using OpenWrt.

We really need some kind of dump the relevant registers here. I have
some very old patch, which dumps the status register, but is has
it's own quirks. But IMHO we should (maybe additional to the
functional tests) look at the locking bits in the corresponding
registers. I.e.
 flash_lock foobar
 <verify the status register>
 flash_unlock foobar
 <verify the status register>
 flash_lock barfoo
 <verify the status register>
 etc.

Just inferring the correctness from behavior (exercised by writing
to the flash and verifying it) will lead to errors as it is hard to
catch all the corner cases.

From what I remember, flashrom has it's own drivers in userspace,
no?

-michael

> > 
> > It's also highly conditional on what sort of range(s) the flash
> > supports. So it's almost like we might want a programmatic
> > write-protection test suite as part of mtd-utils/tests/, rather than a
> > bespoke narrative document. Which ... is getting a little larger than
> > I signed up for.
> > 
>
> Some test in mtd-utils would be good indeed, but narrative shall be also
> ok for now. What I fear is that people just use just a flash lock/unlock
> all sectors test, which is not ideal. We shall also test locking on some
> sectors from the top and bottom, to verify the correctness of the TB
> bit, check if BP3 is working by locking some sectors in that area.
> Haven't looked at the BP area in a while, but you get my point, I feel
> testing is not ideal and a guideline would help.
>
> If you ever feel that you can spend some time on this, help is appreciated.
>
> Thanks,
> ta

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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