diff mbox series

[v4,2/6] mtd: spi-nor: add Octal DTR support for Macronix flash

Message ID 20230908064304.27757-3-jaimeliao.tw@gmail.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series Add octal DTR support for Macronix flash | expand

Commit Message

liao jaime Sept. 8, 2023, 6:43 a.m. UTC 
From: JaimeLiao <jaimeliao@mxic.com.tw>

Create Macronix specify method for enable Octal DTR mode and
set 20 dummy cycles to allow running at the maximum supported
frequency for Macronix Octal flash.

Use number of dummy cycles which is parse by SFDP then convert
it to bit pattern and set in CR2 register.
Set CR2 register for enable octal dtr mode.

Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
Co-developed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/macronix.c | 97 ++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

Comments

Tudor Ambarus Sept. 20, 2023, 12:37 p.m. UTC | #1 
Hi, Jaime,

On 08.09.2023 09:43, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Create Macronix specify method for enable Octal DTR mode and
> set 20 dummy cycles to allow running at the maximum supported
> frequency for Macronix Octal flash.

You didn't answer my question. What happens when you specify 20 dummy
cycles, thus you configure the dummy cycles for the maximum flash speed,
but you program the flash to work on 1 MHz for example. Do you still
have reliable results?
> 
> Use number of dummy cycles which is parse by SFDP then convert
> it to bit pattern and set in CR2 register.

What we should do instead is to determine the flash speed at which the
flash is operated and then to set the correct number of dummy cycles
according to what the flash requires. There should be a table somewhere
in the datasheet that specify a required number of dummy cycles for a
particular frequency, isn't it? Yeah, see "9-3-1. Dummy Cycle and
Frequency Table (MHz)" of the mx66lm1g45g datasheet.

Cheers,
ta
Pratyush Yadav Oct. 5, 2023, 10:18 a.m. UTC | #2 
On Wed, Sep 20 2023, Tudor Ambarus wrote:

> Hi, Jaime,
>
> On 08.09.2023 09:43, Jaime Liao wrote:
>> From: JaimeLiao <jaimeliao@mxic.com.tw>
>> 
>> Create Macronix specify method for enable Octal DTR mode and
>> set 20 dummy cycles to allow running at the maximum supported
>> frequency for Macronix Octal flash.
>
> You didn't answer my question. What happens when you specify 20 dummy
> cycles, thus you configure the dummy cycles for the maximum flash speed,
> but you program the flash to work on 1 MHz for example. Do you still
> have reliable results?

I don't know about this particular flash, but in the past, I have tried
doing this with Spansion and Micron flashes, and they work fine on lower
frequencies even with the maximum dummy cycles set.

When you think about it, the only reason higher frequencies put a
restriction on minimum dummy cycles is because the flash actually has a
restriction on _time_. As time for each cycle gets shorter with higher
frequencies, you need more cycles to wait the same amount of time. Dummy
cycles are just a way to ensure you wait more than the minimum amount of
time needed to get the flash ready to transmit data.

So I see no reason for a flash to break because it waited _longer_ than
the minimum time.

>> 
>> Use number of dummy cycles which is parse by SFDP then convert
>> it to bit pattern and set in CR2 register.
>
> What we should do instead is to determine the flash speed at which the
> flash is operated and then to set the correct number of dummy cycles
> according to what the flash requires. There should be a table somewhere
> in the datasheet that specify a required number of dummy cycles for a
> particular frequency, isn't it? Yeah, see "9-3-1. Dummy Cycle and
> Frequency Table (MHz)" of the mx66lm1g45g datasheet.

Right, most flashes do give such a table, though I don't remember any
more if SFDP has something like this as well.

I remember thinking the same thing when I was adding support for Octal
DTR in SPI NOR. The problem is that SPI NOR currently has no way of
knowing what exact speed the flash will be operated at.

It can look at nor->spimem->spi->max_speed_hz (I am not sure it should
do this directly) which comes from the "spi-max-frequency" DT property,
but that is only the maximum. This can be a good starting point to
decide the maximum number of cycles you would need.

But that is only the maximum. The controller does not guarantee using
the maximum speed. It can use something slower as well. And currently
there is no way for the controller to report that speed back to SPI MEM
or SPI NOR. So if we want to waste the least amount of dummy cycles, we
would need to teach the controller drivers to report back the exact
speed it is going to driver the flash at.

I am not sure this might be worth the trouble though. We should first
quantify how much throughput/latency we stand to gain from doing this.
But I do think looking at "spi-max-frequency" is fairly simple and
should be a good enough start.
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 8ab47691dfbb..28c49c98503a 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,24 @@ 
 
 #include "core.h"
 
+#define SPINOR_OP_MXIC_RD_ANY_REG	0x71		/* Read volatile configuration register 2 */
+#define SPINOR_OP_MXIC_WR_ANY_REG	0x72		/* Write volatile configuration register 2 */
+#define SPINOR_REG_MXIC_CR2_MODE	0x00000000	/* CR2 address for setting octal DTR mode */
+#define SPINOR_REG_MXIC_CR2_DC		0x00000300	/* CR2 address for setting dummy cycles */
+#define SPINOR_REG_MXIC_OPI_DTR_EN	0x2		/* Enable Octal DTR */
+#define SPINOR_REG_MXIC_SPI_EN		0x0		/* Enable SPI */
+#define SPINOR_REG_MXIC_ADDR_BYTES	4		/* Fixed R/W volatile address bytes to 4 */
+/* Convert dummy cycles to bit pattern */
+#define SPINOR_REG_MXIC_DC(p) \
+		((20 - p)/2)
+
+/* Macronix SPI NOR flash operations. */
+#define MXIC_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MXIC_WR_ANY_REG, 0),		\
+		   SPI_MEM_OP_ADDR(naddr, addr, 0),			\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
+
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
 			    const struct sfdp_parameter_header *bfpt_header,
@@ -105,6 +123,84 @@  static const struct flash_info macronix_nor_parts[] = {
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
 };
 
+static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
+{
+	struct spi_mem_op op;
+	u8 *buf = nor->bouncebuf;
+	int ret;
+
+	/* Use dummy cycles which is parse by SFDP and convert to bit pattern. */
+	buf[0] = SPINOR_REG_MXIC_DC(nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].num_wait_states);
+	op = (struct spi_mem_op)
+		MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+				       SPINOR_REG_MXIC_CR2_DC, 1, buf);
+
+	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+	if (ret)
+		return ret;
+
+	/* Set the octal and DTR enable bits. */
+	buf[0] = SPINOR_REG_MXIC_OPI_DTR_EN;
+	op = (struct spi_mem_op)
+		MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+				       SPINOR_REG_MXIC_CR2_MODE, 1, buf);
+	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+	if (ret)
+		return ret;
+
+	/* Read flash ID to make sure the switch was successful. */
+	ret = spi_nor_read_id(nor, 4, 4, buf, SNOR_PROTO_8_8_8_DTR);
+	if (ret) {
+		dev_dbg(nor->dev, "error %d reading JEDEC ID after enabling 8D-8D-8D mode\n", ret);
+		return ret;
+	}
+
+	if (memcmp(buf, nor->info->id, nor->info->id_len))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int macronix_nor_octal_dtr_dis(struct spi_nor *nor)
+{
+	struct spi_mem_op op;
+	u8 *buf = nor->bouncebuf;
+	int ret;
+
+	/*
+	 * The register is 1-byte wide, but 1-byte transactions are not
+	 * allowed in 8D-8D-8D mode. Since there is no register at the
+	 * next location, just initialize the value to 0 and let the
+	 * transaction go on.
+	 */
+	buf[0] = SPINOR_REG_MXIC_SPI_EN;
+	buf[1] = 0x0;
+	op = (struct spi_mem_op)
+		MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+				       SPINOR_REG_MXIC_CR2_MODE, 2, buf);
+	ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
+	if (ret)
+		return ret;
+
+	/* Read flash ID to make sure the switch was successful. */
+	ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
+	if (ret) {
+		dev_dbg(nor->dev, "error %d reading JEDEC ID after disabling 8D-8D-8D mode\n", ret);
+		return ret;
+	}
+
+	if (memcmp(buf, nor->info->id, nor->info->id_len))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int macronix_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
+{
+	return enable ? macronix_nor_octal_dtr_en(nor) :
+			macronix_nor_octal_dtr_dis(nor);
+}
+
 static void macronix_nor_default_init(struct spi_nor *nor)
 {
 	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
@@ -114,6 +210,7 @@  static int macronix_nor_late_init(struct spi_nor *nor)
 {
 	if (!nor->params->set_4byte_addr_mode)
 		nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_en4b_ex4b;
+	nor->params->set_octal_dtr = macronix_nor_set_octal_dtr;
 
 	return 0;
 }