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
signature.asc
Description: PGP signature
