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

[Brian Norris]

On Wed, Jul 31, 2024 at 2:05 AM Michael Walle <mwalle@xxxxxxxxxx> wrote:
> 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.

I don't actually think that would be a very good test. It would be
testing the implementation more than the functionality. What do you
"verify" in the status register? Would the test just re-implement the
swp.c protection-range logic? And notably, this omits *all* checks
that the protection register actually protects from anything (write,
erase).

Or maybe I'm misinterpreting what you mean.

> 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.

Why would that lead to errors? It should be relatively easy to:

1. enumerate the supported protection ranges (MEMLOCK / MEMUNLOCK
ioctls on known-likely ranges, looking for EINVAL return codes)
2. iterate through all such ranges; for a given range:
2(a). erase the whole flash
2(b). write the whole flash with a known pattern
2(c). read the whole flash
2(d). ensure that the expected-protected range remains 0xff
2(e). ensure that the expected-unprotected range contains the known pattern

I suppose step #1 can be tough, because the full slate of possible
protection ranges is technically ... enormous. But "likely" ranges are
much fewer, with a few power-of-2 patterns, top/bottom, and maybe some
"both top and bottom" ranges on some flashes? Anyway, like I said in
my other reply, this should take on the order of 60 minutes on some
flashes, which is expensive but not prohibitive.

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

Yes, and that's all rather ugly. But it also has a linux_mtd backend
since a few years back:

https://review.coreboot.org/plugins/gitiles/flashrom/+/HEAD/linux_mtd.c

Brian