* [PATCH 0/3] Add octal DTR support for Macronix flash
@ 2021-01-29 8:13 zhengxunli
2021-01-29 8:13 ` [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash zhengxunli
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: zhengxunli @ 2021-01-29 8:13 UTC (permalink / raw)
To: linux-mtd, miquel.raynal; +Cc: juliensu, ycllin, zhengxunli
Hi,
This series adds support for Octal DTR for Macronix flashes. The
first set of patches is add Macronix octaflash series octal dtr
mode support. The second set of patches add the Octal DTR mode
support for host driver. The last set of patches is add the
maximum speed limit of the host to prevent the device from not
working properly.
zhengxunli (3):
mtd: spi-nor: macronix: add support for Macronix octaflash
spi: mxic: patch for octal DTR mode support
spi: mxic: add maximum speed of spi host
Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 +
drivers/mtd/spi-nor/macronix.c | 122 +++++++++++++++++++++
drivers/spi/spi-mxic.c | 40 +++++--
3 files changed, 155 insertions(+), 11 deletions(-)
--
1.9.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash 2021-01-29 8:13 [PATCH 0/3] Add octal DTR support for Macronix flash zhengxunli @ 2021-01-29 8:13 ` zhengxunli 2021-02-01 14:44 ` Miquel Raynal 2021-02-01 19:55 ` Pratyush Yadav 2021-01-29 8:13 ` [PATCH 2/3] spi: mxic: patch for octal DTR mode support zhengxunli 2021-01-29 8:13 ` [PATCH 3/3] spi: mxic: add maximum speed of spi host zhengxunli 2 siblings, 2 replies; 13+ messages in thread From: zhengxunli @ 2021-01-29 8:13 UTC (permalink / raw) To: linux-mtd, miquel.raynal; +Cc: juliensu, ycllin, zhengxunli 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) + 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) -- 1.9.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash 2021-01-29 8:13 ` [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash zhengxunli @ 2021-02-01 14:44 ` Miquel Raynal 2021-02-01 15:06 ` Miquel Raynal 2021-02-01 19:55 ` Pratyush Yadav 1 sibling, 1 reply; 13+ messages in thread From: Miquel Raynal @ 2021-02-01 14:44 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd 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 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash 2021-02-01 14:44 ` Miquel Raynal @ 2021-02-01 15:06 ` Miquel Raynal 0 siblings, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-02-01 15:06 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd > > + 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 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash 2021-01-29 8:13 ` [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash zhengxunli 2021-02-01 14:44 ` Miquel Raynal @ 2021-02-01 19:55 ` Pratyush Yadav 1 sibling, 0 replies; 13+ messages in thread From: Pratyush Yadav @ 2021-02-01 19:55 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd, miquel.raynal 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. -- Regards, Pratyush Yadav Texas Instruments Inc. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] spi: mxic: patch for octal DTR mode support 2021-01-29 8:13 [PATCH 0/3] Add octal DTR support for Macronix flash zhengxunli 2021-01-29 8:13 ` [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash zhengxunli @ 2021-01-29 8:13 ` zhengxunli 2021-02-01 14:49 ` Miquel Raynal 2021-01-29 8:13 ` [PATCH 3/3] spi: mxic: add maximum speed of spi host zhengxunli 2 siblings, 1 reply; 13+ messages in thread From: zhengxunli @ 2021-01-29 8:13 UTC (permalink / raw) To: linux-mtd, miquel.raynal; +Cc: juliensu, ycllin, zhengxunli Driver patch for octal 8D-8D-8D mode support. Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> --- drivers/spi/spi-mxic.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c index 96b4182..821328a 100644 --- a/drivers/spi/spi-mxic.c +++ b/drivers/spi/spi-mxic.c @@ -335,8 +335,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, static bool mxic_spi_mem_supports_op(struct spi_mem *mem, const struct spi_mem_op *op) { - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) return false; if (op->data.nbytes && op->dummy.nbytes && @@ -346,7 +346,7 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem, if (op->addr.nbytes > 7) return false; - return spi_mem_default_supports_op(mem, op); + return true; } static int mxic_spi_mem_exec_op(struct spi_mem *mem, @@ -355,14 +355,15 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master); int nio = 1, i, ret; u32 ss_ctrl; - u8 addr[8]; - u8 opcode = op->cmd.opcode; + u8 addr[8], cmd[2]; ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); if (ret) return ret; - if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) + if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL)) + nio = 8; + else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) nio = 4; else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) nio = 2; @@ -374,19 +375,25 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, mxic->regs + HC_CFG); writel(HC_EN_BIT, mxic->regs + HC_EN); - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | + (op->cmd.dtr ? OP_CMD_DDR : 0); if (op->addr.nbytes) ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | + (op->addr.dtr ? OP_ADDR_DDR : 0); if (op->dummy.nbytes) ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); if (op->data.nbytes) { - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | + (op->data.dtr ? OP_DATA_DDR : 0); if (op->data.dir == SPI_MEM_DATA_IN) ss_ctrl |= OP_READ; + if (op->data.dtr) + ss_ctrl |= OP_DQS_EN; } writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select)); @@ -394,7 +401,10 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT, mxic->regs + HC_CFG); - ret = mxic_spi_data_xfer(mxic, &opcode, NULL, 1); + for (i = 0; i < op->cmd.nbytes; i++) + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1)); + + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); if (ret) goto out; @@ -567,7 +577,8 @@ static int mxic_spi_probe(struct platform_device *pdev) master->bits_per_word_mask = SPI_BPW_MASK(8); master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_TX_DUAL | - SPI_RX_QUAD | SPI_TX_QUAD; + SPI_RX_QUAD | SPI_TX_QUAD | + SPI_RX_OCTAL | SPI_TX_OCTAL; mxic_spi_hw_init(mxic); -- 1.9.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] spi: mxic: patch for octal DTR mode support 2021-01-29 8:13 ` [PATCH 2/3] spi: mxic: patch for octal DTR mode support zhengxunli @ 2021-02-01 14:49 ` Miquel Raynal 2021-02-01 20:10 ` Pratyush Yadav [not found] ` <OF6211E4EF.55839180-ON48258670.0011946B-48258670.001EE31F@mxic.com.tw> 0 siblings, 2 replies; 13+ messages in thread From: Miquel Raynal @ 2021-02-01 14:49 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd Hello, zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri, 29 Jan 2021 16:13:37 +0800: > Driver patch for octal 8D-8D-8D mode support. > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > --- > drivers/spi/spi-mxic.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > index 96b4182..821328a 100644 > --- a/drivers/spi/spi-mxic.c > +++ b/drivers/spi/spi-mxic.c > @@ -335,8 +335,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, > static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > const struct spi_mem_op *op) > { > - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || > - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) > + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || > + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) > return false; > > if (op->data.nbytes && op->dummy.nbytes && > @@ -346,7 +346,7 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > if (op->addr.nbytes > 7) > return false; > > - return spi_mem_default_supports_op(mem, op); > + return true; Does not seem correct. Why would you drop this check? > } > > static int mxic_spi_mem_exec_op(struct spi_mem *mem, > @@ -355,14 +355,15 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master); > int nio = 1, i, ret; > u32 ss_ctrl; > - u8 addr[8]; > - u8 opcode = op->cmd.opcode; > + u8 addr[8], cmd[2]; > > ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); > if (ret) > return ret; > > - if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > + if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL)) > + nio = 8; > + else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > nio = 4; > else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) > nio = 2; > @@ -374,19 +375,25 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > mxic->regs + HC_CFG); > writel(HC_EN_BIT, mxic->regs + HC_EN); > > - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); > + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | > + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | > + (op->cmd.dtr ? OP_CMD_DDR : 0); > > if (op->addr.nbytes) > ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | > - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); > + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | > + (op->addr.dtr ? OP_ADDR_DDR : 0); > > if (op->dummy.nbytes) > ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); > > if (op->data.nbytes) { > - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); > + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | > + (op->data.dtr ? OP_DATA_DDR : 0); > if (op->data.dir == SPI_MEM_DATA_IN) > ss_ctrl |= OP_READ; > + if (op->data.dtr) > + ss_ctrl |= OP_DQS_EN; > } > > writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select)); > @@ -394,7 +401,10 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT, > mxic->regs + HC_CFG); > > - ret = mxic_spi_data_xfer(mxic, &opcode, NULL, 1); > + for (i = 0; i < op->cmd.nbytes; i++) Can we add a check in mxic_spi_mem_check_op to ensure nbytes is never > 2 ? > + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1)); > + > + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > if (ret) > goto out; > > @@ -567,7 +577,8 @@ static int mxic_spi_probe(struct platform_device *pdev) > master->bits_per_word_mask = SPI_BPW_MASK(8); > master->mode_bits = SPI_CPOL | SPI_CPHA | > SPI_RX_DUAL | SPI_TX_DUAL | > - SPI_RX_QUAD | SPI_TX_QUAD; > + SPI_RX_QUAD | SPI_TX_QUAD | > + SPI_RX_OCTAL | SPI_TX_OCTAL; > > mxic_spi_hw_init(mxic); > Otherwise looks fine. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] spi: mxic: patch for octal DTR mode support 2021-02-01 14:49 ` Miquel Raynal @ 2021-02-01 20:10 ` Pratyush Yadav 2021-02-01 21:20 ` Miquel Raynal [not found] ` <OF6211E4EF.55839180-ON48258670.0011946B-48258670.001EE31F@mxic.com.tw> 1 sibling, 1 reply; 13+ messages in thread From: Pratyush Yadav @ 2021-02-01 20:10 UTC (permalink / raw) To: Miquel Raynal; +Cc: juliensu, linux-mtd, ycllin, zhengxunli On 01/02/21 03:49PM, Miquel Raynal wrote: > Hello, > > zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri, 29 Jan 2021 16:13:37 > +0800: > > > Driver patch for octal 8D-8D-8D mode support. > > > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > > --- > > drivers/spi/spi-mxic.c | 33 ++++++++++++++++++++++----------- > > 1 file changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > > index 96b4182..821328a 100644 > > --- a/drivers/spi/spi-mxic.c > > +++ b/drivers/spi/spi-mxic.c > > @@ -335,8 +335,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, > > static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > > const struct spi_mem_op *op) > > { > > - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || > > - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) > > + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || > > + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) > > return false; Can the controller support mixed DTR modes? For example, can it support 4S-4D-4D operations? If no, then please add a check for that here. See cqspi_supports_mem_op() for an example. > > > > if (op->data.nbytes && op->dummy.nbytes && > > @@ -346,7 +346,7 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > > if (op->addr.nbytes > 7) > > return false; > > > > - return spi_mem_default_supports_op(mem, op); > > + return true; > > Does not seem correct. Why would you drop this check? spi_mem_default_supports_op() rejects DTR ops for backward compatibility. But skipping that would mean skipping the spi_check_buswidth_req() calls [0]. Maybe we should export that part as a library function so controllers can use it and not have to roll their own logic? [0] They are not _technically_ needed. Not calling them would mean the spi-{rx,tx}-bus-width DT properties would be ignored. The negotiation for supported opcodes will happen on what the controller _actually_ supports and what SPI NOR says the flash supports. So for example you can't force a octal capable flash to use quad mode. Not sure if that is a good thing or a bad thing. > > } > > > > static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > @@ -355,14 +355,15 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master); > > int nio = 1, i, ret; > > u32 ss_ctrl; > > - u8 addr[8]; > > - u8 opcode = op->cmd.opcode; > > + u8 addr[8], cmd[2]; > > > > ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); > > if (ret) > > return ret; > > > > - if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > > + if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL)) > > + nio = 8; > > + else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) Hmm, shouldn't you be looking at op->*.buswidth? > > nio = 4; > > else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) > > nio = 2; > > @@ -374,19 +375,25 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > mxic->regs + HC_CFG); > > writel(HC_EN_BIT, mxic->regs + HC_EN); > > > > - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); > > + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | > > + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | > > + (op->cmd.dtr ? OP_CMD_DDR : 0); > > > > if (op->addr.nbytes) > > ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | > > - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); > > + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | > > + (op->addr.dtr ? OP_ADDR_DDR : 0); > > > > if (op->dummy.nbytes) > > ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); > > > > if (op->data.nbytes) { > > - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); > > + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | > > + (op->data.dtr ? OP_DATA_DDR : 0); > > if (op->data.dir == SPI_MEM_DATA_IN) > > ss_ctrl |= OP_READ; > > + if (op->data.dtr) > > + ss_ctrl |= OP_DQS_EN; > > } > > > > writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select)); > > @@ -394,7 +401,10 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT, > > mxic->regs + HC_CFG); > > > > - ret = mxic_spi_data_xfer(mxic, &opcode, NULL, 1); > > + for (i = 0; i < op->cmd.nbytes; i++) > > Can we add a check in mxic_spi_mem_check_op to ensure nbytes is never > > 2 ? > > > + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1)); > > + > > + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > > if (ret) > > goto out; > > > > @@ -567,7 +577,8 @@ static int mxic_spi_probe(struct platform_device *pdev) > > master->bits_per_word_mask = SPI_BPW_MASK(8); > > master->mode_bits = SPI_CPOL | SPI_CPHA | > > SPI_RX_DUAL | SPI_TX_DUAL | > > - SPI_RX_QUAD | SPI_TX_QUAD; > > + SPI_RX_QUAD | SPI_TX_QUAD | > > + SPI_RX_OCTAL | SPI_TX_OCTAL; > > > > mxic_spi_hw_init(mxic); > > > > Otherwise looks fine. > > Thanks, > Miquèl > -- Regards, Pratyush Yadav Texas Instruments Inc. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] spi: mxic: patch for octal DTR mode support 2021-02-01 20:10 ` Pratyush Yadav @ 2021-02-01 21:20 ` Miquel Raynal 0 siblings, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-02-01 21:20 UTC (permalink / raw) To: Pratyush Yadav; +Cc: juliensu, linux-mtd, ycllin, zhengxunli Hi Pratyush, Pratyush Yadav <p.yadav@ti.com> wrote on Tue, 2 Feb 2021 01:40:30 +0530: > On 01/02/21 03:49PM, Miquel Raynal wrote: > > Hello, > > > > zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri, 29 Jan 2021 16:13:37 > > +0800: > > > > > Driver patch for octal 8D-8D-8D mode support. > > > > > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > > > --- > > > drivers/spi/spi-mxic.c | 33 ++++++++++++++++++++++----------- > > > 1 file changed, 22 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > > > index 96b4182..821328a 100644 > > > --- a/drivers/spi/spi-mxic.c > > > +++ b/drivers/spi/spi-mxic.c > > > @@ -335,8 +335,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, > > > static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > > > const struct spi_mem_op *op) > > > { > > > - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || > > > - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) > > > + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || > > > + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) > > > return false; > > Can the controller support mixed DTR modes? For example, can it support > 4S-4D-4D operations? If no, then please add a check for that here. See > cqspi_supports_mem_op() for an example. > > > > > > > if (op->data.nbytes && op->dummy.nbytes && > > > @@ -346,7 +346,7 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > > > if (op->addr.nbytes > 7) > > > return false; > > > > > > - return spi_mem_default_supports_op(mem, op); > > > + return true; > > > > Does not seem correct. Why would you drop this check? > > spi_mem_default_supports_op() rejects DTR ops for backward > compatibility. > > But skipping that would mean skipping the spi_check_buswidth_req() calls > [0]. Maybe we should export that part as a library function so > controllers can use it and not have to roll their own logic? > > [0] They are not _technically_ needed. Not calling them would mean the > spi-{rx,tx}-bus-width DT properties would be ignored. The negotiation > for supported opcodes will happen on what the controller _actually_ > supports and what SPI NOR says the flash supports. So for example you > can't force a octal capable flash to use quad mode. Not sure if that is > a good thing or a bad thing. I don't think is a good idea to ignore the user inputs. If these properties are populated one will not understand why, in this particular case, they won't get parsed. Perhaps having a spi_mem_dtr_supports_op() or something alike which calls a common helper (eg. spi_mem_generic_supports_op()) shared with spi_mem_default_supports_op() would make sense. > > > > } > > > > > > static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > > @@ -355,14 +355,15 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > > struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master); > > > int nio = 1, i, ret; > > > u32 ss_ctrl; > > > - u8 addr[8]; > > > - u8 opcode = op->cmd.opcode; > > > + u8 addr[8], cmd[2]; > > > > > > ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); > > > if (ret) > > > return ret; > > > > > > - if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > > > + if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL)) > > > + nio = 8; > > > + else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > > Hmm, shouldn't you be looking at op->*.buswidth? > > > > nio = 4; > > > else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) > > > nio = 2; > > > @@ -374,19 +375,25 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > > mxic->regs + HC_CFG); > > > writel(HC_EN_BIT, mxic->regs + HC_EN); > > > > > > - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); > > > + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | > > > + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | > > > + (op->cmd.dtr ? OP_CMD_DDR : 0); > > > > > > if (op->addr.nbytes) > > > ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | > > > - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); > > > + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | > > > + (op->addr.dtr ? OP_ADDR_DDR : 0); > > > > > > if (op->dummy.nbytes) > > > ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); > > > > > > if (op->data.nbytes) { > > > - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); > > > + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | > > > + (op->data.dtr ? OP_DATA_DDR : 0); > > > if (op->data.dir == SPI_MEM_DATA_IN) > > > ss_ctrl |= OP_READ; > > > + if (op->data.dtr) > > > + ss_ctrl |= OP_DQS_EN; > > > } > > > > > > writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select)); > > > @@ -394,7 +401,10 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > > writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT, > > > mxic->regs + HC_CFG); > > > > > > - ret = mxic_spi_data_xfer(mxic, &opcode, NULL, 1); > > > + for (i = 0; i < op->cmd.nbytes; i++) > > > > Can we add a check in mxic_spi_mem_check_op to ensure nbytes is never > > > 2 ? > > > > > + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1)); > > > + > > > + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > > > if (ret) > > > goto out; > > > > > > @@ -567,7 +577,8 @@ static int mxic_spi_probe(struct platform_device *pdev) > > > master->bits_per_word_mask = SPI_BPW_MASK(8); > > > master->mode_bits = SPI_CPOL | SPI_CPHA | > > > SPI_RX_DUAL | SPI_TX_DUAL | > > > - SPI_RX_QUAD | SPI_TX_QUAD; > > > + SPI_RX_QUAD | SPI_TX_QUAD | > > > + SPI_RX_OCTAL | SPI_TX_OCTAL; > > > > > > mxic_spi_hw_init(mxic); > > > > > > > Otherwise looks fine. > > > > Thanks, > > Miquèl > > > Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <OF6211E4EF.55839180-ON48258670.0011946B-48258670.001EE31F@mxic.com.tw>]
* Re: [PATCH 2/3] spi: mxic: patch for octal DTR mode support [not found] ` <OF6211E4EF.55839180-ON48258670.0011946B-48258670.001EE31F@mxic.com.tw> @ 2021-02-02 8:06 ` Miquel Raynal 0 siblings, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-02-02 8:06 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd Hello, zhengxunli@mxic.com.tw wrote on Tue, 2 Feb 2021 13:37:22 +0800: > Hi, > > > > Driver patch for octal 8D-8D-8D mode support. > > > > > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > > > --- > > > drivers/spi/spi-mxic.c | 33 ++++++++++++++++++++++----------- > > > 1 file changed, 22 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > > > index 96b4182..821328a 100644 > > > --- a/drivers/spi/spi-mxic.c > > > +++ b/drivers/spi/spi-mxic.c > > > @@ -335,8 +335,8 @@ static int mxic_spi_data_xfer(struct mxic_spi > > *mxic, const void *txbuf, > > > static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > > > const struct spi_mem_op *op) > > > { > > > - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || > > > - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) > > > + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || > > > + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) > > > return false; > > > > > > if (op->data.nbytes && op->dummy.nbytes && > > > @@ -346,7 +346,7 @@ static bool mxic_spi_mem_supports_op(struct > > spi_mem *mem, > > > if (op->addr.nbytes > 7) > > > return false; > > > > > > - return spi_mem_default_supports_op(mem, op); > > > + return true; > > > > Does not seem correct. Why would you drop this check? > > Same as Pratyush's reply. spi_mem_default_supports_op() does not support > dtr. As I said: spi_mem_default_supports_op() do many checks, I don't think it's worth giving up these checks and having a complementary helper for DTR operations is needed. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] spi: mxic: add maximum speed of spi host 2021-01-29 8:13 [PATCH 0/3] Add octal DTR support for Macronix flash zhengxunli 2021-01-29 8:13 ` [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash zhengxunli 2021-01-29 8:13 ` [PATCH 2/3] spi: mxic: patch for octal DTR mode support zhengxunli @ 2021-01-29 8:13 ` zhengxunli 2021-02-01 15:02 ` Miquel Raynal 2 siblings, 1 reply; 13+ messages in thread From: zhengxunli @ 2021-01-29 8:13 UTC (permalink / raw) To: linux-mtd, miquel.raynal; +Cc: juliensu, ycllin, zhengxunli Add the maximum speed of the host to avoid over-speed operation of the device. Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> --- Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 ++++ drivers/spi/spi-mxic.c | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt b/Documentation/devicetree/bindings/spi/spi-mxic.txt index 529f2da..ff6937c 100644 --- a/Documentation/devicetree/bindings/spi/spi-mxic.txt +++ b/Documentation/devicetree/bindings/spi/spi-mxic.txt @@ -13,6 +13,9 @@ Required properties: - clocks: should contain 3 entries for the "ps_clk", "send_clk" and "send_dly_clk" clocks +Recommended properties: +- spi-max-frequency: Maximum SPI clocking speed of the device in Hz. + Example: spi@43c30000 { @@ -21,6 +24,7 @@ Example: reg-names = "regs", "dirmap"; clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>; clock-names = "send_clk", "send_dly_clk", "ps_clk"; + spi-max-frequency = <200000000>; #address-cells = <1>; #size-cells = <0>; diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c index 821328a..7786d33 100644 --- a/drivers/spi/spi-mxic.c +++ b/drivers/spi/spi-mxic.c @@ -12,6 +12,7 @@ #include <linux/io.h> #include <linux/iopoll.h> #include <linux/module.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/spi/spi.h> @@ -536,7 +537,9 @@ static int mxic_spi_probe(struct platform_device *pdev) { struct spi_master *master; struct resource *res; + struct device_node *np = pdev->dev.of_node; struct mxic_spi *mxic; + u32 max_freq; int ret; master = devm_spi_alloc_master(&pdev->dev, sizeof(struct mxic_spi)); @@ -580,6 +583,10 @@ static int mxic_spi_probe(struct platform_device *pdev) SPI_RX_QUAD | SPI_TX_QUAD | SPI_RX_OCTAL | SPI_TX_OCTAL; + if (of_property_read_u32(np, "spi-max-frequency", + &master->max_speed_hz)) + master->max_speed_hz = 25000000; /* 25MHz */ + mxic_spi_hw_init(mxic); ret = spi_register_master(master); -- 1.9.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] spi: mxic: add maximum speed of spi host 2021-01-29 8:13 ` [PATCH 3/3] spi: mxic: add maximum speed of spi host zhengxunli @ 2021-02-01 15:02 ` Miquel Raynal [not found] ` <OFCBF59ED0.4C198CA3-ON48258670.00236718-48258670.0026E452@mxic.com.tw> 0 siblings, 1 reply; 13+ messages in thread From: Miquel Raynal @ 2021-02-01 15:02 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd Hi, zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri, 29 Jan 2021 16:13:38 +0800: > Add the maximum speed of the host to avoid over-speed > operation of the device. > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > --- > Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 ++++ > drivers/spi/spi-mxic.c | 7 +++++++ > 2 files changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt b/Documentation/devicetree/bindings/spi/spi-mxic.txt > index 529f2da..ff6937c 100644 > --- a/Documentation/devicetree/bindings/spi/spi-mxic.txt > +++ b/Documentation/devicetree/bindings/spi/spi-mxic.txt Binding and driver changes should be in two separate commits. > @@ -13,6 +13,9 @@ Required properties: > - clocks: should contain 3 entries for the "ps_clk", "send_clk" and > "send_dly_clk" clocks > > +Recommended properties: > +- spi-max-frequency: Maximum SPI clocking speed of the device in Hz. I don't think this property should be part of the controller binding. It is a per device property and is already parsed by the core (see of_spi_parse_dt() in spi.c). > + > Example: > > spi@43c30000 { > @@ -21,6 +24,7 @@ Example: > reg-names = "regs", "dirmap"; > clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>; > clock-names = "send_clk", "send_dly_clk", "ps_clk"; > + spi-max-frequency = <200000000>; > #address-cells = <1>; > #size-cells = <0>; > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > index 821328a..7786d33 100644 > --- a/drivers/spi/spi-mxic.c > +++ b/drivers/spi/spi-mxic.c > @@ -12,6 +12,7 @@ > #include <linux/io.h> > #include <linux/iopoll.h> > #include <linux/module.h> > +#include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/spi/spi.h> > @@ -536,7 +537,9 @@ static int mxic_spi_probe(struct platform_device *pdev) > { > struct spi_master *master; > struct resource *res; > + struct device_node *np = pdev->dev.of_node; > struct mxic_spi *mxic; > + u32 max_freq; > int ret; > > master = devm_spi_alloc_master(&pdev->dev, sizeof(struct mxic_spi)); > @@ -580,6 +583,10 @@ static int mxic_spi_probe(struct platform_device *pdev) > SPI_RX_QUAD | SPI_TX_QUAD | > SPI_RX_OCTAL | SPI_TX_OCTAL; > > + if (of_property_read_u32(np, "spi-max-frequency", > + &master->max_speed_hz)) > + master->max_speed_hz = 25000000; /* 25MHz */ As said before, there are two limitations to take into account: 1- your controller maximum speed 2- your flash maximum speed (1) I don't think this needs to be exported through the DT, the driver knows his own limitations and enforces them. (2) Should be either advertised by the DT through the spi-max-frequency property or advertised somehow by the vendor driver (macronix.c). > + > mxic_spi_hw_init(mxic); > > ret = spi_register_master(master); Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <OFCBF59ED0.4C198CA3-ON48258670.00236718-48258670.0026E452@mxic.com.tw>]
* Re: [PATCH 3/3] spi: mxic: add maximum speed of spi host [not found] ` <OFCBF59ED0.4C198CA3-ON48258670.00236718-48258670.0026E452@mxic.com.tw> @ 2021-02-02 8:04 ` Miquel Raynal 0 siblings, 0 replies; 13+ messages in thread From: Miquel Raynal @ 2021-02-02 8:04 UTC (permalink / raw) To: zhengxunli; +Cc: juliensu, ycllin, linux-mtd Hello, > > > @@ -536,7 +537,9 @@ static int mxic_spi_probe(struct platform_device > *pdev) > > > { > > > struct spi_master *master; > > > struct resource *res; > > > + struct device_node *np = pdev->dev.of_node; > > > struct mxic_spi *mxic; > > > + u32 max_freq; > > > int ret; > > > > > > master = devm_spi_alloc_master(&pdev->dev, sizeof(struct > mxic_spi)); > > > @@ -580,6 +583,10 @@ static int mxic_spi_probe(struct platform_device > *pdev) > > > SPI_RX_QUAD | SPI_TX_QUAD | > > > SPI_RX_OCTAL | SPI_TX_OCTAL; > > > > > > + if (of_property_read_u32(np, "spi-max-frequency", > > > + &master->max_speed_hz)) > > > + master->max_speed_hz = 25000000; /* 25MHz */ > > > > As said before, there are two limitations to take into account: > > 1- your controller maximum speed > > 2- your flash maximum speed > > > > (1) I don't think this needs to be exported through the DT, the driver > > knows his own limitations and enforces them. > > In fact, if master->max_speed_hz is not set, spi->max_speed_hz will be 0 > and the > host frequency cannot be set correctly. You should set master->max_speed_hz in the probe. This is the maximum speed your host can run at and has *nothing to do* with whatever device is attached on the bus. In the DT, use the spi-max-frequency property to inform the controller about the maximum speed the device supports. Then the controller should receive requests at the frequency chosen with: min(master_max_freq, device_max_freq). Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-02-02 8:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29 8:13 [PATCH 0/3] Add octal DTR support for Macronix flash zhengxunli
2021-01-29 8:13 ` [PATCH 1/3] mtd: spi-nor: macronix: add support for Macronix octaflash zhengxunli
2021-02-01 14:44 ` Miquel Raynal
2021-02-01 15:06 ` Miquel Raynal
2021-02-01 19:55 ` Pratyush Yadav
2021-01-29 8:13 ` [PATCH 2/3] spi: mxic: patch for octal DTR mode support zhengxunli
2021-02-01 14:49 ` Miquel Raynal
2021-02-01 20:10 ` Pratyush Yadav
2021-02-01 21:20 ` Miquel Raynal
[not found] ` <OF6211E4EF.55839180-ON48258670.0011946B-48258670.001EE31F@mxic.com.tw>
2021-02-02 8:06 ` Miquel Raynal
2021-01-29 8:13 ` [PATCH 3/3] spi: mxic: add maximum speed of spi host zhengxunli
2021-02-01 15:02 ` Miquel Raynal
[not found] ` <OFCBF59ED0.4C198CA3-ON48258670.00236718-48258670.0026E452@mxic.com.tw>
2021-02-02 8:04 ` Miquel Raynal
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.