2023-07-14 18:47:11

by Linus Walleij

[permalink] [raw]
Subject: [PATCH v3] mtd: spi-nor: Correct flags for Winbond w25q128

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.

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

Since we suspect that older chips may be using the same
device ID, we need to keep NO_SFDP_FLAGS(SECT_4K) as these
older chips may not support SFDP.

cat jedec_id
ef4018
cat manufacturer
winbond
cat partname
w25q128
hexdump -v -C sfdp
00000000 53 46 44 50 05 01 00 ff 00 05 01 10 80 00 00 ff
00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000020 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000030 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000040 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000050 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000060 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000070 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00000080 e5 20 f9 ff ff ff ff 07 44 eb 08 6b 08 3b 42 bb
00000090 fe ff ff ff ff ff 00 00 ff ff 40 eb 0c 20 0f 52
000000a0 10 d8 00 00 36 02 a6 00 82 ea 14 c9 e9 63 76 33
000000b0 7a 75 7a 75 f7 a2 d5 5c 19 f7 4d ff e9 30 f8 80

Cc: [email protected]
Suggested-by: Michael Walle <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
---
Changes in v3:
- Keep NO_SFDP_FLAGS(SECT_4K) around.
- Update commit message
- Link to v2: https://lore.kernel.org/r/[email protected]

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/[email protected]
---
drivers/mtd/spi-nor/winbond.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 834d6ba5ce70..8f30a67cd27a 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -121,6 +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)
+ PARSE_SFDP
+ FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
NO_SFDP_FLAGS(SECT_4K) },
{ "w25q256", INFO(0xef4019, 0, 64 * 1024, 512)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)

---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230711-spi-nor-winbond-w25q128-321a602ee267

Best regards,
--
Linus Walleij <[email protected]>



2023-07-14 20:01:25

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: spi-nor: Correct flags for Winbond w25q128

Hi,

Am 2023-07-14 20:29, schrieb Linus Walleij:
> 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

This is still wrong btw. According to:
https://www.winbond.com/resource-files/W25Q128JV%20RevI%2008232021%20Plus.pdf

This should be
0xef4018: "w25q128" W25Q128JV-IN/IQ/JQ
0xef7018: "w25q128jv" W25Q128JV-IM/JM

Otherwise, looks good. Either Tudor can change that in
place while picking up the patch or you could send a new
version.

Either way:
Reviewed-by: Michael Walle <[email protected]>

Thanks!
-michael

> 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.
>
> 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 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.
>
> Since we suspect that older chips may be using the same
> device ID, we need to keep NO_SFDP_FLAGS(SECT_4K) as these
> older chips may not support SFDP.
>
> cat jedec_id
> ef4018
> cat manufacturer
> winbond
> cat partname
> w25q128
> hexdump -v -C sfdp
> 00000000 53 46 44 50 05 01 00 ff 00 05 01 10 80 00 00 ff
> 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000020 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000030 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000040 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000050 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000060 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000070 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 00000080 e5 20 f9 ff ff ff ff 07 44 eb 08 6b 08 3b 42 bb
> 00000090 fe ff ff ff ff ff 00 00 ff ff 40 eb 0c 20 0f 52
> 000000a0 10 d8 00 00 36 02 a6 00 82 ea 14 c9 e9 63 76 33
> 000000b0 7a 75 7a 75 f7 a2 d5 5c 19 f7 4d ff e9 30 f8 80
>
> Cc: [email protected]
> Suggested-by: Michael Walle <[email protected]>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> Changes in v3:
> - Keep NO_SFDP_FLAGS(SECT_4K) around.
> - Update commit message
> - Link to v2:
> https://lore.kernel.org/r/[email protected]
>
> 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/[email protected]
> ---
> drivers/mtd/spi-nor/winbond.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/winbond.c
> b/drivers/mtd/spi-nor/winbond.c
> index 834d6ba5ce70..8f30a67cd27a 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -121,6 +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)
> + PARSE_SFDP
> + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> NO_SFDP_FLAGS(SECT_4K) },
> { "w25q256", INFO(0xef4019, 0, 64 * 1024, 512)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230711-spi-nor-winbond-w25q128-321a602ee267
>
> Best regards,

2023-07-14 20:45:22

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: spi-nor: Correct flags for Winbond w25q128

On Fri, Jul 14, 2023 at 9:30 PM Michael Walle <[email protected]> wrote:
> Am 2023-07-14 20:29, schrieb Linus Walleij:

> > 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
>
> This is still wrong btw. According to:
> https://www.winbond.com/resource-files/W25Q128JV%20RevI%2008232021%20Plus.pdf
>
> This should be
> 0xef4018: "w25q128" W25Q128JV-IN/IQ/JQ
> 0xef7018: "w25q128jv" W25Q128JV-IM/JM
>
> Otherwise, looks good. Either Tudor can change that in
> place while picking up the patch or you could send a new
> version.

Ooops you're right, I'll just pick up your tag and re-send,
sorry!

Yours,
Linus Walleij