[PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

[Michael Walle]

Hi Marek,

On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote:
> On 3/5/24 9:55 AM, Michael Walle wrote:
> > On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> >> Some Winbond SPI NORs have special SR3 register which is
> >> used among other things to control whether non-standard
> >> "Individual Block/Sector Write Protection" (WPS bit)
> >> locking scheme is activated. This non-standard locking
> >> scheme is not supported by either U-Boot or Linux SPI
> >> NOR stack so make sure it is disabled, otherwise the
> >> SPI NOR may appear locked for no obvious reason.
> > 
> > I don't think it is a good idea to just disable the WPS bit.
> > Usually, that bit is non-volatile and the default is not set.
>
> Yes, that's right, the bit is non-volatile and should not be set unless 
> the MTD stack can handle it correctly. Currently, neither U-Boot nor 
> Linux does handle the bit at all, instead both attempt to use the 
> standard SPI NOR locking scheme which is also implemented by this SPI 
> NOR model and they both fail to unlock the SPI NOR that way.
>
> Note that the SR3 WPS bit only switches between two different locking 
> schemes (unset = standard SPI NOR locking scheme, set = custom winbond 
> locking scheme), setting SR3 WPS does not immediately imply the SPI NOR 
> is locked, rather the opposite. Out of manufacturing, the SPI NOR is 
> unlocked in either locking scheme. Depending on the SR3 WPS bit state, 
> different commands are used to lock and unlock the SPI NOR.
>
> I recently ran across a device which had the SR3 WPS bit incorrectly set 
> out of manufacturing of that device (i.e. before it was populated on a 
> board at board manufacturer) and the device was locked using the custom 
> locking scheme. I was not able to unlock or use that device because both 
> U-Boot and Linux tried to use the standard scheme for that purpose.

Still, I don't think it makes any sense, to disable that bit for all
winbond flashes just because there was one vendor which set it the
wrong way - or the board manufacturer didn't test it and programmed
the flash correctly.

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

This should be either be a board level option or maybe expose some
command line interface to let the user change the settings.

> > Thus,
> > there is likely someone, probably the manufacturer of the board,
> > who intentionally set this bit. Now, with this patch it will get
> > disabled *unconditionally*, forever.
>
> In my case, it is exactly the opposite, the SR3 WPS shouldn't have been 
> set and the device should not have been locked, but it was and that 
> confused both U-Boot and Linux.
>
> I would argue that if the board manufacturer intention was to lock the 
> SPI NOR, they would have had multiple better options at their disposal, 
> and those options should have been aligned with the software support:
> - nWP pin of the SPI NOR could be tied low to enable WRITE PROTECT
> - OTP bits could have been programmed to enable permanent WRITE PROTECT
> - standard SPI NOR locking scheme could have been used too
>
> If they did set SR3 WPS and hoped that U-Boot or Linux would fail to 
> unlock the SPI NOR using standard locking scheme commands, that is I 
> think broken design.

There might be software/OS which could handle that correctly. Also,
if linux will ever learn to use the new locking scheme
unconditionally, u-boot will always mess it up then.

-michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240305/309a3ad5/attachment.sig>