[PATCH 1/2] mtd: spi-nor-core: allow overriding 4-byte OPCODE support

Currently, the only way to indicate 4-byte OPCODE support is by setting the SPI_NOR_4B_OPCODES feature flag for each JEDEC ID in spi_nor_ids[].
However, its becoming increasingly common practice for vendors to reuse the same JEDEC ID for new revisions of current parts. For example Winbond W25Q256FV does not fully support 4-byte OPCODE-s while newer W25Q256JV revision does fully support them but they share the same JEDEC ID thus currently its not possible to advertise support for 4-byte OPCODE-s on W25Q256JV.
Luckily for us, there usually is a way to differentiate between parts with the same JEDEC ID by differences in SFDP tables, so in order to be able to apply a fixup after they are parsed lets add a feature flag that we can override.
Signed-off-by: Robert Marko robert.marko@sartura.hr --- drivers/mtd/spi/spi-nor-core.c | 6 ++++-- include/linux/mtd/spi-nor.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index f86003ca8c..7615ba602f 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3879,7 +3879,7 @@ static int spi_nor_init(struct spi_nor *nor) if (nor->addr_width == 4 && !(nor->info->flags & SPI_NOR_OCTAL_DTR_READ) && (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) && - !(nor->info->flags & SPI_NOR_4B_OPCODES)) { + !(nor->flags & SNOR_F_4B_OPCODES)) { /* * If the RESET# pin isn't hooked up properly, or the system * otherwise doesn't perform a reset command in the boot @@ -4118,6 +4118,8 @@ int spi_nor_scan(struct spi_nor *nor) nor->flags |= SNOR_F_NO_OP_CHIP_ERASE; if (info->flags & USE_CLSR) nor->flags |= SNOR_F_USE_CLSR; + if (info->flags & SPI_NOR_4B_OPCODES) + nor->flags |= SNOR_F_4B_OPCODES;
if (info->flags & SPI_NOR_NO_ERASE) mtd->flags |= MTD_NO_ERASE; @@ -4156,7 +4158,7 @@ int spi_nor_scan(struct spi_nor *nor) /* enable 4-byte addressing if the device exceeds 16MiB */ nor->addr_width = 4; if (JEDEC_MFR(info) == SNOR_MFR_SPANSION || - info->flags & SPI_NOR_4B_OPCODES) + nor->flags & SNOR_F_4B_OPCODES) spi_nor_set_4byte_opcodes(nor, info); #else /* Configure the BAR - discover bank cmds and read current bank */ diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index d1dbf3eadb..80e56cf308 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -294,6 +294,7 @@ enum spi_nor_option_flags { SNOR_F_BROKEN_RESET = BIT(6), SNOR_F_SOFT_RESET = BIT(7), SNOR_F_IO_MODE_EN_VOLATILE = BIT(8), + SNOR_F_4B_OPCODES = BIT(9), };
struct spi_nor;

Winbond W25Q256FV and W25Q256JV share the same JEDEC ID, but only W25Q256JV fully supports 4-byte OPCODE-s.
In order to differentiate between them we can use the SFDP header version and apply a fixup post BFPT.
Based on upstream Linux commit ("mtd: spi-nor: winbond: Fix 4-byte opcode support for w25q256").
Signed-off-by: Robert Marko robert.marko@sartura.hr --- drivers/mtd/spi/spi-nor-core.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 7615ba602f..8882b045ce 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3816,6 +3816,32 @@ static struct spi_nor_fixups macronix_octal_fixups = { }; #endif /* CONFIG_SPI_FLASH_MACRONIX */
+#if CONFIG_IS_ENABLED(SPI_FLASH_WINBOND) +static int w25q256_post_bfpt_fixup(struct spi_nor *nor, + const struct sfdp_parameter_header *header, + const struct sfdp_bfpt *bfpt, + struct spi_nor_flash_parameter *params) +{ + /* + * W25Q256JV supports 4B opcodes but W25Q256FV does not. + * Unfortunately, Winbond has re-used the same JEDEC ID for both + * variants which prevents us from defining a new entry in the parts + * table. + * To differentiate between W25Q256JV and W25Q256FV check SFDP header + * version: only JV has JESD216A compliant structure (version 5). + */ + if(header->major == SFDP_JESD216_MAJOR && + header->minor == SFDP_JESD216A_MINOR) + nor->flags |= SNOR_F_4B_OPCODES; + + return 0; +} + +static struct spi_nor_fixups w25q256_fixups = { + .post_bfpt = w25q256_post_bfpt_fixup, +}; +#endif /* CONFIG_SPI_FLASH_WINBOND */ + /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed * @nor: pointer to a 'struct spi_nor' * @@ -4004,6 +4030,11 @@ void spi_nor_set_fixups(struct spi_nor *nor) #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX) nor->fixups = ¯onix_octal_fixups; #endif /* SPI_FLASH_MACRONIX */ + +#if CONFIG_IS_ENABLED(SPI_FLASH_WINBOND) + if (!strcmp(nor->info->name, "w25q256")) + nor->fixups = &w25q256_fixups; +#endif /* SPI_FLASH_WINBOND */ }
int spi_nor_scan(struct spi_nor *nor)

On 25.04.2024 17:36, Robert Marko wrote:
Winbond W25Q256FV and W25Q256JV share the same JEDEC ID, but only W25Q256JV fully supports 4-byte OPCODE-s.
In order to differentiate between them we can use the SFDP header version and apply a fixup post BFPT.
Based on upstream Linux commit ("mtd: spi-nor: winbond: Fix 4-byte opcode support for w25q256").
Linux commit info shall be the first info in the commit message. And please give the sha1 of the commit, it spares me of grepping the log.
How about something along the lines:
Backport linux upstream commit e8aec15dd5842b5b11b0e621a2293348d3574a61 [1].
The copy the upstream commit message here.
Introduce a new paragraph if you need to add your comments.
And then add a link to the commit in torvalds's git:
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... [1] ("mtd: spi-nor: winbond: Fix 4-byte opcode support for w25q256")
Looking good otherwise.
Signed-off-by: Robert Marko robert.marko@sartura.hr
drivers/mtd/spi/spi-nor-core.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 7615ba602f..8882b045ce 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3816,6 +3816,32 @@ static struct spi_nor_fixups macronix_octal_fixups = { }; #endif /* CONFIG_SPI_FLASH_MACRONIX */
+#if CONFIG_IS_ENABLED(SPI_FLASH_WINBOND) +static int w25q256_post_bfpt_fixup(struct spi_nor *nor,
const struct sfdp_parameter_header *header,
const struct sfdp_bfpt *bfpt,
struct spi_nor_flash_parameter *params)
+{
- /*
* W25Q256JV supports 4B opcodes but W25Q256FV does not.
* Unfortunately, Winbond has re-used the same JEDEC ID for both
* variants which prevents us from defining a new entry in the parts
* table.
* To differentiate between W25Q256JV and W25Q256FV check SFDP header
* version: only JV has JESD216A compliant structure (version 5).
*/
- if(header->major == SFDP_JESD216_MAJOR &&
header->minor == SFDP_JESD216A_MINOR)
nor->flags |= SNOR_F_4B_OPCODES;
- return 0;
+}
+static struct spi_nor_fixups w25q256_fixups = {
- .post_bfpt = w25q256_post_bfpt_fixup,
+}; +#endif /* CONFIG_SPI_FLASH_WINBOND */
/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
- @nor: pointer to a 'struct spi_nor'
@@ -4004,6 +4030,11 @@ void spi_nor_set_fixups(struct spi_nor *nor) #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX) nor->fixups = ¯onix_octal_fixups; #endif /* SPI_FLASH_MACRONIX */
+#if CONFIG_IS_ENABLED(SPI_FLASH_WINBOND)
- if (!strcmp(nor->info->name, "w25q256"))
nor->fixups = &w25q256_fixups;
+#endif /* SPI_FLASH_WINBOND */ }
int spi_nor_scan(struct spi_nor *nor)

Hiya,
Please specify which linux commit this patch follows. It helps reviewers and gives credit to the linux author.
Thanks, ta

On Tue, Sep 10, 2024 at 9:55 AM Tudor Ambarus tudor.ambarus@gmail.com wrote:
Hiya,
Please specify which linux commit this patch follows. It helps reviewers and gives credit to the linux author.
Hi, There isn't a specific Linux commit for this as far as I can tell.
Regards, Robert
Thanks, ta

On 9/12/24 11:07 AM, Robert Marko wrote:
On Tue, Sep 10, 2024 at 9:55 AM Tudor Ambarus tudor.ambarus@gmail.com wrote:
Hiya,
Please specify which linux commit this patch follows. It helps reviewers and gives credit to the linux author.
Hi, There isn't a specific Linux commit for this as far as I can tell.
How about the following: 548ed6847f53 ("mtd: spi-nor: Add the SNOR_F_4B_OPCODES flag")
participants (3)
-
Robert Marko
-
Tudor Ambarus
-
Tudor Ambarus