On Tue Mar 5, 2024 at 4:37 PM CET, Marek Vasut wrote: > On 3/5/24 1:50 PM, Michael Walle wrote: > > Hi Marek, > > Hi, > > > 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. > > OK > > >> 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"). -michael > > 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. > > See CONFIG_SPI_FLASH_UNLOCK_ALL above.
signature.asc
Description: PGP signature