Hi, Linus,
On 13.07.2023 00:59, Linus Walleij wrote:
The Winbond "w25q128" (actual vendor name W25Q128JV) has exactly the same flags as the sibling device "w25q128jv". The devices both require unlocking to enable write access.
The actual product naming between devices vs the Linux strings in winbond.c:
0xef4018: "w25q128" W25Q128JV-IM/JM 0xef7018: "w25q128jv" W25Q128JV-IN/IQ/JQ
The latter device, "w25q128jv" supports features named DTQ and QPI, otherwise it is the same.
Not having the right flags has the annoying side effect that write access does not work.
I guess you refer to the locking flags. Probably your flash has the non volatile block protection (BP) bits from the Status Register set, which means the entire flash is write protected. The factory default for these bits is 0/disabled on this flash so someone must have played with them. The reason why one may want write protection set is to avoid inadvertent writes during power-up. One can control whether to disable the software write protection at boot time with the MTD_SPI_NOR_SWP_ configs.
After this patch I can write to the flash on the Inteno XG6846 router.
The flash memory also supports dual and quad SPI modes. This does not currently manifest, but by
The fasted mode is chosen after SFDP parsing, so you should use quad reads if your controller also supports 4 I/O lines.
turning on SFDP parsing, the right SPI modes are emitted in /sys/kernel/debug/spi-nor/spi1.0/capabilities for this chip, so we also turn on this.
Cc: stable@vger.kernel.org Suggested-by: Michael Walle michael@walle.cc Signed-off-by: Linus Walleij linus.walleij@linaro.org
Changes in v2:
- Only add the write access flags.
- Use SFDP parsing to properly detect the various available SPI modes.
- Link to v1: https://lore.kernel.org/r/20230712-spi-nor-winbond-w25q128-v1-1-f78f3bb42a1c...
drivers/mtd/spi-nor/winbond.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c index 834d6ba5ce70..6c82e525c801 100644 --- a/drivers/mtd/spi-nor/winbond.c +++ b/drivers/mtd/spi-nor/winbond.c @@ -121,7 +121,8 @@ static const struct flash_info winbond_nor_parts[] = { { "w25q80bl", INFO(0xef4014, 0, 64 * 1024, 16) NO_SFDP_FLAGS(SECT_4K) }, { "w25q128", INFO(0xef4018, 0, 64 * 1024, 256)
while here try, using INFO with INFO(0xef4018, 0, 0, 0), those parameters shall be discovered at run-time, so we prepare to get rid of explicitly setting them sooner or later.
NO_SFDP_FLAGS(SECT_4K) },
PARSE_SFDPFLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
Looks good. Also I would like you to run a small sanity test, just to make sure the flash works after your changes. You can do that with mtd_debug utility, see an example on Miquel's commit message from: https://lore.kernel.org/linux-mtd/d479489736ee193609816dc2003bd0fb@walle.cc/...
Cheers, ta