Hi Pratyush,
On 2/2/2021 4:40 AM, Pratyush Yadav wrote:
On 28/01/21 01:37PM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
Fixes mode clocks for SPINOR_OP_READ_FAST_4B and volatile QE bit in tiny. The volatile QE bit function, spansion_quad_enable_volatile() supports dual/quad die package parts, by taking 'die_size' parameter that is used to iterate register update for all dies in the device.
I'm not so sure if this is a good idea. spi-nor-tiny should be the minimal set of functionality to get the bootloader to the next stage. 1S-1S-1S mode is sufficient for that. Adding quad enable functions of all the flashes will increase the size quite a bit. I know that some flashes already have their quad enable hooks, and I don't think they should be there either.
Of course, the maintainers have the final call, but from my side,
Nacked-by: Pratyush Yadav p.yadav@ti.com
I understand your point. Let's leave the decision up to the maintainers.
Anyway, comments below in case the maintainers do plan on picking this patch up.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
drivers/mtd/spi/spi-nor-tiny.c | 89 ++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c index 5cc2b7d996..66680df5a9 100644 --- a/drivers/mtd/spi/spi-nor-tiny.c +++ b/drivers/mtd/spi/spi-nor-tiny.c @@ -555,6 +555,85 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor) } #endif /* CONFIG_SPI_FLASH_SPANSION */
+#ifdef CONFIG_SPI_FLASH_SPANSION +/**
- spansion_quad_enable_volatile() - enable Quad I/O mode in volatile register.
- @nor: pointer to a 'struct spi_nor'
- @die_size: maximum number of bytes per die ('mtd.size' > 'die_size' in
multi die package parts).
- @dummy: number of dummy cycles for register read
- It is recommended to update volatile registers in the field application due
- to a risk of the non-volatile registers corruption by power interrupt. This
- function sets Quad Enable bit in CFR1 volatile.
- Return: 0 on success, -errno otherwise.
- */
+static int spansion_quad_enable_volatile(struct spi_nor *nor, u32 die_size,
u8 dummy)+{
- struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(0, 1),SPI_MEM_OP_ADDR(4, 0, 1),SPI_MEM_OP_DUMMY(0, 1),SPI_MEM_OP_DATA_IN(1, NULL, 1));- u32 addr;
- u8 cr;
- int ret;
- /* Use 4-byte address for RDAR/WRAR */
- ret = spi_nor_write_reg(nor, SPINOR_OP_EN4B, NULL, 0);
- if (ret < 0) {
dev_dbg(nor->dev,"error while enabling 4-byte address\n");return ret;- }
- for (addr = 0; addr < nor->mtd.size; addr += die_size) {
op.addr.val = addr + SPINOR_REG_ADDR_CFR1V;So here you add the register offset to the base address, instead of ORing it. Ok.
/* Check current Quad Enable bit value. */op.cmd.opcode = SPINOR_OP_RDAR;op.dummy.nbytes = dummy / 8;op.data.dir = SPI_MEM_DATA_IN;ret = spi_nor_read_write_reg(nor, &op, &cr);if (ret < 0) {dev_dbg(nor->dev,"error while reading configuration register\n");return -EINVAL;}if (cr & CR_QUAD_EN_SPAN)return 0;/* Write new value. */cr |= CR_QUAD_EN_SPAN;op.cmd.opcode = SPINOR_OP_WRAR;op.dummy.nbytes = 0;op.data.dir = SPI_MEM_DATA_OUT;write_enable(nor);ret = spi_nor_read_write_reg(nor, &op, &cr);if (ret < 0) {dev_dbg(nor->dev,"error while writing configuration register\n");return -EINVAL;}/* Read back and check it. */op.data.dir = SPI_MEM_DATA_IN;ret = spi_nor_read_write_reg(nor, &op, &cr);if (ret || !(cr & CR_QUAD_EN_SPAN)) {dev_dbg(nor->dev, "Spansion Quad bit not set\n");return -EINVAL;}- }
- return 0;
+} +#endif
LGTM.
static void spi_nor_set_read_settings(struct spi_nor_read_command *read, u8 num_mode_clocks, @@ -583,6 +662,11 @@ static int spi_nor_init_params(struct spi_nor *nor, spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_FAST], 0, 8, SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1); +#ifdef CONFIG_SPI_FLASH_SPANSION
if (JEDEC_MFR(info) == SNOR_MFR_CYPRESS &&(info->id[1] == 0x2a || info->id[1] == 0x2b))Add a comment about which flash models these two are.
Will do.
params->reads[SNOR_CMD_READ_FAST].num_mode_clocks = 8;+#endif }
if (info->flags & SPI_NOR_QUAD_READ) { @@ -659,6 +743,11 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info, case SNOR_MFR_MACRONIX: err = macronix_quad_enable(nor); break; +#endif +#ifdef CONFIG_SPI_FLASH_SPANSION
case SNOR_MFR_CYPRESS:err = spansion_quad_enable_volatile(nor, SZ_128M, 0);break;#endif case SNOR_MFR_ST: case SNOR_MFR_MICRON: -- 2.25.1
Best Regards, Takahiro