Re: [PATCH] mtd: spi-nor: Enable locking for n25q00a

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello,

>> I am also working on locking these days, have you already started
>> writing extra SPI NOR Documentation/process based on this thread?
>
> I haven't started writing anything.
>
>> I am also trying to clarify what is expected and what the API somehow
>> does, as it was not fully clear for me at first sight.
>
> I agree, as you could probably have figured out.
>
>>> # flash_lock -u /dev/mtd/by-name/spi0.1
>>> # test spi0.1 64
>>> 83a8dc6125ec9672d18f7f18f92e16f867354dbe8e2f3b0aca52b9d0ad7d8ffe  spi0.1
>>> 83a8dc6125ec9672d18f7f18f92e16f867354dbe8e2f3b0aca52b9d0ad7d8ffe  -
>>> # flash_lock -l /dev/mtd/by-name/spi0.1 $((1024 * 64 * 1024)) 1024
>>> # flash_lock -i /dev/mtd/by-name/spi0.1 
>>> Device: /dev/mtd/by-name/spi0.1
>>> Start: 0
>>> Len: 0x8000000
>>> Lock status: unlocked <<<< Wrong!
>> 
>> This isn't wrong, even though at a first glance the output is
>> misleading. The API apparently does not gives you all the
>> locked/unlocked sectors, it is way more basic than that: it tells you
>> whether the full range you asked for is indeed locked.
>
> Yeah, I figured that out eventually.
>
> Actually, the most surprising thing to me is that the lock/unlock APIs
> are not incremental. That is, if I have a flash of 8 seconds, and
> sectors 0-3 are locked and I lock sectors 0-1, it will say "well,
> sectors 2-3 should be unlocked now, but we're not allowed to unlock
> during a lock operation" and fail to lock. I would have expected it to
> say "sectors 0-1 are already locked so we don't need to do anything".
> The only way to go from sectors 0-3 to 0-1 being locked is to issue an
> *unlock* on sectors 2-7.
>
> Conversely, if what you wanted to do was ensure sectors 2-3 were
> unlocked, you can't do the naive thing and unlock sectors 2-3, since
> that will try to lock sectors 0-1 and 4-7, the latter being disallowed
> in an unlock operation. So you actually have to unlock sectors 2-7.
>
> And knowing what to do is complicated by ISLOCKED only returning a
> boolean instead of just telling userspace what sectors are locked (which
> must be a small finite set of ranges (usually one) on all flashes I'm
> familiar with).

I've tried to address most of this, see:

Link: https://lore.kernel.org/linux-mtd/20251114-winbond-v6-18-rc1-spi-nor-swp-v1-0-487bc7129931@bootlin.com/T/#mbae8b874181eb0401b30142f423b73b6389a0c54

Kind regards,
Miquèl