Message-ID: <3f415ff1-834e-4544-a093-dcb4fd25d5c9@linux.dev>
Date: Thu, 13 Nov 2025 10:32:05 -0500
From: Sean Anderson <sean.anderson@...ux.dev>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Tudor Ambarus <tudor.ambarus@...aro.org>,
 Pratyush Yadav <pratyush@...nel.org>, Michael Walle <mwalle@...nel.org>,
 linux-mtd@...ts.infradead.org, Richard Weinberger <richard@....at>,
 linux-kernel@...r.kernel.org, Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH] mtd: spi-nor: Enable locking for n25q00a

On 11/12/25 08:10, Miquel Raynal wrote:
> Hello Sean,
> 
>> # flash_lock -u /dev/mtd/by-name/spi0.1
>> # flash_lock -i /dev/mtd/by-name/spi0.1
>> Device: /dev/mtd/by-name/spi0.1
>> Start: 0
>> Len: 0x8000000
>> Lock status: unlocked
>> Return code: 0
>> # test() {
>>> mtd=/dev/mtd/by-name/$1
>>> start=$(($2 * 64 * 1024))
>>> size=$(($3 * 64 * 1024))
>>> dd if=/dev/urandom of=$1 bs=64k count=$3 status=none && \
>>> mtd_debug erase $mtd $start $size && \
>>> mtd_debug write $mtd $start $size $1 && \
>>> dd if=$mtd bs=64k skip=$2 count=$3 status=none | sha256sum $1 - && \
>>> rm $1
>>> }
> 
> 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).

> When you run "# flash_lock -i /dev/mtd/by-name/spi0.1", you privide no
> start/length values to the command. Hence, the defaults are picked: the
> entire device is considered for the check. The tool asks the kernel
> whether the range 0-0x7ffffff is *fully* locked. Answer is no, it is not
> fully locked.
> 
> In the kernel there are two helpers for that, and they won't give you
> opposite results all the time:
> - is locked:
>     - returns true if the given range is fully locked
>     - returns false otherwise
> - is unlocked:
>     - returns yes if the given range is fully unlocked
>     - returns false otherwise
> 
> So if you want the tool to tell you "yes", you should instead use the
> exact range you locked (1024-2047) or any subset of it.
> 
> Thanks, Miquèl

Powered by blists - more mailing lists