On 3/5/24 5:55 PM, Michael Walle wrote:
[...]
Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
Linux, since Linux that is booted afterward then gets a device that has
locking scheme configured in a way that Linux expects and can operate.
Better yet, if some old LTS version of the Linux kernel is in use, it
will also work correctly, because this patch will configure the SPI NOR
locking scheme to what Linux expects it to be, before the SPI NOR is
handed over to that old kernel.
Agreed, but it should *not* be done automatically and nor
unconditionally. Please don't just overwrite any locking bits just
because there is one flash which have it set.
The unlock code is not triggered unconditionally, it is protected by
if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
Kconfig option, so this can be turned on/off in board config already.
Ahh, OK then :)
But keep in mind that enabling this option is foobar and I've gone
lengths to eliminate it in linux. And actually made that option in
u-boot.
See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
are non-volatile").
So, how shall we proceed here?
Regarding this patch, I think it's fine. It will unlock the whole
flash as advertised.
This change won't unlock the flash, it will switch to the supported
locking scheme, one which can then be used to unlock the flash.
But u-boot should really consider making that a "default n" option.
And most likely adding =y to every existing defconfig out there so
that at least new boards will benefit from it.
Yes, I agree with that.
The way I see this, if Linux ever implements this scheme, Linux can set
the SR3 WPS bit as needed, it does not matter which way bootloader sets
the bit as the protection bits are not cleared when the bit is cleared,
they seem to be stored elsewhere.
On each reboot you'd wear out that cell with two erases/writes.
That's another reason why that whole unlocking thing is foobar for
non-volatile bits. For me, non-volatile bits are for provisioning,
so during a normal boot they should not be touched at all. Just
during board manufacturing or because the user actively want to
protect something.
That is what happens here, the write to clear the bit is triggered only
if the bit is set , so OK .
And if Linux wants to use the new locking scheme, then the bootloader
should ideally be updated to match, i.e. SPI_FLASH_UNLOCK_ALL=n etc, so
that is also OK .
In other words, there should be no writes into the non-volatile bits,
unless U-Boot and Linux disagree on the locking scheme, in which case
writes are unavoidable (if you want unlock to work in both projects).
If you clear this bit during the unlock all command, all the locking
bits are cleared anyway. Or do you mean, the individual bits are
still retained?
The lock bits themselves are retained, this SR3 WPS only selects which
of those bits take effect, whether the SR ones (standard locking scheme)
or the per-page ones (winbond custom locking scheme).
But, if Linux starts to use SR3 WPS, then U-Boot should be updated to
match, i.e. both projects should agree on the locking scheme, so that
there won't be a situation where on the same device, one project uses
one scheme, the other project uses another scheme. I think this would
technically work, but it would be horrible from the user perspective.
And if that happens, both projects should then be updated in lockstep
and this UNLOCK_ALL should be disabled for such a setup then.
If you don't touch it, I don't think you have a problem here. u-boot
and linux can support different schemes, as long as the user is
aware of that. I.e. if they want to lock a region and the flash is
configured in a mode which isn't supported in u-boot (or linux) it
should be rejected. There might of course be a command to switch the
scheme if someone want to do so.
That is why I wrote that it is technically possible, but probably not a
good idea because it is inconsistent and therefore error prone.
