| Message ID | 1611908018-29937-2-git-send-email-zhengxunli@mxic.com.tw |
|---|---|
| State | Superseded |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | Add octal DTR support for Macronix flash | expand |
Hello, zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri, 29 Jan 2021 16:13:36 +0800: > The ocatflash is an xSPI compliant octal DTR flash. Add support > for using it in octal DTR mode. > > Enable Octal DTR mode with 20 dummy cycles to allow running at the > maximum supported frequency of 200Mhz. > > When reading ID in OCTAL DTR mode, ID will appear in a repeated > manner. ex: ID[0] = 0xc2, ID[1] = 0xc2, ID[2] = 0x94, ID[3] = 0x94... > Rearrange the ID so that the ID can pass. > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > --- > drivers/mtd/spi-nor/macronix.c | 122 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 122 insertions(+) > > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c > index 9203aba..7253566 100644 > --- a/drivers/mtd/spi-nor/macronix.c > +++ b/drivers/mtd/spi-nor/macronix.c > @@ -8,6 +8,15 @@ > > #include "core.h" > > +#define SPINOR_OP_RD_CR2 0x71 /* Read configuration register 2 */ > +#define SPINOR_OP_WR_CR2 0x72 /* Write configuration register 2 */ > +#define SPINOR_OP_MXIC_DTR_RD 0xee /* Fast Read opcode in DTR mode */ > +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting octal DTR mode */ > +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */ > +#define SPINOR_REG_MXIC_OPI_DTR_DIS 0x1 /* Disable Octal DTR */ > +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For setting dummy cycles */ > +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting dummy cycles to 20 */ > + > static int > mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > const struct sfdp_parameter_header *bfpt_header, > @@ -33,6 +42,115 @@ > .post_bfpt = mx25l25635_post_bfpt_fixups, > }; > > +/** > + * spi_nor_macronix_octal_dtr_enable() - Enable octal DTR on Macronix flashes. > + * @nor: pointer to a 'struct spi_nor' > + * @enable: whether to enable or disable Octal DTR > + * > + * This also sets the memory access dummy cycles to 20 to allow the flash to > + * run at up to 200MHz. > + * > + * Return: 0 on success, -errno otherwise. > + */ > +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) > +{ > + struct spi_mem_op op; > + u8 *buf = nor->bouncebuf, flash_id[3], i; > + int ret; > + > + if (enable) { > + /* Use 20 dummy cycles for memory array reads. */ > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + *buf = SPINOR_REG_MXIC_DC_20; > + op = (struct spi_mem_op) I don't think you need a cast here, do you? (same below) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(1, buf, 1)); > + > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) > + return ret; > + > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + > + nor->read_dummy = 20; I am not entirely convinced by this value, yet. If I understand correctly your issue, the flash needs some extra time before receiving/sending data. You estimate it to be around 800ns (20 bytes @ 20MHz), so would it be possible to derive the minimum number of dummy cycles needed for the flash and use a dynamic value? Otherwise if the controller is not running at the maximum frequency you'll end up waiting a lot more than expected. > + } > + > + /* Set/unset the octal and DTR enable bits. */ > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + if (enable) > + *buf = SPINOR_REG_MXIC_OPI_DTR_EN; > + else > + *buf = SPINOR_REG_MXIC_OPI_DTR_DIS; > + > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(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; > + > + /* Read flash ID to make sure the switch was successful. */ > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1), > + SPI_MEM_OP_ADDR(enable ? 4 : 0, 0, 1), > + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), > + SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, 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; > + > + for (i = 0; i < nor->info->id_len; i++) > + flash_id[i] = buf[i * 2]; > + > + if (memcmp(flash_id, nor->info->id, nor->info->id_len)) > + return -EINVAL; I am a bit confused by the current code organization: here you are trying to validate the flash ID, but you already enabled the octal DTR mode. I would have imagined something more: - Try to detect the chip correctly - Does it supports octal DTR mode ? - If yes, enable it. > + > + return 0; > +} > + > +static void octaflash_default_init(struct spi_nor *nor) > +{ > + nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; > +} > + > +static void octaflash_post_sfdp_fixup(struct spi_nor *nor) > +{ > + /* 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_MXIC_DTR_RD, ^^ If this is the number of dummy cycles, I guess you should use nor->read_dummy instead, unless it has not been populated yet. In this case, please use a define with a generic name and the "highest value" that will make all frequencies this chip is able to run at valid". > + 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 octaflash_fixups = { > + .default_init = octaflash_default_init, > + .post_sfdp = octaflash_post_sfdp_fixup, > +}; > + > static const struct flash_info macronix_parts[] = { > /* Macronix */ > { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, > @@ -90,6 +208,10 @@ > { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096, > SECT_4K | SPI_NOR_DUAL_READ | > SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, > + { "mx66uw2g345g", INFO(0xc2943c, 0, 64 * 1024, 4096, > + SECT_4K | SPI_NOR_OCTAL_DTR_READ | > + SPI_NOR_OCTAL_DTR_PP | SPI_NOR_4B_OPCODES) > + .fixups = &octaflash_fixups }, > }; > > static void macronix_default_init(struct spi_nor *nor) Thanks, Miquèl
> > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), > > + SPI_MEM_OP_NO_DUMMY, > > + SPI_MEM_OP_DATA_OUT(1, buf, 1)); > > + > > + ret = spi_mem_exec_op(nor->spimem, &op); > > + if (ret) > > + return ret; > > + > > + ret = spi_nor_wait_till_ready(nor); > > + if (ret) > > + return ret; > > + > > + nor->read_dummy = 20; > > I am not entirely convinced by this value, yet. > > If I understand correctly your issue, the flash needs some extra time > before receiving/sending data. You estimate it to be around 800ns > (20 bytes @ 20MHz), so would it be possible to derive the minimum > number of dummy cycles needed for the flash and use a dynamic value? > Otherwise if the controller is not running at the maximum frequency > you'll end up waiting a lot more than expected. Just to clarify, I meant 20 bytes @ 200Mhz, not 20MHz, and I got possibly fooled by the fact that we are talking about DDR mode, so 400ns is probably more accurate than 800. But no matter the actual numbers, let's discuss the logic more than the values. Thanks, Miquèl
On 29/01/21 04:13PM, zhengxunli wrote: > The ocatflash is an xSPI compliant octal DTR flash. Add support s/ocatflash/octaflash/ > for using it in octal DTR mode. > > Enable Octal DTR mode with 20 dummy cycles to allow running at the > maximum supported frequency of 200Mhz. > > When reading ID in OCTAL DTR mode, ID will appear in a repeated > manner. ex: ID[0] = 0xc2, ID[1] = 0xc2, ID[2] = 0x94, ID[3] = 0x94... > Rearrange the ID so that the ID can pass. > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > --- > drivers/mtd/spi-nor/macronix.c | 122 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 122 insertions(+) > > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c > index 9203aba..7253566 100644 > --- a/drivers/mtd/spi-nor/macronix.c > +++ b/drivers/mtd/spi-nor/macronix.c > @@ -8,6 +8,15 @@ > > #include "core.h" > > +#define SPINOR_OP_RD_CR2 0x71 /* Read configuration register 2 */ > +#define SPINOR_OP_WR_CR2 0x72 /* Write configuration register 2 */ > +#define SPINOR_OP_MXIC_DTR_RD 0xee /* Fast Read opcode in DTR mode */ > +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting octal DTR mode */ > +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */ > +#define SPINOR_REG_MXIC_OPI_DTR_DIS 0x1 /* Disable Octal DTR */ > +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For setting dummy cycles */ > +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting dummy cycles to 20 */ > + > static int > mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > const struct sfdp_parameter_header *bfpt_header, > @@ -33,6 +42,115 @@ > .post_bfpt = mx25l25635_post_bfpt_fixups, > }; > > +/** > + * spi_nor_macronix_octal_dtr_enable() - Enable octal DTR on Macronix flashes. > + * @nor: pointer to a 'struct spi_nor' > + * @enable: whether to enable or disable Octal DTR > + * > + * This also sets the memory access dummy cycles to 20 to allow the flash to > + * run at up to 200MHz. > + * > + * Return: 0 on success, -errno otherwise. > + */ > +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) > +{ > + struct spi_mem_op op; > + u8 *buf = nor->bouncebuf, flash_id[3], i; > + int ret; > + > + if (enable) { > + /* Use 20 dummy cycles for memory array reads. */ > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + *buf = SPINOR_REG_MXIC_DC_20; > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(1, buf, 1)); > + > + ret = spi_mem_exec_op(nor->spimem, &op); > + if (ret) > + return ret; > + > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + > + nor->read_dummy = 20; > + } > + > + /* Set/unset the octal and DTR enable bits. */ > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + if (enable) > + *buf = SPINOR_REG_MXIC_OPI_DTR_EN; > + else > + *buf = SPINOR_REG_MXIC_OPI_DTR_DIS; > + > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(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; > + > + /* Read flash ID to make sure the switch was successful. */ > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1), > + SPI_MEM_OP_ADDR(enable ? 4 : 0, 0, 1), > + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), > + SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, 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; > + > + for (i = 0; i < nor->info->id_len; i++) > + flash_id[i] = buf[i * 2]; > + > + if (memcmp(flash_id, nor->info->id, nor->info->id_len)) > + return -EINVAL; If you are bothering to loop through 'buf' then just compare it with nor->info->id right there. No need to copy it around and do a memcmp(). > + > + return 0; > +} > + > +static void octaflash_default_init(struct spi_nor *nor) > +{ > + nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; > +} > + > +static void octaflash_post_sfdp_fixup(struct spi_nor *nor) > +{ > + /* 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_MXIC_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 octaflash_fixups = { > + .default_init = octaflash_default_init, > + .post_sfdp = octaflash_post_sfdp_fixup, > +}; > + > static const struct flash_info macronix_parts[] = { > /* Macronix */ > { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, > @@ -90,6 +208,10 @@ > { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096, > SECT_4K | SPI_NOR_DUAL_READ | > SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, > + { "mx66uw2g345g", INFO(0xc2943c, 0, 64 * 1024, 4096, > + SECT_4K | SPI_NOR_OCTAL_DTR_READ | > + SPI_NOR_OCTAL_DTR_PP | SPI_NOR_4B_OPCODES) > + .fixups = &octaflash_fixups }, > }; > > static void macronix_default_init(struct spi_nor *nor) I haven't gone through the flash datasheet so I won't add my R-by tag here, but FWIW the patch fundamentally sound to me.
diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c index 9203aba..7253566 100644 --- a/drivers/mtd/spi-nor/macronix.c +++ b/drivers/mtd/spi-nor/macronix.c @@ -8,6 +8,15 @@ #include "core.h" +#define SPINOR_OP_RD_CR2 0x71 /* Read configuration register 2 */ +#define SPINOR_OP_WR_CR2 0x72 /* Write configuration register 2 */ +#define SPINOR_OP_MXIC_DTR_RD 0xee /* Fast Read opcode in DTR mode */ +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting octal DTR mode */ +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */ +#define SPINOR_REG_MXIC_OPI_DTR_DIS 0x1 /* Disable Octal DTR */ +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For setting dummy cycles */ +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting dummy cycles to 20 */ + static int mx25l25635_post_bfpt_fixups(struct spi_nor *nor, const struct sfdp_parameter_header *bfpt_header, @@ -33,6 +42,115 @@ .post_bfpt = mx25l25635_post_bfpt_fixups, }; +/** + * spi_nor_macronix_octal_dtr_enable() - Enable octal DTR on Macronix flashes. + * @nor: pointer to a 'struct spi_nor' + * @enable: whether to enable or disable Octal DTR + * + * This also sets the memory access dummy cycles to 20 to allow the flash to + * run at up to 200MHz. + * + * Return: 0 on success, -errno otherwise. + */ +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable) +{ + struct spi_mem_op op; + u8 *buf = nor->bouncebuf, flash_id[3], i; + int ret; + + if (enable) { + /* Use 20 dummy cycles for memory array reads. */ + ret = spi_nor_write_enable(nor); + if (ret) + return ret; + + *buf = SPINOR_REG_MXIC_DC_20; + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, buf, 1)); + + ret = spi_mem_exec_op(nor->spimem, &op); + if (ret) + return ret; + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + nor->read_dummy = 20; + } + + /* Set/unset the octal and DTR enable bits. */ + ret = spi_nor_write_enable(nor); + if (ret) + return ret; + + if (enable) + *buf = SPINOR_REG_MXIC_OPI_DTR_EN; + else + *buf = SPINOR_REG_MXIC_OPI_DTR_DIS; + + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(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; + + /* Read flash ID to make sure the switch was successful. */ + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1), + SPI_MEM_OP_ADDR(enable ? 4 : 0, 0, 1), + SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1), + SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, 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; + + for (i = 0; i < nor->info->id_len; i++) + flash_id[i] = buf[i * 2]; + + if (memcmp(flash_id, nor->info->id, nor->info->id_len)) + return -EINVAL; + + return 0; +} + +static void octaflash_default_init(struct spi_nor *nor) +{ + nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; +} + +static void octaflash_post_sfdp_fixup(struct spi_nor *nor) +{ + /* 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_MXIC_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 octaflash_fixups = { + .default_init = octaflash_default_init, + .post_sfdp = octaflash_post_sfdp_fixup, +}; + static const struct flash_info macronix_parts[] = { /* Macronix */ { "mx25l512e", INFO(0xc22010, 0, 64 * 1024, 1, SECT_4K) }, @@ -90,6 +208,10 @@ { "mx66u2g45g", INFO(0xc2253c, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, + { "mx66uw2g345g", INFO(0xc2943c, 0, 64 * 1024, 4096, + SECT_4K | SPI_NOR_OCTAL_DTR_READ | + SPI_NOR_OCTAL_DTR_PP | SPI_NOR_4B_OPCODES) + .fixups = &octaflash_fixups }, }; static void macronix_default_init(struct spi_nor *nor)
The ocatflash is an xSPI compliant octal DTR flash. Add support for using it in octal DTR mode. Enable Octal DTR mode with 20 dummy cycles to allow running at the maximum supported frequency of 200Mhz. When reading ID in OCTAL DTR mode, ID will appear in a repeated manner. ex: ID[0] = 0xc2, ID[1] = 0xc2, ID[2] = 0x94, ID[3] = 0x94... Rearrange the ID so that the ID can pass. Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> --- drivers/mtd/spi-nor/macronix.c | 122 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+)