[PATCH v2 0/2] Add octal DTR support for Macronix flash

[PATCH v2 0/2] Add octal DTR support for Macronix flash

[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.

Changes in v2:
- Define with a generic name to describe the maximum dummy cycles.

- Compare the ID directly in the loop, no longer copy and execute
  memcmp().

- Add spi_mem_dtr_supports_op() to support dtr operation.

zhengxunli (2):
  mtd: spi-nor: macronix: add support for Macronix octaflash
  spi: mxic: patch for octal DTR mode support

 drivers/mtd/spi-nor/macronix.c | 121 +++++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-mxic.c         |  41 ++++++++++----
 2 files changed, 151 insertions(+), 11 deletions(-)

-- 
1.9.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[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.

Try to verify the flash ID to check whether the flash memory in octal
DTR mode is correct. 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 order so that the ID can pass.

Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
---
 drivers/mtd/spi-nor/macronix.c | 121 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 9203aba..7498978 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,16 @@
 
 #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 */
+#define MXIC_MAX_DC			20		/* Maximum value of dummy cycles */
+
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
 			    const struct sfdp_parameter_header *bfpt_header,
@@ -33,6 +43,113 @@
 	.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, 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 = MXIC_MAX_DC;
+	}
+
+	/* 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++)
+		if (buf[i * 2] != nor->info->id[i])
+			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, MXIC_MAX_DC, 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 +207,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/

[PATCH v2 2/2] spi: mxic: patch for octal DTR mode support

[zhengxunli]

Driver patch for octal DTR mode support.

Owing to the spi_mem_default_supports_op() is not support dtr
operation. Based on Pratyush patch "spi: spi-mem: add spi_mem_dtr
_supports_op()" add spi_mem_dtr_supports_op() to support dtr and
keep checking the buswidth and command bytes.

Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
---
 drivers/spi/spi-mxic.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 96b4182..32e757a 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -335,8 +335,10 @@ 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)
+	bool all_false;
+
+	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 +348,13 @@ 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);
+	all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr &&
+		    !op->data.dtr;
+
+	if (all_false)
+		return spi_mem_default_supports_op(mem, op);
+	else
+		return spi_mem_dtr_supports_op(mem, op);
 }
 
 static int mxic_spi_mem_exec_op(struct spi_mem *mem,
@@ -355,14 +363,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 +383,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 +409,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 +585,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/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Miquel Raynal]

Hello,

zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri,  5 Feb 2021 17:36:47
+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.
> 
> Try to verify the flash ID to check whether the flash memory in octal
> DTR mode is correct. 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 order so that the ID can pass.
> 
> Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 121 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 121 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 9203aba..7498978 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -8,6 +8,16 @@
>  
>  #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 */
> +#define MXIC_MAX_DC			20		/* Maximum value of dummy cycles */
> +
>  static int
>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
>  			    const struct sfdp_parameter_header *bfpt_header,
> @@ -33,6 +43,113 @@
>  	.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, 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 = MXIC_MAX_DC;

I am still not convinced by this constant value.

The rest looks good to me.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Pratyush Yadav]

On 05/02/21 10:47AM, Miquel Raynal wrote:
> Hello,
> 
> zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri,  5 Feb 2021 17:36:47
> +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.
> > 
> > Try to verify the flash ID to check whether the flash memory in octal
> > DTR mode is correct. 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 order so that the ID can pass.
> > 
> > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
> > ---
> >  drivers/mtd/spi-nor/macronix.c | 121 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 121 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> > index 9203aba..7498978 100644
> > --- a/drivers/mtd/spi-nor/macronix.c
> > +++ b/drivers/mtd/spi-nor/macronix.c
> > @@ -8,6 +8,16 @@
> >  
> >  #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 */
> > +#define MXIC_MAX_DC			20		/* Maximum value of dummy cycles */
> > +
> >  static int
> >  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
> >  			    const struct sfdp_parameter_header *bfpt_header,
> > @@ -33,6 +43,113 @@
> >  	.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, 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 = MXIC_MAX_DC;
> 
> I am still not convinced by this constant value.

I think a constant value is fine. This dummy cycle value reflects how 
many cycles the master clock would go through before the flash starts 
emitting the data. If the master (aka the controller) is running at a 
lower frequency then those cycles go through slower, but the flash still 
waits for them to finish before emitting data. And since the master is 
driving the clock and the flash is just "reading" it, both remain in 
sync.

The dummy cycles need to be set for the worst case scenario [0]. The 
flash usually needs a minimum amount of time before it is ready to emit 
the data. So for example if the master is at 25 MHz, the clock period is 
longer so 8 clock cycles [1] might be long enough to exceed that minimum 
time. But when the master is running at 200 MHz, the clock period is 
smaller so 8 cycles might not give the flash enough time to prepare. So 
we need to to wait at least 20 cycles [1] before emitting data.

This is what my patches do for the Cypress S28 flash. I have tested it 
on both 25 MHz and 166 MHz with 22 dummy cycles. It is not the most 
efficient at 25 MHz since 5 dummy cycles is all that is needed for that 
speed, but its the best we can do right now.

[0] Since SPI NOR has no way of knowing what speed the controller is 
running at, assume the fastest speed the flash can run at.
[1] Hypothetical example. Don't know the actual values for this flash.
 
> The rest looks good to me.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Miquel Raynal]

Hi Pratyush,

+Tudor, I don't know why he was'nt Cc:'ed.

Pratyush Yadav <p.yadav@ti.com> wrote on Fri, 5 Feb 2021 19:04:04 +0530:

> On 05/02/21 10:47AM, Miquel Raynal wrote:
> > Hello,
> > 
> > zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri,  5 Feb 2021 17:36:47
> > +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.
> > > 
> > > Try to verify the flash ID to check whether the flash memory in octal
> > > DTR mode is correct. 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 order so that the ID can pass.
> > > 
> > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
> > > ---
> > >  drivers/mtd/spi-nor/macronix.c | 121 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 121 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> > > index 9203aba..7498978 100644
> > > --- a/drivers/mtd/spi-nor/macronix.c
> > > +++ b/drivers/mtd/spi-nor/macronix.c
> > > @@ -8,6 +8,16 @@
> > >  
> > >  #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 */
> > > +#define MXIC_MAX_DC			20		/* Maximum value of dummy cycles */
> > > +
> > >  static int
> > >  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
> > >  			    const struct sfdp_parameter_header *bfpt_header,
> > > @@ -33,6 +43,113 @@
> > >  	.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, 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 = MXIC_MAX_DC;  
> > 
> > I am still not convinced by this constant value.  
> 
> I think a constant value is fine. This dummy cycle value reflects how 
> many cycles the master clock would go through before the flash starts 
> emitting the data. If the master (aka the controller) is running at a 
> lower frequency then those cycles go through slower, but the flash still 
> waits for them to finish before emitting data. And since the master is 
> driving the clock and the flash is just "reading" it, both remain in 
> sync.
> 
> The dummy cycles need to be set for the worst case scenario [0]. The 
> flash usually needs a minimum amount of time before it is ready to emit 
> the data. So for example if the master is at 25 MHz, the clock period is 
> longer so 8 clock cycles [1] might be long enough to exceed that minimum 
> time. But when the master is running at 200 MHz, the clock period is 
> smaller so 8 cycles might not give the flash enough time to prepare. So 
> we need to to wait at least 20 cycles [1] before emitting data.
> 
> This is what my patches do for the Cypress S28 flash. I have tested it 
> on both 25 MHz and 166 MHz with 22 dummy cycles. It is not the most 
> efficient at 25 MHz since 5 dummy cycles is all that is needed for that 
> speed, but its the best we can do right now.
> 
> [0] Since SPI NOR has no way of knowing what speed the controller is 
> running at, assume the fastest speed the flash can run at.
> [1] Hypothetical example. Don't know the actual values for this flash.

Isn't this a precious loss of time?

If there is actually no way to retrieve the actual SPI speed I fully
get your point, but I doubt it. Or at least, I think this should be
optimized.

Perhaps Mark can shed some light on this?

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/2] spi: mxic: patch for octal DTR mode support

[zhengxunli]

Hi Mark,

I see that Pratyush patch "spi: spi-mem: add spi_mem_dtr_supports_op()"
has been accepted, can you help review this patch and make some 
suggestions?


"zhengxunli" <zhengxunli@mxic.com.tw> wrote on 2021/02/05 下午 05:36:48:

> "zhengxunli" <zhengxunli@mxic.com.tw> 
> 2021/02/05 下午 05:40
> 
> To
> 
> linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org, 
> miquel.raynal@bootlin.com, broonie@kernel.org, vigneshr@ti.com, 
> 
> cc
> 
> ycllin@mxic.com.tw, juliensu@mxic.com.tw, "zhengxunli" 
> <zhengxunli@mxic.com.tw>
> 
> Subject
> 
> [PATCH v2 2/2] spi: mxic: patch for octal DTR mode support
> 
> Driver patch for octal DTR mode support.
> 
> Owing to the spi_mem_default_supports_op() is not support dtr
> operation. Based on Pratyush patch "spi: spi-mem: add spi_mem_dtr
> _supports_op()" add spi_mem_dtr_supports_op() to support dtr and
> keep checking the buswidth and command bytes.
> 
> Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
> ---
>  drivers/spi/spi-mxic.c | 41 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
> index 96b4182..32e757a 100644
> --- a/drivers/spi/spi-mxic.c
> +++ b/drivers/spi/spi-mxic.c
> @@ -335,8 +335,10 @@ 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)
> +   bool all_false;
> +
> +   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 +348,13 @@ 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);
> +   all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr &&
> +          !op->data.dtr;
> +
> +   if (all_false)
> +      return spi_mem_default_supports_op(mem, op);
> +   else
> +      return spi_mem_dtr_supports_op(mem, op);
>  }
> 
>  static int mxic_spi_mem_exec_op(struct spi_mem *mem,
> @@ -355,14 +363,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 +383,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 +409,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 +585,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
> 

Thanks,
Zhengxun


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Miquel Raynal]

Hi Pratyush,

Pratyush Yadav <p.yadav@ti.com> wrote on Fri, 5 Feb 2021 19:04:04 +0530:

> On 05/02/21 10:47AM, Miquel Raynal wrote:
> > Hello,
> > 
> > zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri,  5 Feb 2021 17:36:47
> > +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.
> > > 
> > > Try to verify the flash ID to check whether the flash memory in octal
> > > DTR mode is correct. 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 order so that the ID can pass.
> > > 
> > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
> > > ---
> > >  drivers/mtd/spi-nor/macronix.c | 121 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 121 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> > > index 9203aba..7498978 100644
> > > --- a/drivers/mtd/spi-nor/macronix.c
> > > +++ b/drivers/mtd/spi-nor/macronix.c
> > > @@ -8,6 +8,16 @@
> > >  
> > >  #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 */
> > > +#define MXIC_MAX_DC			20		/* Maximum value of dummy cycles */
> > > +
> > >  static int
> > >  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
> > >  			    const struct sfdp_parameter_header *bfpt_header,
> > > @@ -33,6 +43,113 @@
> > >  	.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, 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 = MXIC_MAX_DC;  
> > 
> > I am still not convinced by this constant value.  
> 
> I think a constant value is fine. This dummy cycle value reflects how 
> many cycles the master clock would go through before the flash starts 
> emitting the data. If the master (aka the controller) is running at a 
> lower frequency then those cycles go through slower, but the flash still 
> waits for them to finish before emitting data. And since the master is 
> driving the clock and the flash is just "reading" it, both remain in 
> sync.
> 
> The dummy cycles need to be set for the worst case scenario [0]. The 
> flash usually needs a minimum amount of time before it is ready to emit 
> the data. So for example if the master is at 25 MHz, the clock period is 
> longer so 8 clock cycles [1] might be long enough to exceed that minimum 
> time. But when the master is running at 200 MHz, the clock period is 
> smaller so 8 cycles might not give the flash enough time to prepare. So 
> we need to to wait at least 20 cycles [1] before emitting data.
> 
> This is what my patches do for the Cypress S28 flash. I have tested it 
> on both 25 MHz and 166 MHz with 22 dummy cycles. It is not the most 
> efficient at 25 MHz since 5 dummy cycles is all that is needed for that 
> speed, but its the best we can do right now.
> 
> [0] Since SPI NOR has no way of knowing what speed the controller is 
> running at, assume the fastest speed the flash can run at.

Ok, I am not entirely clear about what is available/not available from
the SPI core.

If this is true then I guess we can't do better with the current code
base and this can be improved in the future if needed. So I'm fine with
the current implementation.

> [1] Hypothetical example. Don't know the actual values for this flash.
>  
> > The rest looks good to me.  
> 

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/2] spi: mxic: patch for octal DTR mode support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1259 bytes --]

On Fri, Feb 19, 2021 at 01:58:40PM +0800, zhengxunli@mxic.com.tw wrote:
> Hi Mark,
> 
> I see that Pratyush patch "spi: spi-mem: add spi_mem_dtr_supports_op()"
> has been accepted, can you help review this patch and make some 
> suggestions?

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 652 bytes --]

On Tue, Feb 23, 2021 at 02:13:44PM +0100, Miquel Raynal wrote:
> Pratyush Yadav <p.yadav@ti.com> wrote on Fri, 5 Feb 2021 19:04:04 +0530:

> > [0] Since SPI NOR has no way of knowing what speed the controller is 
> > running at, assume the fastest speed the flash can run at.

> Ok, I am not entirely clear about what is available/not available from
> the SPI core.

> If this is true then I guess we can't do better with the current code
> base and this can be improved in the future if needed. So I'm fine with
> the current implementation.

For normal SPI operations you can set the speed (really, top speed) on a
per transfer basis.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Pratyush Yadav]

On 23/02/21 01:36PM, Mark Brown wrote:
> On Tue, Feb 23, 2021 at 02:13:44PM +0100, Miquel Raynal wrote:
> > Pratyush Yadav <p.yadav@ti.com> wrote on Fri, 5 Feb 2021 19:04:04 +0530:
> 
> > > [0] Since SPI NOR has no way of knowing what speed the controller is 
> > > running at, assume the fastest speed the flash can run at.
> 
> > Ok, I am not entirely clear about what is available/not available from
> > the SPI core.
> 
> > If this is true then I guess we can't do better with the current code
> > base and this can be improved in the future if needed. So I'm fine with
> > the current implementation.
> 
> For normal SPI operations you can set the speed (really, top speed) on a
> per transfer basis.

To select the optimal number of dummy cycles we need to know what speed 
the controller is running at, not the other way around. The flash would 
always set the top speed to its maximum (say 200 MHz). But if the 
controller is only capable of running at 50 MHz, you will end up wasting 
dummy cycles. I don't see any API to communicate speed from controller 
to flash.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Geert Uytterhoeven]

Hi Pratyush,

On Tue, Feb 23, 2021 at 4:25 PM Pratyush Yadav <p.yadav@ti.com> wrote:
> On 23/02/21 01:36PM, Mark Brown wrote:
> > On Tue, Feb 23, 2021 at 02:13:44PM +0100, Miquel Raynal wrote:
> > > Pratyush Yadav <p.yadav@ti.com> wrote on Fri, 5 Feb 2021 19:04:04 +0530:
> >
> > > > [0] Since SPI NOR has no way of knowing what speed the controller is
> > > > running at, assume the fastest speed the flash can run at.
> >
> > > Ok, I am not entirely clear about what is available/not available from
> > > the SPI core.
> >
> > > If this is true then I guess we can't do better with the current code
> > > base and this can be improved in the future if needed. So I'm fine with
> > > the current implementation.
> >
> > For normal SPI operations you can set the speed (really, top speed) on a
> > per transfer basis.
>
> To select the optimal number of dummy cycles we need to know what speed
> the controller is running at, not the other way around. The flash would
> always set the top speed to its maximum (say 200 MHz). But if the
> controller is only capable of running at 50 MHz, you will end up wasting
> dummy cycles. I don't see any API to communicate speed from controller
> to flash.

spi_transfer.effective_speed_hz?

If the driver has filled this in (after the first transfer), you can optimize
dummy cycles before doing the next transfer.  Note that effective_speed_hz
might not always be the same, if e.g. the SPI controller shares its parent
clock with another component.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1092 bytes --]

On Tue, Feb 23, 2021 at 07:07:37PM +0100, Geert Uytterhoeven wrote:
> On Tue, Feb 23, 2021 at 4:25 PM Pratyush Yadav <p.yadav@ti.com> wrote:

> > To select the optimal number of dummy cycles we need to know what speed
> > the controller is running at, not the other way around. The flash would
> > always set the top speed to its maximum (say 200 MHz). But if the
> > controller is only capable of running at 50 MHz, you will end up wasting
> > dummy cycles. I don't see any API to communicate speed from controller
> > to flash.

> spi_transfer.effective_speed_hz?

> If the driver has filled this in (after the first transfer), you can optimize
> dummy cycles before doing the next transfer.  Note that effective_speed_hz
> might not always be the same, if e.g. the SPI controller shares its parent
> clock with another component.

Yes, that's what that's for, or just go with the speed set by the client
on the basis that it should be safe even if potentially wasteful.  You'd
need to fall back to that anyway in the cases where the controller
doesn't or can't report the effective speed.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] mtd: spi-nor: macronix: add support for Macronix octaflash

[Tudor.Ambarus]

On 3/29/21 9:35 AM, zhengxunli@mxic.com.tw wrote:
> Hi Tudor,

Hi,

> 
> We are discussing flash dummy cycle and SPI speed settings, but there
> are no new comments. Can you help comment on this series and provide
> some suggestions?

Sure, I'll probably be able to look at this next week.

Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: (subset) [PATCH v2 0/2] Add octal DTR support for Macronix flash

[Mark Brown]

On Fri, 5 Feb 2021 17:36:46 +0800, zhengxunli wrote:
> 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.
> 
> Changes in v2:
> - Define with a generic name to describe the maximum dummy cycles.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[2/2] spi: mxic: patch for octal DTR mode support
      commit: d05aaa66ba3ca3fdc2b5cd774ff218deb238b352

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 0/2] Add octal DTR support for Macronix flash

[Jaime Liao]

From: JaimeLiao <jaimeliao@mxic.com.tw>

This series add method for Macronix Octal DTR Eable/Disable
and add Macronix Octal flash support.

v2:
  Following exsting rules to re-create Macronix specify Octal DTR method.
  change signature to jaimeliao@mxic.com.tw
  Clear sector size information in flash INFO.

JaimeLiao (2):
  mtd: spi-nor: add Octal DTR support for Macronix flash
  mtd: spi-nor: add support for Macronix Octal flash

 drivers/mtd/spi-nor/macronix.c | 123 ++++++++++++++++++++++++++++++++-
 1 file changed, 121 insertions(+), 2 deletions(-)

-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/