| Message ID | 20250110-winbond-6-12-rc1-nor-volatile-bit-v3-1-735363f8cc7d@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: spi-nor: winbond: Add support for flashes with several dies | expand |
On Fri, Jan 10 2025, Miquel Raynal wrote: > Add support for Winbond w25q01jv spi-nor chip. > > This chip is internally made of two dies with linear addressing > capabilities to make it transparent to the user that two dies were > used. There is one drawback however, the read status operation is racy > as the status bit only gives the active die status and not the status of > the other die. For commands affecting the two dies, it means if another > command is sent too fast after the first die has returned a valid status > (deviation can be up to 200us), the chip will get corrupted/in an > unstable state. > > This chip hence requires a better status register read. There are three > solutions here: > > 1- If we assume that the most common situation producing this problem is > status register writes, maybe we could change the "non-volatile" > status register write commands to become "volatile" status register > writes. In practice, what takes time is the write operation of the bits > themselves, and not the activation of the feature in the internal > circuitry. Enabling "volatile" status register writes would make the > writes nearly instant. > > This approach, besides probably being the less impacting one, could > overlook other possible actions where both dies can be used at the same > time like a chip erase (or any erase over die boundaries in general). > > 2- Wait about 200us after getting a first status ready feedback. This > 200us is about the maximum possible deviation between dies and would > cover all cases. > > 3- We iterate manually over all internal dies (which takes about 30us > per die) until all are ready. This approach will always be faster than > a blind delay which represents the maximum deviation, while also being > totally safe. > > This third approach has been adopted. A flash-specific hook for the > status register read had to be implemented. Testing with the flash_speed > benchmark shown no difference with the existing performances (using the > regular status read core function). In practice there are difference in > the experimental results below, but they are part of the natural > deviation of the benchmark: > > > Without the fixup > $ flash_speed /dev/mtd0 -c100 -d > eraseblock write speed is 442 KiB/s > eraseblock read speed is 1606 KiB/s > page write speed is 439 KiB/s > page read speed is 1520 KiB/s > 2 page write speed is 441 KiB/s > 2 page read speed is 1562 KiB/s > erase speed is 68 KiB/s > > > With the fixup > $ flash_speed /dev/mtd0 -c100 -d > eraseblock write speed is 428 KiB/s > eraseblock read speed is 1626 KiB/s > page write speed is 426 KiB/s > page read speed is 1538 KiB/s > 2 page write speed is 426 KiB/s > 2 page read speed is 1574 KiB/s > erase speed is 66 KiB/s > > However, the fixup, whatever which one we pick, must be applied on > multi-die chips, which hence must be properly flagged. The SFDP tables > implemented give a lot of information but the die details are part of an > optional table that is not implemented, hence we use a post parsing > fixup hook to set the params->n_dice value manually. > > Link: https://www.winbond.com/resource-files/W25Q01JV%20SPI%20RevE%2003042024%20Plus.pdf > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c index 8d0a00d69e1233988876a15479d73c5fe899c542..a4c0d99dde4fcd2b123eb7ded4f7822110c6a4ae 100644 --- a/drivers/mtd/spi-nor/winbond.c +++ b/drivers/mtd/spi-nor/winbond.c @@ -10,6 +10,7 @@ #define WINBOND_NOR_OP_RDEAR 0xc8 /* Read Extended Address Register */ #define WINBOND_NOR_OP_WREAR 0xc5 /* Write Extended Address Register */ +#define WINBOND_NOR_OP_SELDIE 0xc2 /* Select active die */ #define WINBOND_NOR_WREAR_OP(buf) \ SPI_MEM_OP(SPI_MEM_OP_CMD(WINBOND_NOR_OP_WREAR, 0), \ @@ -17,6 +18,12 @@ SPI_MEM_OP_NO_DUMMY, \ SPI_MEM_OP_DATA_OUT(1, buf, 0)) +#define WINBOND_NOR_SELDIE_OP(buf) \ + SPI_MEM_OP(SPI_MEM_OP_CMD(WINBOND_NOR_OP_SELDIE, 0), \ + SPI_MEM_OP_NO_ADDR, \ + SPI_MEM_OP_NO_DUMMY, \ + SPI_MEM_OP_DATA_OUT(1, buf, 0)) + static int w25q128_post_bfpt_fixups(struct spi_nor *nor, const struct sfdp_parameter_header *bfpt_header, @@ -66,6 +73,79 @@ static const struct spi_nor_fixups w25q256_fixups = { .post_bfpt = w25q256_post_bfpt_fixups, }; +/** + * winbond_nor_select_die() - Set active die. + * @nor: pointer to 'struct spi_nor'. + * @die: die to set active. + * + * Certain Winbond chips feature more than a single die. This is mostly hidden + * to the user, except that some chips may experience time deviation when + * modifying the status bits between dies, which in some corner cases may + * produce problematic races. Being able to explicitly select a die to check its + * state in this case may be useful. + * + * Return: 0 on success, -errno otherwise. + */ +static int winbond_nor_select_die(struct spi_nor *nor, u8 die) +{ + int ret; + + nor->bouncebuf[0] = die; + + if (nor->spimem) { + struct spi_mem_op op = WINBOND_NOR_SELDIE_OP(nor->bouncebuf); + + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto); + + ret = spi_mem_exec_op(nor->spimem, &op); + } else { + ret = spi_nor_controller_ops_write_reg(nor, + WINBOND_NOR_OP_SELDIE, + nor->bouncebuf, 1); + } + + if (ret) + dev_dbg(nor->dev, "error %d selecting die %d\n", ret, die); + + return ret; +} + +static int winbond_nor_multi_die_ready(struct spi_nor *nor) +{ + int ret, i; + + for (i = 0; i < nor->params->n_dice; i++) { + ret = winbond_nor_select_die(nor, i); + if (ret) + return ret; + + ret = spi_nor_sr_ready(nor); + if (ret <= 0) + return ret; + } + + return 1; +} + +static int +winbond_nor_multi_die_post_sfdp_fixups(struct spi_nor *nor) +{ + /* + * SFDP supports dice numbers, but this information is only available in + * optional additional tables which are not provided by these chips. + * Dice number has an impact though, because these devices need extra + * care when reading the busy bit. + */ + nor->params->n_dice = nor->params->size / SZ_64M; + nor->params->ready = winbond_nor_multi_die_ready; + + return 0; +} + +static const struct spi_nor_fixups winbond_nor_multi_die_fixups = { + .post_sfdp = winbond_nor_multi_die_post_sfdp_fixups, +}; + static const struct flash_info winbond_nor_parts[] = { { .id = SNOR_ID(0xef, 0x30, 0x10), @@ -146,6 +226,10 @@ static const struct flash_info winbond_nor_parts[] = { .name = "w25q512jvq", .size = SZ_64M, .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, + }, { + /* W25Q01JV */ + .id = SNOR_ID(0xef, 0x40, 0x21), + .fixups = &winbond_nor_multi_die_fixups, }, { .id = SNOR_ID(0xef, 0x50, 0x12), .name = "w25q20bw",