Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/4] Add DDR (Dual Data-Rate) support for DW SPI controller
@ 2026-10-22 10:26 Changhuang Liang
  2026-10-22 10:26 ` [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode Changhuang Liang
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Changhuang Liang @ 2026-10-22 10:26 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi, linux-kernel, linux-mtd, Changhuang Liang

This series implements DDR support for the DW SPI controller and also
implements 1-4-4-DTR read support for the MX66L1G45G.
This series has been tested on the JHB100 SoC.

# cat /sys/kernel/debug/spi-nor/spi0.1/capabilities
Supported read modes by the flash
[...]
 1S-4D-4D
  opcode        0xee
  mode cycles   0
  dummy cycles  6

Changhuang Liang (4):
  spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in
    spi_nor_spimem_setup_op()
  mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes
  mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G

 drivers/mtd/spi-nor/core.c     | 21 +++++++++-----
 drivers/mtd/spi-nor/debugfs.c  |  8 +++---
 drivers/mtd/spi-nor/macronix.c | 28 +++++++++++++++++-
 drivers/spi/spi-dw-core.c      | 52 +++++++++++++++++++++++++++++-----
 drivers/spi/spi-dw.h           |  6 ++++
 5 files changed, 96 insertions(+), 19 deletions(-)

--
2.25.1

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  2026-10-22 10:26 [PATCH v1 0/4] Add DDR (Dual Data-Rate) support for DW SPI controller Changhuang Liang
@ 2026-10-22 10:26 ` Changhuang Liang
  2026-10-22 10:41   ` Miquel Raynal
  2026-10-22 10:48   ` sashiko-bot
  2026-10-22 10:26 ` [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op() Changhuang Liang
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: Changhuang Liang @ 2026-10-22 10:26 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi, linux-kernel, linux-mtd, Changhuang Liang

Add support for DDR (Dual Data-Rate) transfers in the enhanced SPI
mode of the DW SPI controller.

The enhanced SPI control register (SPI_CTRLR0) has dedicated fields to
enable DDR mode for the instruction phase (INST_DDR_EN) and for the
address/dummy/data phases (SPI_DDR_EN). Populate these from the
corresponding dtr flags of the spi_mem_op.

For DDR transfers, the dummy phase consumes two bits per clock per
line, so the computed wait cycles must be halved. Factor the wait cycle
calculation into a helper, dw_spi_enh_wait_cycles(), so that both the
supports check and the exec path use the same value.

When SPI DDR is enabled, program the DDR_DRIVE_EDGE register. The
recommended TDE value is clk_div / 4, clamped to clk_div / 2 - 1 and to
the field width.

Finally, advertise DTR support to the SPI memory layer through a
dedicated dw_spi_enh_mem_caps, selected when the controller has
DW_SPI_CAP_EMODE, so that spi-mem may issue DDR operations.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 52 +++++++++++++++++++++++++++++++++------
 drivers/spi/spi-dw.h      |  6 +++++
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 47434fa78e4b..6ff142c8a6bd 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -46,6 +46,8 @@ struct dw_spi_enh_cfg {
 	u8 inst_l;
 	u8 addr_l;
 	u8 trans_t;
+	bool inst_ddr;
+	bool spi_ddr;
 };

 #ifdef CONFIG_DEBUG_FS
@@ -418,6 +420,7 @@ static void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 	u32 cr0 = chip->cr0;
 	u32 speed_hz;
 	u16 clk_div;
+	u32 tde = 0;

 	/* CTRLR0[ 4/3: 0] or CTRLR0[ 20: 16] Data Frame Size */
 	cr0 |= (cfg->dfs - 1) << dws->dfs_offset;
@@ -468,11 +471,26 @@ static void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,

 	if (enh_cfg) {
 		cr0 = DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN;
+		if (enh_cfg->inst_ddr)
+			cr0 |= DW_SPI_ENH_CTRLR0_INST_DDR_EN;
+		if (enh_cfg->spi_ddr)
+			cr0 |= DW_SPI_ENH_CTRLR0_SPI_DDR_EN;
 		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK, enh_cfg->wait_c);
 		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_INST_L_MASK, enh_cfg->inst_l);
 		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_ADDR_L_MASK, enh_cfg->addr_l);
 		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_TRANS_TYPE_MASK, enh_cfg->trans_t);
 		dw_writel(dws, DW_SPI_SPI_CTRLR0, cr0);
+
+		/*
+		 * The recommended value of TDE is clk_div / 4, and it must not exceed
+		 * SCKDV / 2 - 1
+		 */
+		if (enh_cfg->spi_ddr)
+			tde = min3(clk_div / 4U, clk_div / 2U - 1,
+				   FIELD_MAX(DW_SPI_DDR_DRIVE_EDGE_TDE_MASK));
+
+		dw_writel(dws, DW_SPI_DDR_DRIVE_EDGE,
+			  FIELD_PREP(DW_SPI_DDR_DRIVE_EDGE_TDE_MASK, tde));
 	}
 }

@@ -656,6 +674,21 @@ static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	return 0;
 }

+static u32 dw_spi_enh_wait_cycles(const struct spi_mem_op *op)
+{
+	u32 wait_c;
+
+	if (!op->dummy.nbytes || !op->dummy.buswidth)
+		return 0;
+
+	wait_c = op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth;
+	/* DTR transfers two bits per clock on each line. */
+	if (op->dummy.dtr)
+		wait_c >>= 1;
+
+	return wait_c;
+}
+
 static bool dw_spi_supports_enh_mem_op(struct spi_mem *mem,
 				       const struct spi_mem_op *op)
 {
@@ -678,10 +711,7 @@ static bool dw_spi_supports_enh_mem_op(struct spi_mem *mem,
 	if (op->dummy.nbytes != 0 && op->data.dir == SPI_MEM_DATA_OUT)
 		return false;

-	/* WAIT_CYCLES is a 5-bit field in SPI_CTRLR0 */
-	if (op->dummy.nbytes != 0 &&
-	    op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth >
-	    FIELD_MAX(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK))
+	if (dw_spi_enh_wait_cycles(op) > FIELD_MAX(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK))
 		return false;

 	if ((dws->quirk_flags & DW_SPI_QUIRK_JHB100) && op->addr.nbytes &&
@@ -1074,8 +1104,10 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
 	else
 		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L0;

-	if (op->dummy.buswidth)
-		enh_cfg.wait_c = op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth;
+	enh_cfg.wait_c = dw_spi_enh_wait_cycles(op);
+
+	enh_cfg.inst_ddr = op->cmd.dtr;
+	enh_cfg.spi_ddr = op->addr.dtr || op->dummy.dtr || op->data.dtr;

 	dw_spi_enable_chip(dws, 0);

@@ -1383,6 +1415,11 @@ static const struct spi_controller_mem_caps dw_spi_mem_caps = {
 	.per_op_freq = true,
 };

+static const struct spi_controller_mem_caps dw_spi_enh_mem_caps = {
+	.per_op_freq = true,
+	.dtr = true,
+};
+
 int dw_spi_add_controller(struct device *dev, struct dw_spi *dws)
 {
 	struct spi_controller *ctlr;
@@ -1439,7 +1476,8 @@ int dw_spi_add_controller(struct device *dev, struct dw_spi *dws)
 			ctlr->set_cs = dw_spi_set_cs;
 		if (dws->mem_ops.exec_op) {
 			ctlr->mem_ops = &dws->mem_ops;
-			ctlr->mem_caps = &dw_spi_mem_caps;
+			ctlr->mem_caps = (dws->caps & DW_SPI_CAP_EMODE) ?
+					 &dw_spi_enh_mem_caps : &dw_spi_mem_caps;
 		}
 		ctlr->max_speed_hz = dws->max_freq;
 		ctlr->flags = SPI_CONTROLLER_GPIO_SS;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index c16225c5c887..67b4e0098ac3 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -66,6 +66,7 @@
 #define DW_SPI_RX_SAMPLE_DLY		0xf0
 #define DW_SPI_SPI_CTRLR0		0xf4
 #define DW_SPI_CS_OVERRIDE		0xf4
+#define DW_SPI_DDR_DRIVE_EDGE		0xf8

 /* Register offsets (StarFive JHB100 DWC SSI IP-cores) */
 #define DW_SPI_JHB100_INST		0x1000
@@ -139,6 +140,8 @@

 /* Bit fields in SPI_CTRLR0 */
 #define DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN	BIT(30)
+#define DW_SPI_ENH_CTRLR0_INST_DDR_EN		BIT(17)
+#define DW_SPI_ENH_CTRLR0_SPI_DDR_EN		BIT(16)
 #define DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK	GENMASK(15, 11)
 #define DW_SPI_ENH_CTRLR0_INST_L_MASK		GENMASK(9, 8)
 #define DW_SPI_ENH_CTRLR0_INST_L_INST_L0	0x0
@@ -150,6 +153,9 @@
 #define DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT2	0x2
 #define DW_SPI_ENH_CTRLR0_TRANS_TYPE_MASK	GENMASK(1, 0)

+/* Bit fields in DDR_DRIVE_EDGE */
+#define DW_SPI_DDR_DRIVE_EDGE_TDE_MASK		GENMASK(7, 0)
+
 /* Mem/DMA operations helpers */
 #define DW_SPI_WAIT_RETRIES			5
 #define DW_SPI_BUF_SIZE \
--
2.25.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op()
  2026-10-22 10:26 [PATCH v1 0/4] Add DDR (Dual Data-Rate) support for DW SPI controller Changhuang Liang
  2026-10-22 10:26 ` [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode Changhuang Liang
@ 2026-10-22 10:26 ` Changhuang Liang
  2026-10-22 10:45   ` Miquel Raynal
  2026-10-22 10:26 ` [PATCH v1 3/4] mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes Changhuang Liang
  2026-10-22 10:26 ` [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G Changhuang Liang
  3 siblings, 1 reply; 17+ messages in thread
From: Changhuang Liang @ 2026-10-22 10:26 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi, linux-kernel, linux-mtd, Changhuang Liang

Currently spi_nor_spimem_setup_op() sets all four phases (cmd, addr,
dummy, data) to DTR whenever spi_nor_protocol_is_dtr(proto) is true.
This is too coarse: the 1S-xD-xD DTR protocols (1_1_1_DTR, 1_2_2_DTR,
1_4_4_DTR, 1_8_8_DTR) send the command phase in STR on a single line,
and only the address, dummy and data phases in DTR. SPIMEM supports
such mixed DTR modes.

Split the handling:

  - For SNOR_PROTO_8_8_8_DTR, keep the existing behavior: set all
    phases to DTR and use a 2-byte opcode (opcode + extension).

  - For the other DTR protocols, set the command phase to STR and only
    the address, dummy and data phases to DTR. Double op->dummy.nbytes
    since DTR transfers 2 bytes per clock cycle, matching how the dummy
    cycle count is specified for these protocols.

No in-tree flash currently declares SNOR_HWCAPS_READ_1_x_x_DTR, so
this is not a bug fix but preparation for enabling these protocols.
A follow-up patch will enable 1S-4D-4D for the MX66L1G45G. The
8D-8D-8D path is unchanged.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/mtd/spi-nor/core.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 90524c58602e..43e506e728ab 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -97,13 +97,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
 	if (op->data.nbytes)
 		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
 
-	if (spi_nor_protocol_is_dtr(proto)) {
-		/*
-		 * SPIMEM supports mixed DTR modes, but right now we can only
-		 * have all phases either DTR or STR. IOW, SPIMEM can have
-		 * something like 4S-4D-4D, but SPI NOR can't. So, set all 4
-		 * phases to either DTR or STR.
-		 */
+	if (proto == SNOR_PROTO_8_8_8_DTR) {
 		op->cmd.dtr = true;
 		op->addr.dtr = true;
 		op->dummy.dtr = true;
@@ -115,6 +109,19 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
 		ext = spi_nor_get_cmd_ext(nor, op);
 		op->cmd.opcode = (op->cmd.opcode << 8) | ext;
 		op->cmd.nbytes = 2;
+	} else if (spi_nor_protocol_is_dtr(proto)) {
+		/*
+		 * 1S-xD-xD protocols: the opcode is sent in STR on a single
+		 * line, only the address, dummy and data phases are DTR. The
+		 * opcode stays 1 byte (no command extension).
+		 */
+		op->cmd.dtr = false;
+		op->addr.dtr = true;
+		op->dummy.dtr = true;
+		op->data.dtr = true;
+
+		/* 2 bytes per clock cycle in DTR mode. */
+		op->dummy.nbytes *= 2;
 	}
 
 	if (proto == SNOR_PROTO_8_8_8_DTR && params->flags & SNOR_F_SWAP16)
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 3/4] mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes
  2026-10-22 10:26 [PATCH v1 0/4] Add DDR (Dual Data-Rate) support for DW SPI controller Changhuang Liang
  2026-10-22 10:26 ` [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode Changhuang Liang
  2026-10-22 10:26 ` [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op() Changhuang Liang
@ 2026-10-22 10:26 ` Changhuang Liang
  2026-10-22 10:46   ` Miquel Raynal
  2026-10-22 10:26 ` [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G Changhuang Liang
  3 siblings, 1 reply; 17+ messages in thread
From: Changhuang Liang @ 2026-10-22 10:26 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi, linux-kernel, linux-mtd, Changhuang Liang

The debugfs protocol names for the mixed DTR modes (1-1-1, 1-2-2,
1-4-4, 1-8-8) currently print the command phase as "1D", i.e. dual
transfer on a single line.  That does not match the protocol
definition: these modes issue the opcode on a single line in single
data rate (S), and only the address, dummy and data phases use DTR
(D).  The "1D" prefix is therefore wrong.

Rename them to "1S-xD-xD" so the names reflect that the first phase
is single-data-rate on one line, consistent with the "8D-8D-8D"
naming for SNOR_PROTO_8_8_8_DTR.

This only changes the strings printed by debugfs; no functional
behavior is affected.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/mtd/spi-nor/debugfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 3f8d586a7622..f9ec5697aab0 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -45,10 +45,10 @@ static const char *spi_nor_protocol_name(enum spi_nor_protocol proto)
 	case SNOR_PROTO_2_2_2:     return "2S-2S-2S";
 	case SNOR_PROTO_4_4_4:     return "4S-4S-4S";
 	case SNOR_PROTO_8_8_8:     return "8S-8S-8S";
-	case SNOR_PROTO_1_1_1_DTR: return "1D-1D-1D";
-	case SNOR_PROTO_1_2_2_DTR: return "1D-2D-2D";
-	case SNOR_PROTO_1_4_4_DTR: return "1D-4D-4D";
-	case SNOR_PROTO_1_8_8_DTR: return "1D-8D-8D";
+	case SNOR_PROTO_1_1_1_DTR: return "1S-1D-1D";
+	case SNOR_PROTO_1_2_2_DTR: return "1S-2D-2D";
+	case SNOR_PROTO_1_4_4_DTR: return "1S-4D-4D";
+	case SNOR_PROTO_1_8_8_DTR: return "1S-8D-8D";
 	case SNOR_PROTO_8_8_8_DTR: return "8D-8D-8D";
 	}
 
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G
  2026-10-22 10:26 [PATCH v1 0/4] Add DDR (Dual Data-Rate) support for DW SPI controller Changhuang Liang
                   ` (2 preceding siblings ...)
  2026-10-22 10:26 ` [PATCH v1 3/4] mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes Changhuang Liang
@ 2026-10-22 10:26 ` Changhuang Liang
  2026-10-22 10:47   ` Miquel Raynal
  2026-10-22 12:23   ` Michael Walle
  3 siblings, 2 replies; 17+ messages in thread
From: Changhuang Liang @ 2026-10-22 10:26 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi, linux-kernel, linux-mtd, Changhuang Liang

The MX66L1G45G supports the 1-4-4 DTR read mode. This flash uses 6
dummy cycles by default in 1-4-4 DTR mode, but the kernel's SFDP
parser does not derive any SNOR_HWCAPS_READ_1_4_4_DTR capability from
the BFPT table, and 4BAIT only filters already-known capabilities.
The exact number of dummy cycles is also not directly given in the
SFDP tables.

Therefore, implement late_init to initialize support for this opcode,
configuring the read settings with the default 6 dummy cycles. When a
4BAIT table is present, SFDP parsing has already converted all read
opcodes to their 4-byte variants and spi_nor_set_4byte_opcodes() is
skipped later on, so select the 4-byte opcode in that case.

The part also supports the 4-byte opcode set but its SFDP does not
expose a 4BAIT table, so set SPI_NOR_4B_OPCODES like MX66L51235F
(0xc2 0x20 0x1a) to let spi_nor_set_4byte_opcodes() pick the 4-byte
variants, including 0xEE for 1-4-4 DTR.

Add a dedicated fixup for the part (SNOR_ID 0xc2, 0x20, 0x1b) that
keeps the existing QPP 4-byte post-SFDP fixup and additionally
advertises the 1-4-4 DTR read capability in late_init.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/mtd/spi-nor/macronix.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index b3fdb4b2fa1c..3401275f096b 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -103,6 +103,26 @@ mx25l12805d_4pp3b_post_sfdp_fixups(struct spi_nor *nor)
 	return 0;
 }
 
+static int mx66l1g45g_late_init_fixups(struct spi_nor *nor)
+{
+	struct spi_nor_flash_parameter *params = nor->params;
+	u8 opcode = SPINOR_OP_READ_1_4_4_DTR;
+
+	/*
+	 * When a 4BAIT table is present, SFDP parsing already converted all
+	 * read opcodes to their 4-byte variants and spi_nor_set_4byte_opcodes()
+	 * is skipped later on, so select the 4-byte opcode here.
+	 */
+	if (params->flags & SNOR_F_HAS_4BAIT)
+		opcode = SPINOR_OP_READ_1_4_4_DTR_4B;
+
+	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_4_4_DTR;
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_4_4_DTR],
+				  0, 6, opcode, SNOR_PROTO_1_4_4_DTR);
+
+	return 0;
+}
+
 static const struct spi_nor_fixups mx25l25635_fixups = {
 	.post_bfpt = mx25l25635_post_bfpt_fixups,
 	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
@@ -120,6 +140,11 @@ static const struct spi_nor_fixups mx25l12805d_4pp3b_fixups = {
 	.post_sfdp = mx25l12805d_4pp3b_post_sfdp_fixups,
 };
 
+static const struct spi_nor_fixups mx66l1g45g_fixups = {
+	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
+	.late_init = mx66l1g45g_late_init_fixups,
+};
+
 static const struct flash_info macronix_nor_parts[] = {
 	{
 		.id = SNOR_ID(0xc2, 0x20, 0x10),
@@ -355,7 +380,8 @@ static const struct spi_nor_fixup macronix_fixups[] = {
 	{ .id = SNOR_ID(0xc2, 0x20, 0x19), .fixups = &mx25l25635_fixups },
 	{ .id = SNOR_ID(0xc2, 0x20, 0x1a), .fixups = &macronix_qpp4b_fixups,
 	  .fixup_flags = SPI_NOR_4B_OPCODES },
-	{ .id = SNOR_ID(0xc2, 0x20, 0x1b), .fixups = &macronix_qpp4b_fixups },
+	{ .id = SNOR_ID(0xc2, 0x20, 0x1b), .fixups = &mx66l1g45g_fixups,
+	  .fixup_flags = SPI_NOR_4B_OPCODES },
 	{ .id = SNOR_ID(0xc2, 0x20, 0x1c), .fixups = &macronix_qpp4b_fixups },
 	{ .id = SNOR_ID(0xc2, 0x25, 0x3a), .fixups = &macronix_qpp4b_fixups },
 	{ .id = SNOR_ID(0xc2, 0x25, 0x3b), .fixups = &macronix_qpp4b_fixups },
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  2026-10-22 10:26 ` [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode Changhuang Liang
@ 2026-10-22 10:41   ` Miquel Raynal
  2026-10-23  9:42     ` Changhuang Liang
  2026-10-22 10:48   ` sashiko-bot
  1 sibling, 1 reply; 17+ messages in thread
From: Miquel Raynal @ 2026-10-22 10:41 UTC (permalink / raw)
  To: Changhuang Liang
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra, linux-spi, linux-kernel,
	linux-mtd

On 22/10/2026 at 03:26:11 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:

> Add support for DDR (Dual Data-Rate) transfers in the enhanced SPI
> mode of the DW SPI controller.
>
> The enhanced SPI control register (SPI_CTRLR0) has dedicated fields to
> enable DDR mode for the instruction phase (INST_DDR_EN) and for the
> address/dummy/data phases (SPI_DDR_EN). Populate these from the
> corresponding dtr flags of the spi_mem_op.
>
> For DDR transfers, the dummy phase consumes two bits per clock per
> line, so the computed wait cycles must be halved. Factor the wait cycle
> calculation into a helper, dw_spi_enh_wait_cycles(), so that both the
> supports check and the exec path use the same value.
>
> When SPI DDR is enabled, program the DDR_DRIVE_EDGE register. The
> recommended TDE value is clk_div / 4, clamped to clk_div / 2 - 1 and to
> the field width.
>
> Finally, advertise DTR support to the SPI memory layer through a
> dedicated dw_spi_enh_mem_caps, selected when the controller has
> DW_SPI_CAP_EMODE, so that spi-mem may issue DDR operations.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/spi/spi-dw-core.c | 52 +++++++++++++++++++++++++++++++++------
>  drivers/spi/spi-dw.h      |  6 +++++
>  2 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 47434fa78e4b..6ff142c8a6bd 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -46,6 +46,8 @@ struct dw_spi_enh_cfg {
>  	u8 inst_l;
>  	u8 addr_l;
>  	u8 trans_t;
> +	bool inst_ddr;
> +	bool spi_ddr;

If I understand this correctly, spi_ddr is quite a bad name because it
means address and dummy and data ddr (but not instr). Can you try to
come up with a better name for this variable?

Also, if my above understanding is correct, you must provide a
supports_op() callback which would refuse any xS-xS-xD but allow
xD-xD-xD and xS-xD-D modes.

Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op()
  2026-10-22 10:26 ` [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op() Changhuang Liang
@ 2026-10-22 10:45   ` Miquel Raynal
  2026-10-22 12:19     ` Michael Walle
  0 siblings, 1 reply; 17+ messages in thread
From: Miquel Raynal @ 2026-10-22 10:45 UTC (permalink / raw)
  To: Changhuang Liang
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra, linux-spi, linux-kernel,
	linux-mtd

On 22/10/2026 at 03:26:12 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:

> Currently spi_nor_spimem_setup_op() sets all four phases (cmd, addr,
> dummy, data) to DTR whenever spi_nor_protocol_is_dtr(proto) is true.
> This is too coarse: the 1S-xD-xD DTR protocols (1_1_1_DTR, 1_2_2_DTR,
> 1_4_4_DTR, 1_8_8_DTR) send the command phase in STR on a single line,
> and only the address, dummy and data phases in DTR. SPIMEM supports
> such mixed DTR modes.
>
> Split the handling:
>
>   - For SNOR_PROTO_8_8_8_DTR, keep the existing behavior: set all
>     phases to DTR and use a 2-byte opcode (opcode + extension).
>
>   - For the other DTR protocols, set the command phase to STR and only
>     the address, dummy and data phases to DTR. Double op->dummy.nbytes
>     since DTR transfers 2 bytes per clock cycle, matching how the dummy
>     cycle count is specified for these protocols.
>
> No in-tree flash currently declares SNOR_HWCAPS_READ_1_x_x_DTR, so
> this is not a bug fix but preparation for enabling these protocols.
> A follow-up patch will enable 1S-4D-4D for the MX66L1G45G. The
> 8D-8D-8D path is unchanged.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/mtd/spi-nor/core.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 90524c58602e..43e506e728ab 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -97,13 +97,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>  	if (op->data.nbytes)
>  		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
>  
> -	if (spi_nor_protocol_is_dtr(proto)) {
> -		/*
> -		 * SPIMEM supports mixed DTR modes, but right now we can only
> -		 * have all phases either DTR or STR. IOW, SPIMEM can have
> -		 * something like 4S-4D-4D, but SPI NOR can't. So, set all 4
> -		 * phases to either DTR or STR.
> -		 */
> +	if (proto == SNOR_PROTO_8_8_8_DTR) {
>  		op->cmd.dtr = true;
>  		op->addr.dtr = true;
>  		op->dummy.dtr = true;
> @@ -115,6 +109,19 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>  		ext = spi_nor_get_cmd_ext(nor, op);
>  		op->cmd.opcode = (op->cmd.opcode << 8) | ext;
>  		op->cmd.nbytes = 2;
> +	} else if (spi_nor_protocol_is_dtr(proto)) {
> +		/*
> +		 * 1S-xD-xD protocols: the opcode is sent in STR on a single
> +		 * line, only the address, dummy and data phases are DTR. The
> +		 * opcode stays 1 byte (no command extension).
> +		 */
> +		op->cmd.dtr = false;
> +		op->addr.dtr = true;
> +		op->dummy.dtr = true;
> +		op->data.dtr = true;

Still to coarse IMHO.

What about xS-xS-xD?

As mentioned in the spi controller patch you must make sure these are
refused if they are not supported by your controller because SPI NAND at
least allow them (and I believe SPI NOR might as well)?.

> +
> +		/* 2 bytes per clock cycle in DTR mode. */
> +		op->dummy.nbytes *= 2;
>  	}
>  
>  	if (proto == SNOR_PROTO_8_8_8_DTR && params->flags & SNOR_F_SWAP16)

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/4] mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes
  2026-10-22 10:26 ` [PATCH v1 3/4] mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes Changhuang Liang
@ 2026-10-22 10:46   ` Miquel Raynal
  0 siblings, 0 replies; 17+ messages in thread
From: Miquel Raynal @ 2026-10-22 10:46 UTC (permalink / raw)
  To: Changhuang Liang
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra, linux-spi, linux-kernel,
	linux-mtd

On 22/10/2026 at 03:26:13 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:

> The debugfs protocol names for the mixed DTR modes (1-1-1, 1-2-2,
> 1-4-4, 1-8-8) currently print the command phase as "1D", i.e. dual
> transfer on a single line.  That does not match the protocol
> definition: these modes issue the opcode on a single line in single
> data rate (S), and only the address, dummy and data phases use DTR
> (D).  The "1D" prefix is therefore wrong.
>
> Rename them to "1S-xD-xD" so the names reflect that the first phase
> is single-data-rate on one line, consistent with the "8D-8D-8D"
> naming for SNOR_PROTO_8_8_8_DTR.
>
> This only changes the strings printed by debugfs; no functional
> behavior is affected.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---

That's likely correct.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G
  2026-10-22 10:26 ` [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G Changhuang Liang
@ 2026-10-22 10:47   ` Miquel Raynal
  2026-10-22 12:23   ` Michael Walle
  1 sibling, 0 replies; 17+ messages in thread
From: Miquel Raynal @ 2026-10-22 10:47 UTC (permalink / raw)
  To: Changhuang Liang
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra, linux-spi, linux-kernel,
	linux-mtd

On 22/10/2026 at 03:26:14 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:

> The MX66L1G45G supports the 1-4-4 DTR read mode. This flash uses 6
> dummy cycles by default in 1-4-4 DTR mode, but the kernel's SFDP
> parser does not derive any SNOR_HWCAPS_READ_1_4_4_DTR capability from
> the BFPT table, and 4BAIT only filters already-known capabilities.
> The exact number of dummy cycles is also not directly given in the
> SFDP tables.
>
> Therefore, implement late_init to initialize support for this opcode,
> configuring the read settings with the default 6 dummy cycles. When a
> 4BAIT table is present, SFDP parsing has already converted all read
> opcodes to their 4-byte variants and spi_nor_set_4byte_opcodes() is
> skipped later on, so select the 4-byte opcode in that case.
>
> The part also supports the 4-byte opcode set but its SFDP does not
> expose a 4BAIT table, so set SPI_NOR_4B_OPCODES like MX66L51235F
> (0xc2 0x20 0x1a) to let spi_nor_set_4byte_opcodes() pick the 4-byte
> variants, including 0xEE for 1-4-4 DTR.
>
> Add a dedicated fixup for the part (SNOR_ID 0xc2, 0x20, 0x1b) that
> keeps the existing QPP 4-byte post-SFDP fixup and additionally
> advertises the 1-4-4 DTR read capability in late_init.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Maybe in your v2 you can add (as hinted in the documentation) traces of
a test run with the SFDP tables, proof that it works in this mode, etc.

Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  2026-10-22 10:26 ` [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode Changhuang Liang
  2026-10-22 10:41   ` Miquel Raynal
@ 2026-10-22 10:48   ` sashiko-bot
  1 sibling, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-10-22 10:48 UTC (permalink / raw)
  To: Changhuang Liang, Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Mark Brown, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: linux-mtd

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] spi: dw: unconditional DTR capability advertisement in dw_spi_add_controller()
- [High] spi: dw: missing DTR phase validation in dw_spi_supports_enh_mem_op()
- [High] spi: dw: broken no-data 8D-0-0 and 8D-8D-0 ops in dw_spi_supports_enh_mem_op()
- [Medium] spi: dw: odd dummy half-cycle truncation in dw_spi_enh_wait_cycles()
--

commit b43a1a39eac428f70be5daaca07fad4710978bc3
Author: Changhuang Liang <changhuang.liang@starfivetech.com>

spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode

This commit adds Dual Data-Rate (DDR) transfer support for the enhanced SPI
mode of the DesignWare SPI controller by configuring the instruction and SPI
DDR enable bits in SPI_CTRLR0, halving the dummy wait cycles for DTR phases,
and programming the DDR_DRIVE_EDGE register. It also advertises DTR capability
to spi-mem when DW_SPI_CAP_EMODE is present.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 47434fa78e4b6..6ff142c8a6bd9 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c

[ ... ]

> @@ -468,11 +471,26 @@ static void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>  
>  	if (enh_cfg) {
>  		cr0 = DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN;
> +		if (enh_cfg->inst_ddr)
> +			cr0 |= DW_SPI_ENH_CTRLR0_INST_DDR_EN;
> +		if (enh_cfg->spi_ddr)
> +			cr0 |= DW_SPI_ENH_CTRLR0_SPI_DDR_EN;
>  		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK, enh_cfg->wait_c);
>  		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_INST_L_MASK, enh_cfg->inst_l);
>  		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_ADDR_L_MASK, enh_cfg->addr_l);
>  		cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_TRANS_TYPE_MASK, enh_cfg->trans_t);
>  		dw_writel(dws, DW_SPI_SPI_CTRLR0, cr0);
> +
> +		/*
> +		 * The recommended value of TDE is clk_div / 4, and it must not exceed
> +		 * SCKDV / 2 - 1
> +		 */
> +		if (enh_cfg->spi_ddr)
> +			tde = min3(clk_div / 4U, clk_div / 2U - 1,
> +				   FIELD_MAX(DW_SPI_DDR_DRIVE_EDGE_TDE_MASK));
> +
> +		dw_writel(dws, DW_SPI_DDR_DRIVE_EDGE,
> +			  FIELD_PREP(DW_SPI_DDR_DRIVE_EDGE_TDE_MASK, tde));
>  	}
>  }
>  
> @@ -656,6 +674,21 @@ static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>  	return 0;
>  }
>  
> +static u32 dw_spi_enh_wait_cycles(const struct spi_mem_op *op)
> +{
> +	u32 wait_c;
> +
> +	if (!op->dummy.nbytes || !op->dummy.buswidth)
> +		return 0;
> +
> +	wait_c = op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth;
> +	/* DTR transfers two bits per clock on each line. */
> +	if (op->dummy.dtr)
> +		wait_c >>= 1;
> +
> +	return wait_c;
> +}

[Severity: Medium]
Can wait_c >>= 1 in dw_spi_enh_wait_cycles() silently truncate odd dummy
half-cycles instead of rejecting unsupported operations in
dw_spi_supports_enh_mem_op()?

The WAIT_CYCLES field in DW_SPI_SPI_CTRLR0 counts whole clock cycles, where
each DTR clock cycle transfers two bits per line. When op->dummy.dtr is true
and op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth is odd (for
example, a 1S-8D-8D operation with op->dummy.buswidth == 8 and an odd
op->dummy.nbytes, which spi_mem_default_supports_op() does not reject
because op->cmd.dtr is false), shifting wait_c right by 1 discards the
remainder half-cycle (turning 1 dummy byte on 8 lines into 0 wait cycles).

Should dw_spi_supports_enh_mem_op() verify that the dummy phase represents
an exact integer number of clock cycles before accepting the operation?

> +
>  static bool dw_spi_supports_enh_mem_op(struct spi_mem *mem,
>  				       const struct spi_mem_op *op)
>  {
> @@ -678,10 +711,7 @@ static bool dw_spi_supports_enh_mem_op(struct spi_mem *mem,
>  	if (op->dummy.nbytes != 0 && op->data.dir == SPI_MEM_DATA_OUT)
>  		return false;
>  
> -	/* WAIT_CYCLES is a 5-bit field in SPI_CTRLR0 */
> -	if (op->dummy.nbytes != 0 &&
> -	    op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth >
> -	    FIELD_MAX(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK))
> +	if (dw_spi_enh_wait_cycles(op) > FIELD_MAX(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK))
>  		return false;
>  
>  	if ((dws->quirk_flags & DW_SPI_QUIRK_JHB100) && op->addr.nbytes &&

[Severity: High]
Does dw_spi_supports_enh_mem_op() accept DTR phase combinations that
dw_spi_exec_enh_mem_op() and the DW SSI hardware cannot execute?

When ctlr->mem_caps->dtr is set, spi_mem_supports_op() calls
dw_spi_supports_enh_mem_op(), which delegates to
spi_mem_default_supports_op() without checking DTR buswidths, transfer type
constraints, or instruction lengths:

drivers/spi/spi-dw-core.c:dw_spi_exec_enh_mem_op() {
	...
	switch (op->data.buswidth) {
	case 0:
	case 1:
		cfg.spi_frf = DW_SPI_CTRLR0_SPI_FRF_STD_SPI;
		break;
	...
	if (op->data.buswidth == op->addr.buswidth &&
	    op->data.buswidth == op->cmd.buswidth)
		enh_cfg.trans_t = DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT2;
	else if (op->data.buswidth == op->addr.buswidth)
		enh_cfg.trans_t = DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT1;
	else
		enh_cfg.trans_t = DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT0;
	...
	enh_cfg.inst_ddr = op->cmd.dtr;
	enh_cfg.spi_ddr = op->addr.dtr || op->dummy.dtr || op->data.dtr;
}

This allows several unsupported DTR patterns through validation during SPI
NOR and SPI NAND probing:

1. For 1-wire DTR operations (op->data.buswidth == 1, such as
SNOR_PROTO_1_1_1_DTR or SPINAND_PAGE_READ_FROM_CACHE_1S_1D_1D_OP),
dw_spi_exec_enh_mem_op() selects DW_SPI_CTRLR0_SPI_FRF_STD_SPI, where
DW_SPI_SPI_CTRLR0 and DW_SPI_ENH_CTRLR0_SPI_DDR_EN do not operate.

2. For 1-bit DTR address with multi-bit DTR data (op->addr.buswidth == 1 &&
op->data.buswidth > 1, such as SPINAND_PAGE_READ_FROM_CACHE_1S_1D_8D_OP,
1S_1D_4D_OP, and 1S_1D_2D_OP in drivers/mtd/nand/spi/winbond.c),
dw_spi_exec_enh_mem_op() selects DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT0, which
transmits both the instruction and address phases in 1-wire SDR mode rather
than DDR.

3. For 1-bit DTR instruction with multi-bit DTR address/data
(op->cmd.buswidth == 1 && op->cmd.dtr), dw_spi_exec_enh_mem_op() selects
TT0 or TT1 with DW_SPI_ENH_CTRLR0_INST_DDR_EN set, whereas INST_DDR_EN is
only valid in DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT2 when
DW_SPI_ENH_CTRLR0_SPI_DDR_EN is also enabled.

4. Because op->addr.dtr || op->dummy.dtr || op->data.dtr is collapsed into a
single DW_SPI_ENH_CTRLR0_SPI_DDR_EN bit controlling both address and data
phases simultaneously, mixed SDR/DDR address and data phases cannot be
configured independently.

5. In spi_mem_default_supports_op(), the op->cmd.nbytes != 1 check is
skipped whenever any phase has dtr set, and op->cmd.nbytes == 2 is only
enforced when op->cmd.dtr && op->cmd.buswidth == 8. Without an explicit
op->cmd.nbytes <= 2 check in dw_spi_supports_enh_mem_op(), non-8D DTR ops
with op->cmd.nbytes > 2 are accepted and programmed with
DW_SPI_ENH_CTRLR0_INST_L_INST_L0.

Should dw_spi_supports_enh_mem_op() reject these unsupported DTR phase and
instruction length combinations?

> @@ -1074,8 +1104,10 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
>  	else
>  		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L0;
>  
> -	if (op->dummy.buswidth)
> -		enh_cfg.wait_c = op->dummy.nbytes * BITS_PER_BYTE / op->dummy.buswidth;
> +	enh_cfg.wait_c = dw_spi_enh_wait_cycles(op);
> +
> +	enh_cfg.inst_ddr = op->cmd.dtr;
> +	enh_cfg.spi_ddr = op->addr.dtr || op->dummy.dtr || op->data.dtr;
>  
>  	dw_spi_enable_chip(dws, 0);
>  

[Severity: High]
How are non-data Octal DTR operations (8D-0-0 and 8D-8D-0) handled when
op->data.nbytes == 0 and op->data.buswidth == 0?

Once an Octal DTR flash enters stateful 8D-8D-8D mode (for example via
spi_nor_init() setting SNOR_PROTO_8_8_8_DTR after 8D-8D-8D read and page
program pass dw_spi_supports_enh_mem_op()), command-only operations such as
Write Enable or Soft Reset (8D-0-0) and address-only operations such as
Sector or Block Erase (8D-8D-0) have op->data.nbytes == 0 and
op->data.buswidth == 0.

In dw_spi_supports_enh_mem_op(), the buswidth checks compare
op->addr.buswidth and op->cmd.buswidth directly against op->data.buswidth:

drivers/spi/spi-dw-core.c:dw_spi_supports_enh_mem_op() {
	if (op->addr.nbytes != 0 && op->addr.buswidth != 1 &&
	    op->addr.buswidth != op->data.buswidth)
		return false;
	...
	if (op->cmd.buswidth != 1 && op->cmd.buswidth != op->addr.buswidth &&
	    op->cmd.buswidth != op->data.buswidth)
		return false;
}

When op->data.buswidth is 0, both 8D-8D-0 and 8D-0-0 operations fail these
checks and return false.

Additionally, in dw_spi_exec_enh_mem_op(), cfg.spi_frf and enh_cfg.trans_t
are derived from op->data.buswidth (falling back to
DW_SPI_CTRLR0_SPI_FRF_STD_SPI and DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT0 when
op->data.buswidth is 0), and for 8D-0-0 operations where only op->cmd.dtr is
set, enh_cfg.spi_ddr evaluates to false, leaving
DW_SPI_ENH_CTRLR0_SPI_DDR_EN unset and tde = 0 in dw_spi_update_config().

Could dw_spi_supports_enh_mem_op() and dw_spi_exec_enh_mem_op() derive the
transfer buswidth, frame format, and DDR enable state from the active phases
when no data phase is present?

> @@ -1383,6 +1415,11 @@ static const struct spi_controller_mem_caps dw_spi_mem_caps = {
>  	.per_op_freq = true,
>  };
>  
> +static const struct spi_controller_mem_caps dw_spi_enh_mem_caps = {
> +	.per_op_freq = true,
> +	.dtr = true,
> +};
> +
>  int dw_spi_add_controller(struct device *dev, struct dw_spi *dws)
>  {
>  	struct spi_controller *ctlr;
> @@ -1439,7 +1476,8 @@ int dw_spi_add_controller(struct device *dev, struct dw_spi *dws)
>  			ctlr->set_cs = dw_spi_set_cs;
>  		if (dws->mem_ops.exec_op) {
>  			ctlr->mem_ops = &dws->mem_ops;
> -			ctlr->mem_caps = &dw_spi_mem_caps;
> +			ctlr->mem_caps = (dws->caps & DW_SPI_CAP_EMODE) ?
> +					 &dw_spi_enh_mem_caps : &dw_spi_mem_caps;
>  		}
>  		ctlr->max_speed_hz = dws->max_freq;
>  		ctlr->flags = SPI_CONTROLLER_GPIO_SS;

[Severity: High]
Is it safe to advertise dw_spi_enh_mem_caps (.dtr = true) for all
controllers with DW_SPI_CAP_EMODE without probing whether the hardware
actually synthesizes DDR support?

During initialization in dw_spi_add_controller() -> dw_spi_hw_init() ->
detect_enh_mode(), the driver sets DW_SPI_CAP_EMODE after testing whether
Dual/Quad/Octal frame formats in DW_SPI_CTRLR0 and
DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN in DW_SPI_SPI_CTRLR0 are writable:

drivers/spi/spi-dw-core.c:detect_enh_mode() {
	...
	/* test clock stretching */
	dw_writel(dws, DW_SPI_SPI_CTRLR0, DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN);
	if ((DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN & dw_readl(dws, DW_SPI_SPI_CTRLR0)) !=
	    DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN) {
		mode = 0;
		goto disable_enh;
	}

	dws->caps |= DW_SPI_CAP_EMODE;
}

On Synopsys DW_apb_ssi (PSSI) and DWC_ssi (HSSI) controllers, enhanced SPI
mode, clock stretching, and DDR support (SSI_HAS_DDR, introduced in
DW_apb_ssi v4.02a) are independent synthesis parameters. When SSI_HAS_DDR is
not synthesized, DW_SPI_ENH_CTRLR0_SPI_DDR_EN and
DW_SPI_ENH_CTRLR0_INST_DDR_EN are read-only zero bits, and the
DW_SPI_DDR_DRIVE_EDGE register at offset 0xf8 is not implemented.

If a DTR-capable flash is attached to a controller that has DW_SPI_CAP_EMODE
but lacks hardware DDR support, advertising dw_spi_enh_mem_caps causes
spi-mem to issue DTR operations where DW_SPI_ENH_CTRLR0_SPI_DDR_EN is
ignored by hardware while dw_spi_enh_wait_cycles() halves the dummy cycles,
breaking flash transfers. In addition, dw_spi_update_config() will
unconditionally write to the unimplemented DW_SPI_DDR_DRIVE_EDGE register on
every enhanced SPI transfer.

Should detect_enh_mode() probe whether DW_SPI_ENH_CTRLR0_SPI_DDR_EN is
writable in DW_SPI_SPI_CTRLR0 (similar to the clock stretching check) and
record a dedicated capability flag before advertising DTR support or writing
DW_SPI_DDR_DRIVE_EDGE?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022102614.75696-1-changhuang.liang@starfivetech.com?part=1

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op()
  2026-10-22 10:45   ` Miquel Raynal
@ 2026-10-22 12:19     ` Michael Walle
  2026-10-22 13:27       ` Miquel Raynal
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Walle @ 2026-10-22 12:19 UTC (permalink / raw)
  To: Miquel Raynal, Changhuang Liang
  Cc: Pratyush Yadav, Takahiro Kuwano, Mark Brown, Richard Weinberger,
	Vignesh Raghavendra, linux-spi, linux-kernel, linux-mtd


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

On Thu Oct 22, 2026 at 12:45 PM CEST, Miquel Raynal wrote:
> On 22/10/2026 at 03:26:12 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:
>
>> Currently spi_nor_spimem_setup_op() sets all four phases (cmd, addr,
>> dummy, data) to DTR whenever spi_nor_protocol_is_dtr(proto) is true.
>> This is too coarse: the 1S-xD-xD DTR protocols (1_1_1_DTR, 1_2_2_DTR,
>> 1_4_4_DTR, 1_8_8_DTR) send the command phase in STR on a single line,
>> and only the address, dummy and data phases in DTR. SPIMEM supports
>> such mixed DTR modes.
>>
>> Split the handling:
>>
>>   - For SNOR_PROTO_8_8_8_DTR, keep the existing behavior: set all
>>     phases to DTR and use a 2-byte opcode (opcode + extension).
>>
>>   - For the other DTR protocols, set the command phase to STR and only
>>     the address, dummy and data phases to DTR. Double op->dummy.nbytes
>>     since DTR transfers 2 bytes per clock cycle, matching how the dummy
>>     cycle count is specified for these protocols.
>>
>> No in-tree flash currently declares SNOR_HWCAPS_READ_1_x_x_DTR, so
>> this is not a bug fix but preparation for enabling these protocols.
>> A follow-up patch will enable 1S-4D-4D for the MX66L1G45G. The
>> 8D-8D-8D path is unchanged.
>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>> ---
>>  drivers/mtd/spi-nor/core.c | 21 ++++++++++++++-------
>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 90524c58602e..43e506e728ab 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -97,13 +97,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>>  	if (op->data.nbytes)
>>  		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
>>  
>> -	if (spi_nor_protocol_is_dtr(proto)) {
>> -		/*
>> -		 * SPIMEM supports mixed DTR modes, but right now we can only
>> -		 * have all phases either DTR or STR. IOW, SPIMEM can have
>> -		 * something like 4S-4D-4D, but SPI NOR can't. So, set all 4
>> -		 * phases to either DTR or STR.
>> -		 */
>> +	if (proto == SNOR_PROTO_8_8_8_DTR) {
>>  		op->cmd.dtr = true;
>>  		op->addr.dtr = true;
>>  		op->dummy.dtr = true;
>> @@ -115,6 +109,19 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>>  		ext = spi_nor_get_cmd_ext(nor, op);
>>  		op->cmd.opcode = (op->cmd.opcode << 8) | ext;
>>  		op->cmd.nbytes = 2;
>> +	} else if (spi_nor_protocol_is_dtr(proto)) {
>> +		/*
>> +		 * 1S-xD-xD protocols: the opcode is sent in STR on a single
>> +		 * line, only the address, dummy and data phases are DTR. The
>> +		 * opcode stays 1 byte (no command extension).
>> +		 */
>> +		op->cmd.dtr = false;
>> +		op->addr.dtr = true;
>> +		op->dummy.dtr = true;
>> +		op->data.dtr = true;
>
> Still to coarse IMHO.
>
> What about xS-xS-xD?

Is this a thing? Are there any NOR flashes which support the address
being SDR? That doesn't make much sense, does it? The command being
SDR makes sense because you don't have to switch modes and thus
you're stateless.

Also spi-nor.h only defines 1S-nD-nD (for n=2,4,8) for DTR. IIRC the
original DTR code was just for 8D-8D-8D because that is actually
backed by the xSPI standard.

Anyway, can we just have something like:


if (spi_nor_protocol_is_dtr(proto)) {
		/* we only support 1SnDnD or 8D8D8D */
		op->addr.dtr = true;
		op->dummy.dtr = true;
		op->data.dtr = true;

		/* 2 bytes per clock cycle in DTR mode. */
		op->dummy.nbytes *= 2;

		if (SNOR_PROTO_8_8_8_DTR) {
			op->cmd.dtr = true;
			ext = spi_nor_get_cmd_ext(nor, op);
			op->cmd.opcode = (op->cmd.opcode << 8) | ext;
			op->cmd.nbytes = 2;
		}
}

Also you missed the support for SNOR_F_SWAP16.

-michael

> As mentioned in the spi controller patch you must make sure these are
> refused if they are not supported by your controller because SPI NAND at
> least allow them (and I believe SPI NOR might as well)?.
>
>> +
>> +		/* 2 bytes per clock cycle in DTR mode. */
>> +		op->dummy.nbytes *= 2;
>>  	}
>>  
>>  	if (proto == SNOR_PROTO_8_8_8_DTR && params->flags & SNOR_F_SWAP16)


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

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G
  2026-10-22 10:26 ` [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G Changhuang Liang
  2026-10-22 10:47   ` Miquel Raynal
@ 2026-10-22 12:23   ` Michael Walle
  2026-10-23 10:42     ` Changhuang Liang
  1 sibling, 1 reply; 17+ messages in thread
From: Michael Walle @ 2026-10-22 12:23 UTC (permalink / raw)
  To: Changhuang Liang, Pratyush Yadav, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi, linux-kernel, linux-mtd


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

On Thu Oct 22, 2026 at 12:26 PM CEST, Changhuang Liang wrote:
> The MX66L1G45G supports the 1-4-4 DTR read mode. This flash uses 6
> dummy cycles by default in 1-4-4 DTR mode, but the kernel's SFDP
> parser does not derive any SNOR_HWCAPS_READ_1_4_4_DTR capability from
> the BFPT table

So why don't you fix that?

> , and 4BAIT only filters already-known capabilities.
> The exact number of dummy cycles is also not directly given in the
> SFDP tables.

What is BFPT 3rd DWORD bits 4:0 then?

Could you please dump the SFDP for this part? See [1].

-michael

[1] https://docs.kernel.org/6.2/driver-api/mtd/spi-nor.html

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

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

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op()
  2026-10-22 12:19     ` Michael Walle
@ 2026-10-22 13:27       ` Miquel Raynal
  0 siblings, 0 replies; 17+ messages in thread
From: Miquel Raynal @ 2026-10-22 13:27 UTC (permalink / raw)
  To: Michael Walle
  Cc: Changhuang Liang, Pratyush Yadav, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra, linux-spi, linux-kernel,
	linux-mtd

On 22/10/2026 at 14:19:27 +02, "Michael Walle" <mwalle@kernel.org> wrote:

> On Thu Oct 22, 2026 at 12:45 PM CEST, Miquel Raynal wrote:
>> On 22/10/2026 at 03:26:12 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:
>>
>>> Currently spi_nor_spimem_setup_op() sets all four phases (cmd, addr,
>>> dummy, data) to DTR whenever spi_nor_protocol_is_dtr(proto) is true.
>>> This is too coarse: the 1S-xD-xD DTR protocols (1_1_1_DTR, 1_2_2_DTR,
>>> 1_4_4_DTR, 1_8_8_DTR) send the command phase in STR on a single line,
>>> and only the address, dummy and data phases in DTR. SPIMEM supports
>>> such mixed DTR modes.
>>>
>>> Split the handling:
>>>
>>>   - For SNOR_PROTO_8_8_8_DTR, keep the existing behavior: set all
>>>     phases to DTR and use a 2-byte opcode (opcode + extension).
>>>
>>>   - For the other DTR protocols, set the command phase to STR and only
>>>     the address, dummy and data phases to DTR. Double op->dummy.nbytes
>>>     since DTR transfers 2 bytes per clock cycle, matching how the dummy
>>>     cycle count is specified for these protocols.
>>>
>>> No in-tree flash currently declares SNOR_HWCAPS_READ_1_x_x_DTR, so
>>> this is not a bug fix but preparation for enabling these protocols.
>>> A follow-up patch will enable 1S-4D-4D for the MX66L1G45G. The
>>> 8D-8D-8D path is unchanged.
>>>
>>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>>> ---
>>>  drivers/mtd/spi-nor/core.c | 21 ++++++++++++++-------
>>>  1 file changed, 14 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>> index 90524c58602e..43e506e728ab 100644
>>> --- a/drivers/mtd/spi-nor/core.c
>>> +++ b/drivers/mtd/spi-nor/core.c
>>> @@ -97,13 +97,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>>>  	if (op->data.nbytes)
>>>  		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
>>>  
>>> -	if (spi_nor_protocol_is_dtr(proto)) {
>>> -		/*
>>> -		 * SPIMEM supports mixed DTR modes, but right now we can only
>>> -		 * have all phases either DTR or STR. IOW, SPIMEM can have
>>> -		 * something like 4S-4D-4D, but SPI NOR can't. So, set all 4
>>> -		 * phases to either DTR or STR.
>>> -		 */
>>> +	if (proto == SNOR_PROTO_8_8_8_DTR) {
>>>  		op->cmd.dtr = true;
>>>  		op->addr.dtr = true;
>>>  		op->dummy.dtr = true;
>>> @@ -115,6 +109,19 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>>>  		ext = spi_nor_get_cmd_ext(nor, op);
>>>  		op->cmd.opcode = (op->cmd.opcode << 8) | ext;
>>>  		op->cmd.nbytes = 2;
>>> +	} else if (spi_nor_protocol_is_dtr(proto)) {
>>> +		/*
>>> +		 * 1S-xD-xD protocols: the opcode is sent in STR on a single
>>> +		 * line, only the address, dummy and data phases are DTR. The
>>> +		 * opcode stays 1 byte (no command extension).
>>> +		 */
>>> +		op->cmd.dtr = false;
>>> +		op->addr.dtr = true;
>>> +		op->dummy.dtr = true;
>>> +		op->data.dtr = true;
>>
>> Still to coarse IMHO.

too*

>> What about xS-xS-xD?
>
> Is this a thing? Are there any NOR flashes which support the address
> being SDR? That doesn't make much sense, does it? The command being
> SDR makes sense because you don't have to switch modes and thus
> you're stateless.

I honnestly don't know if it's actually a thing today in SPI NOR, but it
is in SPI NAND. Just rising the flag.


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  2026-10-22 10:41   ` Miquel Raynal
@ 2026-10-23  9:42     ` Changhuang Liang
  2026-10-23  9:52       ` Miquel Raynal
  0 siblings, 1 reply; 17+ messages in thread
From: Changhuang Liang @ 2026-10-23  9:42 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mtd@lists.infradead.org

Hi, Miquel

Thanks for the review.

> On 22/10/2026 at 03:26:11 -07, Changhuang Liang
> <changhuang.liang@starfivetech.com> wrote:
> 
> > Add support for DDR (Dual Data-Rate) transfers in the enhanced SPI
> > mode of the DW SPI controller.
> >
> > The enhanced SPI control register (SPI_CTRLR0) has dedicated fields to
> > enable DDR mode for the instruction phase (INST_DDR_EN) and for the
> > address/dummy/data phases (SPI_DDR_EN). Populate these from the
> > corresponding dtr flags of the spi_mem_op.
> >
> > For DDR transfers, the dummy phase consumes two bits per clock per
> > line, so the computed wait cycles must be halved. Factor the wait
> > cycle calculation into a helper, dw_spi_enh_wait_cycles(), so that
> > both the supports check and the exec path use the same value.
> >
> > When SPI DDR is enabled, program the DDR_DRIVE_EDGE register. The
> > recommended TDE value is clk_div / 4, clamped to clk_div / 2 - 1 and
> > to the field width.
> >
> > Finally, advertise DTR support to the SPI memory layer through a
> > dedicated dw_spi_enh_mem_caps, selected when the controller has
> > DW_SPI_CAP_EMODE, so that spi-mem may issue DDR operations.
> >
> > Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> > ---
> >  drivers/spi/spi-dw-core.c | 52
> +++++++++++++++++++++++++++++++++------
> >  drivers/spi/spi-dw.h      |  6 +++++
> >  2 files changed, 51 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index 47434fa78e4b..6ff142c8a6bd 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -46,6 +46,8 @@ struct dw_spi_enh_cfg {
> >  	u8 inst_l;
> >  	u8 addr_l;
> >  	u8 trans_t;
> > +	bool inst_ddr;
> > +	bool spi_ddr;
> 
> If I understand this correctly, spi_ddr is quite a bad name because it means
> address and dummy and data ddr (but not instr). Can you try to come up with
> a better name for this variable?

spi_ddr is named exactly that way on the register field. Perhaps I won't change 
this variable name, and just add some comments to this variable—what do you think?

> Also, if my above understanding is correct, you must provide a
> supports_op() callback which would refuse any xS-xS-xD but allow xD-xD-xD
> and xS-xD-D modes.

OK, I'll add some filter conditions.

Best Regards,
Changhuang

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  2026-10-23  9:42     ` Changhuang Liang
@ 2026-10-23  9:52       ` Miquel Raynal
  2026-10-23  9:54         ` Changhuang Liang
  0 siblings, 1 reply; 17+ messages in thread
From: Miquel Raynal @ 2026-10-23  9:52 UTC (permalink / raw)
  To: Changhuang Liang
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mtd@lists.infradead.org


>> > --- a/drivers/spi/spi-dw-core.c
>> > +++ b/drivers/spi/spi-dw-core.c
>> > @@ -46,6 +46,8 @@ struct dw_spi_enh_cfg {
>> >  	u8 inst_l;
>> >  	u8 addr_l;
>> >  	u8 trans_t;
>> > +	bool inst_ddr;
>> > +	bool spi_ddr;
>> 
>> If I understand this correctly, spi_ddr is quite a bad name because it means
>> address and dummy and data ddr (but not instr). Can you try to come up with
>> a better name for this variable?
>
> spi_ddr is named exactly that way on the register field. Perhaps I won't change 
> this variable name, and just add some comments to this variable—what
> do you think?

This is a boolean, not a register bit field. Please use a name that
makes sense here. This is the part of the software that people must
understanding without deep hardware knowlege. You can keep the hardware
name for naming the bitfield operation if that's relevant though.

>> Also, if my above understanding is correct, you must provide a
>> supports_op() callback which would refuse any xS-xS-xD but allow xD-xD-xD
>> and xS-xD-D modes.
>
> OK, I'll add some filter conditions.
>
> Best Regards,
> Changhuang

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode
  2026-10-23  9:52       ` Miquel Raynal
@ 2026-10-23  9:54         ` Changhuang Liang
  0 siblings, 0 replies; 17+ messages in thread
From: Changhuang Liang @ 2026-10-23  9:54 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Mark Brown,
	Richard Weinberger, Vignesh Raghavendra,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mtd@lists.infradead.org

Hi , Miquel

> >> > --- a/drivers/spi/spi-dw-core.c
> >> > +++ b/drivers/spi/spi-dw-core.c
> >> > @@ -46,6 +46,8 @@ struct dw_spi_enh_cfg {
> >> >  	u8 inst_l;
> >> >  	u8 addr_l;
> >> >  	u8 trans_t;
> >> > +	bool inst_ddr;
> >> > +	bool spi_ddr;
> >>
> >> If I understand this correctly, spi_ddr is quite a bad name because
> >> it means address and dummy and data ddr (but not instr). Can you try
> >> to come up with a better name for this variable?
> >
> > spi_ddr is named exactly that way on the register field. Perhaps I
> > won't change this variable name, and just add some comments to this
> > variable—what do you think?
> 
> This is a boolean, not a register bit field. Please use a name that makes sense
> here. This is the part of the software that people must understanding without
> deep hardware knowlege. You can keep the hardware name for naming the
> bitfield operation if that's relevant though.

OK. Will update.

> 
> >> Also, if my above understanding is correct, you must provide a
> >> supports_op() callback which would refuse any xS-xS-xD but allow
> >> xD-xD-xD and xS-xD-D modes.
> >
> > OK, I'll add some filter conditions.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G
  2026-10-22 12:23   ` Michael Walle
@ 2026-10-23 10:42     ` Changhuang Liang
  0 siblings, 0 replies; 17+ messages in thread
From: Changhuang Liang @ 2026-10-23 10:42 UTC (permalink / raw)
  To: Michael Walle, Pratyush Yadav, Takahiro Kuwano, Mark Brown,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mtd@lists.infradead.org

Hi, Michael

Thanks for the review.

> On Thu Oct 22, 2026 at 12:26 PM CEST, Changhuang Liang wrote:
> > The MX66L1G45G supports the 1-4-4 DTR read mode. This flash uses 6
> > dummy cycles by default in 1-4-4 DTR mode, but the kernel's SFDP
> > parser does not derive any SNOR_HWCAPS_READ_1_4_4_DTR capability
> from
> > the BFPT table
> 
> So why don't you fix that?

Perhaps my commit message description wasn't good. For 1S-4D-4D, its support 
bit is in the 21st DWORD of the BFPT table, while the dummy cycles are described
in the 23rd DWORD. Unfortunately, the current flash model's BFPT only supports 
16 DWORDs.

> 
> > , and 4BAIT only filters already-known capabilities.
> > The exact number of dummy cycles is also not directly given in the
> > SFDP tables.
> 
> What is BFPT 3rd DWORD bits 4:0 then?

If I'm not mistaken, this one is for 1S-4S-4S.

> Could you please dump the SFDP for this part? See [1].
> 

# hexdump -C /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
00000000  53 46 44 50 06 01 02 ff  00 06 01 10 30 00 00 ff  |SFDP........0...|
00000010  c2 00 01 04 10 01 00 ff  84 00 01 02 c0 00 00 ff  |................|
00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000030  e5 20 fb ff ff ff ff 3f  44 eb 08 6b 08 3b 04 bb  |. .....?D..k.;..|
00000040  fe ff ff ff ff ff 00 ff  ff ff 44 eb 0c 20 0f 52  |..........D.. .R|
00000050  10 d8 00 ff d6 49 c5 00  85 df 04 e3 44 03 67 38  |.....I......D.g8|
00000060  30 b0 30 b0 f7 bd d5 5c  4a 9e 29 ff f0 50 f9 85  |0.0....\J.)..P..|
00000070  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
000000c0  7f ef ff ff 21 5c dc ff  ff ff ff ff ff ff ff ff  |....!\..........|
000000d0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000110  00 36 00 27 9d f9 c0 64  85 cb ff ff ff ff ff ff  |.6.'...d........|

> -michael
> 
> [1] https://docs.kernel.org/6.2/driver-api/mtd/spi-nor.html

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-10-23 10:43 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-22 10:26 [PATCH v1 0/4] Add DDR (Dual Data-Rate) support for DW SPI controller Changhuang Liang
2026-10-22 10:26 ` [PATCH v1 1/4] spi: dw: Add DDR (Dual Data-Rate) support for enhanced SPI mode Changhuang Liang
2026-10-22 10:41   ` Miquel Raynal
2026-10-23  9:42     ` Changhuang Liang
2026-10-23  9:52       ` Miquel Raynal
2026-10-23  9:54         ` Changhuang Liang
2026-10-22 10:48   ` sashiko-bot
2026-10-22 10:26 ` [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op() Changhuang Liang
2026-10-22 10:45   ` Miquel Raynal
2026-10-22 12:19     ` Michael Walle
2026-10-22 13:27       ` Miquel Raynal
2026-10-22 10:26 ` [PATCH v1 3/4] mtd: spi-nor: debugfs: Correct DTR protocol names for 1S-xD-xD modes Changhuang Liang
2026-10-22 10:46   ` Miquel Raynal
2026-10-22 10:26 ` [PATCH v1 4/4] mtd: spi-nor: macronix: Add 1-4-4 DTR read support for MX66L1G45G Changhuang Liang
2026-10-22 10:47   ` Miquel Raynal
2026-10-22 12:23   ` Michael Walle
2026-10-23 10:42     ` Changhuang Liang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox