* [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag @ 2021-12-09 19:04 Tudor Ambarus 2021-12-09 19:04 ` [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus 2021-12-17 10:42 ` [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Pratyush Yadav 0 siblings, 2 replies; 8+ messages in thread From: Tudor Ambarus @ 2021-12-09 19:04 UTC (permalink / raw) To: p.yadav, michael Cc: vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, nicolas.ferre, zhengxunli, jaimeliao, Tudor Ambarus The Soft Reset and Rescue Sequence Support is defined in BFPT_DWORD(16) starting with JESD216A. The first version of SFDP, JESD216 (April 2011), defines just the first 9 BFPT DWORDS, thus it does not contain information about the Software Reset and Rescue Support. Since this support can not be discovered by parsing the first SFDP version, introduce a flash_info fixup_flag that will be used either by flashes that define JESD216 (April 2011) or by flashes that do not define SFDP at all. In case a flash defines BFPT_DWORD(16) but with wrong values, one should instead use a post_bfpt() hook and set SNOR_F_SOFT_RESET. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- v2: no changes drivers/mtd/spi-nor/core.c | 3 +++ drivers/mtd/spi-nor/core.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 2e21d5ac0e2d..32d80fdaa2a2 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -2699,6 +2699,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor) if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE) nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE; + + if (fixup_flags & SPI_NOR_SOFT_RESET) + nor->flags |= SNOR_F_SOFT_RESET; } /** diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index 2afb610853a9..70c6bb7f5f04 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -373,6 +373,8 @@ struct spi_nor_fixups { * memory size above 128Mib. * SPI_NOR_IO_MODE_EN_VOLATILE: flash enables the best available I/O mode * via a volatile bit. + * SPI_NOR_SOFT_RESET: flash supports software reset enable, reset + * sequence. * @mfr_flags: manufacturer private flags. Used in the manufacturer fixup * hooks to differentiate support between flashes of the same * manufacturer. @@ -416,6 +418,7 @@ struct flash_info { u8 fixup_flags; #define SPI_NOR_4B_OPCODES BIT(0) #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) +#define SPI_NOR_SOFT_RESET BIT(2) u8 mfr_flags; -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g 2021-12-09 19:04 [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Tudor Ambarus @ 2021-12-09 19:04 ` Tudor Ambarus 2021-12-17 11:38 ` Pratyush Yadav 2021-12-17 10:42 ` [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Pratyush Yadav 1 sibling, 1 reply; 8+ messages in thread From: Tudor Ambarus @ 2021-12-09 19:04 UTC (permalink / raw) To: p.yadav, michael Cc: vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, nicolas.ferre, zhengxunli, jaimeliao, Tudor Ambarus mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. There are versions of mx66lm1g45g which do not support SFDP, thus use SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral interface outputs data always in STR mode for whatever reason. Since 8d-8d-8s is not common, avoid reading the ID when enabling the octal dtr mode. Instead, read back the CR2 to check if the switch was successful. Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id c2853b # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer macronix # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname mx66lm1g45g # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory drivers/mtd/spi-nor/macronix.c | 113 +++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c index 67aaa83038b6..9d71149233e3 100644 --- a/drivers/mtd/spi-nor/macronix.c +++ b/drivers/mtd/spi-nor/macronix.c @@ -32,6 +32,112 @@ static struct spi_nor_fixups mx25l25635_fixups = { .post_bfpt = mx25l25635_post_bfpt_fixups, }; +#define SPINOR_OP_READ_CR2 0x71 +#define SPINOR_OP_WRITE_CR2 0x72 +#define SPINOR_OP_MX_DTR_RD 0xee + +#define SPINOR_REG_CR2_MODE_ADDR 0 +#define SPINOR_REG_CR2_DTR_OPI_ENABLE BIT(1) +#define SPINOR_REG_CR2_SPI 0 + +#define SPINOR_REG_CR2_DUMMY_ADDR 0x300 +#define SPINOR_REG_CR2_DUMMY_20 0 + +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) +{ + struct spi_mem_op op; + int ret; + + /* Use 20 dummy cycles for memory array reads. */ + if (enable) { + nor->bouncebuf[0] = SPINOR_REG_CR2_DUMMY_20; + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_DUMMY_ADDR, + 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1)); + + ret = spi_nor_write_enable(nor); + if (ret) + return ret; + + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) + return ret; + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + } + + /* Set/unset the octal and DTR enable bits. */ + if (enable) + nor->bouncebuf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE; + else + nor->bouncebuf[0] = SPINOR_REG_CR2_SPI; + + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1)); + if (!enable) + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); + + ret = spi_nor_write_enable(nor); + if (ret) + return ret; + + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) + return ret; + + /* Read back CR2 to make sure the switch was successful. */ + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), + SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1)); + if (enable) + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); + + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) + return ret; + + if (enable) { + if (nor->bouncebuf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) { + dev_dbg(nor->dev, "Failed to enable 8d-8d-8d mode.\n"); + return -EINVAL; + } + } else if (nor->bouncebuf[0] != SPINOR_REG_CR2_SPI) { + dev_dbg(nor->dev, "Failed to disable 8d-8d-8d mode.\n"); + return -EINVAL; + } + + return 0; +} + +static void mx66lm1g45g_late_init(struct spi_nor *nor) +{ + nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; + + /* Set the Fast Read settings. */ + nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR; + spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR], + 0, 20, SPINOR_OP_MX_DTR_RD, + SNOR_PROTO_8_8_8_DTR); + + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; + nor->params->rdsr_dummy = 4; + nor->params->rdsr_addr_nbytes = 4; +} + +static struct spi_nor_fixups mx66lm1g45g_fixups = { + .late_init = mx66lm1g45g_late_init, +}; + static const struct flash_info macronix_parts[] = { /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1) @@ -100,6 +206,13 @@ static const struct flash_info macronix_parts[] = { { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096) NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) FIXUP_FLAGS(SPI_NOR_4B_OPCODES) }, + { "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048) + NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K | + SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) + FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE | + SPI_NOR_SOFT_RESET) + .fixups = &mx66lm1g45g_fixups, + }, }; static void macronix_default_init(struct spi_nor *nor) -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g 2021-12-09 19:04 ` [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus @ 2021-12-17 11:38 ` Pratyush Yadav 2021-12-17 12:38 ` Tudor.Ambarus 0 siblings, 1 reply; 8+ messages in thread From: Pratyush Yadav @ 2021-12-17 11:38 UTC (permalink / raw) To: Tudor Ambarus Cc: michael, vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, nicolas.ferre, zhengxunli, jaimeliao On 09/12/21 09:04PM, Tudor Ambarus wrote: > mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. There are > versions of mx66lm1g45g which do not support SFDP, thus use > SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral > interface outputs data always in STR mode for whatever reason. Since Huh! I hope this is a mistake from the chip designers, because if it isn't they need a stern talking-to ;-) > 8d-8d-8s is not common, avoid reading the ID when enabling the octal dtr > mode. Instead, read back the CR2 to check if the switch was successful. > Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP. Datasheet? > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG > > # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id > c2853b > # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer > macronix > # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname > mx66lm1g45g > # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp > cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory > > drivers/mtd/spi-nor/macronix.c | 113 +++++++++++++++++++++++++++++++++ > 1 file changed, 113 insertions(+) > > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c > index 67aaa83038b6..9d71149233e3 100644 > --- a/drivers/mtd/spi-nor/macronix.c > +++ b/drivers/mtd/spi-nor/macronix.c > @@ -32,6 +32,112 @@ static struct spi_nor_fixups mx25l25635_fixups = { > .post_bfpt = mx25l25635_post_bfpt_fixups, > }; > > +#define SPINOR_OP_READ_CR2 0x71 > +#define SPINOR_OP_WRITE_CR2 0x72 > +#define SPINOR_OP_MX_DTR_RD 0xee > + > +#define SPINOR_REG_CR2_MODE_ADDR 0 > +#define SPINOR_REG_CR2_DTR_OPI_ENABLE BIT(1) > +#define SPINOR_REG_CR2_SPI 0 > + > +#define SPINOR_REG_CR2_DUMMY_ADDR 0x300 > +#define SPINOR_REG_CR2_DUMMY_20 0 > + > +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) > +{ > + struct spi_mem_op op; > + int ret; > + > + /* Use 20 dummy cycles for memory array reads. */ I just want to point out that if the default dummy cycle value can work at the maximum frequency the flash supports, you don't need to do this. I did it for S28 and MT35 because this wasn't the case but I remember some flashes having sane defaults and not needing this. > + if (enable) { > + nor->bouncebuf[0] = SPINOR_REG_CR2_DUMMY_20; > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_DUMMY_ADDR, > + 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1)); > + > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) > + return ret; > + > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + } > + > + /* Set/unset the octal and DTR enable bits. */ > + if (enable) > + nor->bouncebuf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE; > + else > + nor->bouncebuf[0] = SPINOR_REG_CR2_SPI; > + > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1)); This is not quite right. You can't have a 1-byte data phase in 8D mode since then your data phase is only half a cycle. What happens to the other half cycle would be undefined behavior for most controllers. I sent some patches addressing this some time back [0]. Unfortunately they fell off my radar. > + if (!enable) > + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); > + > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) > + return ret; > + > + /* Read back CR2 to make sure the switch was successful. */ > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), > + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), > + SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1)); Same as above. > + if (enable) > + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); > + > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) > + return ret; > + > + if (enable) { > + if (nor->bouncebuf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) { > + dev_dbg(nor->dev, "Failed to enable 8d-8d-8d mode.\n"); Nitpick: s/8d-8d-8d/8D-8D-8D/ > + return -EINVAL; > + } > + } else if (nor->bouncebuf[0] != SPINOR_REG_CR2_SPI) { > + dev_dbg(nor->dev, "Failed to disable 8d-8d-8d mode.\n"); Nitpick: s/8d-8d-8d/8D-8D-8D/ > + return -EINVAL; > + } > + > + return 0; > +} > + > +static void mx66lm1g45g_late_init(struct spi_nor *nor) > +{ > + nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; > + > + /* Set the Fast Read settings. */ > + nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR; > + spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR], > + 0, 20, SPINOR_OP_MX_DTR_RD, > + SNOR_PROTO_8_8_8_DTR); > + > + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; > + nor->params->rdsr_dummy = 4; > + nor->params->rdsr_addr_nbytes = 4; > +} > + > +static struct spi_nor_fixups mx66lm1g45g_fixups = { > + .late_init = mx66lm1g45g_late_init, > +}; > + > static const struct flash_info macronix_parts[] = { > /* Macronix */ > { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1) > @@ -100,6 +206,13 @@ static const struct flash_info macronix_parts[] = { > { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096) > NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) > FIXUP_FLAGS(SPI_NOR_4B_OPCODES) }, > + { "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048) > + NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K | > + SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) > + FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE | > + SPI_NOR_SOFT_RESET) > + .fixups = &mx66lm1g45g_fixups, > + }, > }; > > static void macronix_default_init(struct spi_nor *nor) > -- > 2.25.1 > [0] https://lore.kernel.org/all/20210531181757.19458-1-p.yadav@ti.com/ -- Regards, Pratyush Yadav Texas Instruments Inc. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g 2021-12-17 11:38 ` Pratyush Yadav @ 2021-12-17 12:38 ` Tudor.Ambarus 2021-12-20 10:16 ` Pratyush Yadav 0 siblings, 1 reply; 8+ messages in thread From: Tudor.Ambarus @ 2021-12-17 12:38 UTC (permalink / raw) To: p.yadav Cc: michael, vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, Nicolas.Ferre, zhengxunli, jaimeliao On 12/17/21 1:38 PM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 09/12/21 09:04PM, Tudor Ambarus wrote: >> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. There are >> versions of mx66lm1g45g which do not support SFDP, thus use >> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral >> interface outputs data always in STR mode for whatever reason. Since > > Huh! I hope this is a mistake from the chip designers, because if it > isn't they need a stern talking-to ;-) > >> 8d-8d-8s is not common, avoid reading the ID when enabling the octal dtr >> mode. Instead, read back the CR2 to check if the switch was successful. >> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP. > > Datasheet? MX66LM1G45G datasheet: https://www.macronix.com/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf If you meant the controller's datasheet, it's not publicly available yet. > >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> v2: SPI_NOR_SOFT_RESET as a FIXUP_FLAG >> >> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id >> c2853b >> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer >> macronix >> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname >> mx66lm1g45g >> # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp >> cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory >> >> drivers/mtd/spi-nor/macronix.c | 113 +++++++++++++++++++++++++++++++++ >> 1 file changed, 113 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c >> index 67aaa83038b6..9d71149233e3 100644 >> --- a/drivers/mtd/spi-nor/macronix.c >> +++ b/drivers/mtd/spi-nor/macronix.c >> @@ -32,6 +32,112 @@ static struct spi_nor_fixups mx25l25635_fixups = { >> .post_bfpt = mx25l25635_post_bfpt_fixups, >> }; >> >> +#define SPINOR_OP_READ_CR2 0x71 >> +#define SPINOR_OP_WRITE_CR2 0x72 >> +#define SPINOR_OP_MX_DTR_RD 0xee >> + >> +#define SPINOR_REG_CR2_MODE_ADDR 0 >> +#define SPINOR_REG_CR2_DTR_OPI_ENABLE BIT(1) >> +#define SPINOR_REG_CR2_SPI 0 >> + >> +#define SPINOR_REG_CR2_DUMMY_ADDR 0x300 >> +#define SPINOR_REG_CR2_DUMMY_20 0 >> + >> +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) >> +{ >> + struct spi_mem_op op; >> + int ret; >> + >> + /* Use 20 dummy cycles for memory array reads. */ > > I just want to point out that if the default dummy cycle value can work > at the maximum frequency the flash supports, you don't need to do this. > I did it for S28 and MT35 because this wasn't the case but I remember > some flashes having sane defaults and not needing this. This is a volatile bit field with the default value of 20 dummy cycles indeed, so it should be fine if the bootloaders do not touch it. Anyway, it's probably a good idea to reset the flash at kernel level before starting configuring it, so we can consider the default values as sane. I'll drop this config, sure. > >> + if (enable) { >> + nor->bouncebuf[0] = SPINOR_REG_CR2_DUMMY_20; >> + op = (struct spi_mem_op) >> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), >> + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_DUMMY_ADDR, >> + 1), >> + SPI_MEM_OP_NO_DUMMY, >> + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1)); >> + >> + ret = spi_nor_write_enable(nor); >> + if (ret) >> + return ret; >> + >> + ret = spi_mem_exec_op(nor->spimem, &op); >> + if (ret) >> + return ret; >> + >> + ret = spi_nor_wait_till_ready(nor); >> + if (ret) >> + return ret; >> + } >> + >> + /* Set/unset the octal and DTR enable bits. */ >> + if (enable) >> + nor->bouncebuf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE; >> + else >> + nor->bouncebuf[0] = SPINOR_REG_CR2_SPI; >> + >> + op = (struct spi_mem_op) >> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), >> + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), >> + SPI_MEM_OP_NO_DUMMY, >> + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1)); > > This is not quite right. You can't have a 1-byte data phase in 8D mode > since then your data phase is only half a cycle. What happens to the > other half cycle would be undefined behavior for most controllers. I > sent some patches addressing this some time back [0]. Unfortunately they > fell off my radar. right, will update. Please resend the series when you find time. > >> + if (!enable) >> + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); >> + >> + ret = spi_nor_write_enable(nor); >> + if (ret) >> + return ret; >> + >> + ret = spi_mem_exec_op(nor->spimem, &op); >> + if (ret) >> + return ret; >> + >> + /* Read back CR2 to make sure the switch was successful. */ >> + op = (struct spi_mem_op) >> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1), >> + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), >> + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), >> + SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1)); > > Same as above. ok > >> + if (enable) >> + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); >> + >> + ret = spi_mem_exec_op(nor->spimem, &op); >> + if (ret) >> + return ret; >> + >> + if (enable) { >> + if (nor->bouncebuf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) { >> + dev_dbg(nor->dev, "Failed to enable 8d-8d-8d mode.\n"); > > Nitpick: s/8d-8d-8d/8D-8D-8D/ sure > >> + return -EINVAL; >> + } >> + } else if (nor->bouncebuf[0] != SPINOR_REG_CR2_SPI) { >> + dev_dbg(nor->dev, "Failed to disable 8d-8d-8d mode.\n"); > > Nitpick: s/8d-8d-8d/8D-8D-8D/ ok. Thanks! ta ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g 2021-12-17 12:38 ` Tudor.Ambarus @ 2021-12-20 10:16 ` Pratyush Yadav 2021-12-20 11:05 ` Tudor.Ambarus 0 siblings, 1 reply; 8+ messages in thread From: Pratyush Yadav @ 2021-12-20 10:16 UTC (permalink / raw) To: Tudor.Ambarus Cc: michael, vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, Nicolas.Ferre, zhengxunli, jaimeliao Hi Tudor, On 17/12/21 12:38PM, Tudor.Ambarus@microchip.com wrote: > On 12/17/21 1:38 PM, Pratyush Yadav wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On 09/12/21 09:04PM, Tudor Ambarus wrote: > >> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. There are > >> versions of mx66lm1g45g which do not support SFDP, thus use > >> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral > >> interface outputs data always in STR mode for whatever reason. Since > > > > Huh! I hope this is a mistake from the chip designers, because if it > > isn't they need a stern talking-to ;-) > > > >> 8d-8d-8s is not common, avoid reading the ID when enabling the octal dtr > >> mode. Instead, read back the CR2 to check if the switch was successful. > >> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP. > > > > Datasheet? > > MX66LM1G45G datasheet: > https://www.macronix.com/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf Thanks. I see that the RDID waveform holds each byte of the ID for a whole clock cycle. So you would read ab ab cd cd ef ef. I've seen this before somewhere, and sure enough, digging through my inbox I've found this patch [0]. In this read ID is performed but only alternate bytes are compared since they are repeated. I think you should do the same. I feel like reading/comparing 3 bytes is more "robust". BTW, this patch series also adds support for mx66lm1g45g. You might want to use this as reference. [0] https://lore.kernel.org/all/20210812150135.4005-2-zhengxunli.mxic@gmail.com/ -- Regards, Pratyush Yadav Texas Instruments Inc. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g 2021-12-20 10:16 ` Pratyush Yadav @ 2021-12-20 11:05 ` Tudor.Ambarus 0 siblings, 0 replies; 8+ messages in thread From: Tudor.Ambarus @ 2021-12-20 11:05 UTC (permalink / raw) To: p.yadav Cc: michael, vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, Nicolas.Ferre, zhengxunli, jaimeliao On 12/20/21 12:16 PM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Tudor, > > On 17/12/21 12:38PM, Tudor.Ambarus@microchip.com wrote: >> On 12/17/21 1:38 PM, Pratyush Yadav wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On 09/12/21 09:04PM, Tudor Ambarus wrote: >>>> mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. There are >>>> versions of mx66lm1g45g which do not support SFDP, thus use >>>> SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral >>>> interface outputs data always in STR mode for whatever reason. Since >>> >>> Huh! I hope this is a mistake from the chip designers, because if it >>> isn't they need a stern talking-to ;-) >>> >>>> 8d-8d-8s is not common, avoid reading the ID when enabling the octal dtr >>>> mode. Instead, read back the CR2 to check if the switch was successful. >>>> Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP. >>> >>> Datasheet? >> >> MX66LM1G45G datasheet: >> https://www.macronix.com/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf > > Thanks. > > I see that the RDID waveform holds each byte of the ID for a whole clock > cycle. So you would read ab ab cd cd ef ef. I've seen this before > somewhere, and sure enough, digging through my inbox I've found this > patch [0]. In this read ID is performed but only alternate bytes are > compared since they are repeated. I think you should do the same. I feel > like reading/comparing 3 bytes is more "robust". I didn't want to do the 8D-8D-8S readID because I'm not sure if macronix generalizes 8D-8D-8S for all its octal flashes. Since 8D-8D-8S is odd, I felt that reading CR2 is safer. But maybe we'll follow the read CR2 approach when the need arises, so I'll follow Zhengxun's proposal. > > BTW, this patch series also adds support for mx66lm1g45g. You might want > to use this as reference. > > [0] https://lore.kernel.org/all/20210812150135.4005-2-zhengxunli.mxic@gmail.com/ > Right. I will rework it's 01/15 patch and keep Zhengxunli as author, I assume Zhengxunli sent the proposal before me. ta ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag 2021-12-09 19:04 [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Tudor Ambarus 2021-12-09 19:04 ` [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus @ 2021-12-17 10:42 ` Pratyush Yadav 1 sibling, 0 replies; 8+ messages in thread From: Pratyush Yadav @ 2021-12-17 10:42 UTC (permalink / raw) To: Tudor Ambarus Cc: michael, vigneshr, miquel.raynal, richard, linux-mtd, linux-kernel, nicolas.ferre, zhengxunli, jaimeliao On 09/12/21 09:04PM, Tudor Ambarus wrote: > The Soft Reset and Rescue Sequence Support is defined in BFPT_DWORD(16) > starting with JESD216A. The first version of SFDP, JESD216 (April 2011), > defines just the first 9 BFPT DWORDS, thus it does not contain information > about the Software Reset and Rescue Support. Since this support can not > be discovered by parsing the first SFDP version, introduce a flash_info > fixup_flag that will be used either by flashes that define > JESD216 (April 2011) or by flashes that do not define SFDP at all. > In case a flash defines BFPT_DWORD(16) but with wrong values, one should > instead use a post_bfpt() hook and set SNOR_F_SOFT_RESET. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Reviewed-by: Pratyush Yadav <p.yadav@ti.com> -- Regards, Pratyush Yadav Texas Instruments Inc. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag @ 2021-12-17 13:41 Tudor Ambarus 2021-12-17 13:41 ` [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus 0 siblings, 1 reply; 8+ messages in thread From: Tudor Ambarus @ 2021-12-17 13:41 UTC (permalink / raw) To: p.yadav, michael Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel, Tudor Ambarus The Soft Reset and Rescue Sequence Support is defined in BFPT_DWORD(16) starting with JESD216A. The first version of SFDP, JESD216 (April 2011), defines just the first 9 BFPT DWORDS, thus it does not contain information about the Software Reset and Rescue Support. Since this support can not be discovered by parsing the first SFDP version, introduce a flash_info fixup_flag that will be used either by flashes that define JESD216 (April 2011) or by flashes that do not define SFDP at all. In case a flash defines BFPT_DWORD(16) but with wrong values, one should instead use a post_bfpt() hook and set SNOR_F_SOFT_RESET. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Reviewed-by: Pratyush Yadav <p.yadav@ti.com> --- v2: collect R-b drivers/mtd/spi-nor/core.c | 3 +++ drivers/mtd/spi-nor/core.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 2e21d5ac0e2d..32d80fdaa2a2 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -2699,6 +2699,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor) if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE) nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE; + + if (fixup_flags & SPI_NOR_SOFT_RESET) + nor->flags |= SNOR_F_SOFT_RESET; } /** diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index 2afb610853a9..70c6bb7f5f04 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -373,6 +373,8 @@ struct spi_nor_fixups { * memory size above 128Mib. * SPI_NOR_IO_MODE_EN_VOLATILE: flash enables the best available I/O mode * via a volatile bit. + * SPI_NOR_SOFT_RESET: flash supports software reset enable, reset + * sequence. * @mfr_flags: manufacturer private flags. Used in the manufacturer fixup * hooks to differentiate support between flashes of the same * manufacturer. @@ -416,6 +418,7 @@ struct flash_info { u8 fixup_flags; #define SPI_NOR_4B_OPCODES BIT(0) #define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1) +#define SPI_NOR_SOFT_RESET BIT(2) u8 mfr_flags; -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g 2021-12-17 13:41 Tudor Ambarus @ 2021-12-17 13:41 ` Tudor Ambarus 0 siblings, 0 replies; 8+ messages in thread From: Tudor Ambarus @ 2021-12-17 13:41 UTC (permalink / raw) To: p.yadav, michael Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel, Tudor Ambarus mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. There are versions of mx66lm1g45g which do not support SFDP, thus use SPI_NOR_SKIP_SFDP. The RDID command issued through the octal peripheral interface outputs data always in STR mode for whatever reason. Since 8d-8d-8s is not common, avoid reading the ID when enabling the octal dtr mode. Instead, read back the CR2 to check if the switch was successful. Tested in 1-1-1 and 8d-8d-8d modes using sama7g5 QSPI IP. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- v2: - drop setting of dummy cycles, use the default value - avoid odd lengths in octal dtr mode - s/8d-8d-8d/8D-8D-8D # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id c2853b # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer macronix # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/partname mx66lm1g45g # cat /sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp cat: can't open '/sys/devices/platform/soc/e080c000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp': No such file or directory drivers/mtd/spi-nor/macronix.c | 96 ++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c index 67aaa83038b6..4c672deb1d1c 100644 --- a/drivers/mtd/spi-nor/macronix.c +++ b/drivers/mtd/spi-nor/macronix.c @@ -32,6 +32,95 @@ static struct spi_nor_fixups mx25l25635_fixups = { .post_bfpt = mx25l25635_post_bfpt_fixups, }; +#define SPINOR_OP_READ_CR2 0x71 +#define SPINOR_OP_WRITE_CR2 0x72 +#define SPINOR_OP_MX_DTR_RD 0xee + +#define SPINOR_REG_CR2_MODE_ADDR 0 +#define SPINOR_REG_CR2_DTR_OPI_ENABLE BIT(1) +#define SPINOR_REG_CR2_SPI 0 + +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) +{ + u8 *buf = nor->bouncebuf; + struct spi_mem_op op; + int ret; + + /* Set/unset the octal and DTR enable bits. */ + if (enable) { + buf[0] = SPINOR_REG_CR2_DTR_OPI_ENABLE; + } else { + /* + * The register is one byte wide, but the one byte transactions + * are not allowed in 8D-8D-8D mode. Since there is no register + * at the next location, just initialize the value to zero and + * let the transaction go on. + */ + buf[0] = SPINOR_REG_CR2_SPI; + buf[1] = 0; + } + + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(enable ? 1 : 2, buf, 1)); + if (!enable) + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); + + ret = spi_nor_write_enable(nor); + if (ret) + return ret; + + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) + return ret; + + /* Read back CR2 to make sure the switch was successful. */ + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_READ_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), + SPI_MEM_OP_DATA_IN(enable ? 2 : 1, buf, 1)); + if (enable) + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); + + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) + return ret; + + if (enable) { + if (buf[0] != SPINOR_REG_CR2_DTR_OPI_ENABLE) { + dev_dbg(nor->dev, "Failed to enable 8D-8D-8D mode.\n"); + return -EINVAL; + } + } else if (buf[0] != SPINOR_REG_CR2_SPI) { + dev_dbg(nor->dev, "Failed to disable 8D-8D-8D mode.\n"); + return -EINVAL; + } + + return 0; +} + +static void mx66lm1g45g_late_init(struct spi_nor *nor) +{ + nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; + + /* Set the Fast Read settings. */ + nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR; + spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR], + 0, 20, SPINOR_OP_MX_DTR_RD, + SNOR_PROTO_8_8_8_DTR); + + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; + nor->params->rdsr_dummy = 4; + nor->params->rdsr_addr_nbytes = 4; +} + +static struct spi_nor_fixups mx66lm1g45g_fixups = { + .late_init = mx66lm1g45g_late_init, +}; + static const struct flash_info macronix_parts[] = { /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1) @@ -100,6 +189,13 @@ static const struct flash_info macronix_parts[] = { { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096) NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) FIXUP_FLAGS(SPI_NOR_4B_OPCODES) }, + { "mx66lm1g45g", INFO(0xc2853b, 0, 64 * 1024, 2048) + NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SECT_4K | + SPI_NOR_OCTAL_DTR_READ | SPI_NOR_OCTAL_DTR_PP) + FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_IO_MODE_EN_VOLATILE | + SPI_NOR_SOFT_RESET) + .fixups = &mx66lm1g45g_fixups, + }, }; static void macronix_default_init(struct spi_nor *nor) -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-20 11:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-09 19:04 [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Tudor Ambarus 2021-12-09 19:04 ` [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus 2021-12-17 11:38 ` Pratyush Yadav 2021-12-17 12:38 ` Tudor.Ambarus 2021-12-20 10:16 ` Pratyush Yadav 2021-12-20 11:05 ` Tudor.Ambarus 2021-12-17 10:42 ` [PATCH v2 1/2] mtd: spi-nor: core: Introduce SPI_NOR_SOFT_RESET flash_info fixup_flag Pratyush Yadav 2021-12-17 13:41 Tudor Ambarus 2021-12-17 13:41 ` [PATCH v2 2/2] mtd: spi-nor: macronix: Add support for mx66lm1g45g Tudor Ambarus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).