[PATCH v1 00/11] Add support for StarFive JHB100 SFC

[PATCH v1 00/11] Add support for StarFive JHB100 SFC

[Changhuang Liang]

This serial add support for the StarFive JHB100 SoC SPI Flash
Controller (SFC), which is based on the Synopsys DesignWare SSI
version 2.00a but with some customizations and it also add
enhanced SPI for DesignWare SPI controllers.

I picked up some patches from series [1].
This series depends on the series [2]:

[1] https://lore.kernel.org/all/20221212180732.79167-1-sudip.mukherjee@sifive.com/
[2] https://lore.kernel.org/all/20260521012932.24163-1-changhuang.liang@starfivetech.com/

The expected SFC device tree configuration is as follows:

sfc0_filter_syscon: syscon@14090000 {
	compatible = "starfive,jhb100-sfc-filter-syscon", "syscon";
	reg = <0x0 0x14090000 0x0 0x1000>;
};

sfc0: spi@18000000 {
	compatible = "starfive,jhb100-sfc";
	reg = <0x0 0x18000000 0x0 0x10000>;
	#address-cells = <1>;
	#size-cells = <0>;
	interrupts = <111>;
	clocks = <&per1crg JHB100_PER1CLK_MAIN_ICG_EN_SFC0>;
	resets = <&per1crg JHB100_PER1RST_MAIN_RSTN_SFC0>;
	reset-names = "spi";
	starfive,sfc-filter-syscon = <&sfc0_filter_syscon>;
	num-cs = <2>;
	status = "disabled";
};

I will send a new version of series [2] to add the sfc0_filter_syscon node.

Changhuang Liang (2):
  spi: dt-bindings: snps,dw-apb-ssi: Add starfive,jhb100-sfc
  spi: dw: Add support for StarFive JHB100 SoC SFC

Sudip Mukherjee (9):
  spi: dw: Introduce spi_frf and STD_SPI
  spi: dw: update NDF while using enhanced spi mode
  spi: dw: update SPI_CTRLR0 register
  spi: dw: add check for support of enhanced spi
  spi: dw: Introduce enhanced single/dual/quad/octal spi
  spi: dw: send cmd and addr to start the spi transfer
  spi: dw: use irq handler for enhanced spi
  spi: dw: adjust size of mem_op
  spi: dw: detect enhanced spi mode

 .../bindings/spi/snps,dw-apb-ssi.yaml         |  22 ++
 drivers/spi/spi-dw-core.c                     | 344 +++++++++++++++++-
 drivers/spi/spi-dw-mmio.c                     |  42 +++
 drivers/spi/spi-dw.h                          |  52 ++-
 4 files changed, 450 insertions(+), 10 deletions(-)

--
2.25.1

[PATCH v1 01/11] spi: dw: Introduce spi_frf and STD_SPI

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

The DW APB SSI controllers of v4.x and newer and DW AHB SSI controllers
supports enhanced SPI modes which can be defined from SPI_FRF of
DW_SPI_CTRLR0 register. Without enhanced mode, these controllers will
work in the standard spi mode.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 13 ++++++++++++-
 drivers/spi/spi-dw.h      |  7 +++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index eff08461c2f5..86e3e7487bc7 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -330,6 +330,16 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 		/* CTRLR0[11:10] Transfer Mode */
 		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
 
+	if (dw_spi_ver_is_ge(dws, HSSI, 103A)) {
+		cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK;
+		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK,
+				  cfg->spi_frf);
+	} else if (dw_spi_ver_is_ge(dws, PSSI, 400A)) {
+		cr0 &= ~DW_PSSI_CTRLR0_SPI_FRF_MASK;
+		cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_SPI_FRF_MASK,
+				  cfg->spi_frf);
+	}
+
 	dw_writel(dws, DW_SPI_CTRLR0, cr0);
 
 	if (spi_controller_is_target(dws->ctlr))
@@ -422,6 +432,7 @@ static int dw_spi_transfer_one(struct spi_controller *ctlr,
 		.tmode = DW_SPI_CTRLR0_TMOD_TR,
 		.dfs = transfer->bits_per_word,
 		.freq = transfer->speed_hz,
+		.spi_frf = DW_SPI_CTRLR0_SPI_FRF_STD_SPI,
 	};
 	int ret;
 
@@ -677,7 +688,7 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
 static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
-	struct dw_spi_cfg cfg;
+	struct dw_spi_cfg cfg = {0};
 	unsigned long flags;
 	int ret;
 
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 2f2debc64e73..19cf1b1a5d4f 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -17,6 +17,9 @@
 
 /* Synopsys DW SSI component versions (FourCC sequence) */
 #define DW_HSSI_102A			0x3130322a
+#define DW_HSSI_103A			0x3130332a
+#define DW_HSSI_200A			0x3230302a
+#define DW_PSSI_400A			0x3430302a
 
 /* DW SSI IP-core ID and version check helpers */
 #define dw_spi_ip_is(_dws, _ip) \
@@ -94,6 +97,9 @@
 #define DW_HSSI_CTRLR0_TMOD_MASK		GENMASK(11, 10)
 #define DW_HSSI_CTRLR0_SRL			BIT(13)
 #define DW_HSSI_CTRLR0_MST			BIT(31)
+#define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22)
+#define DW_PSSI_CTRLR0_SPI_FRF_MASK		GENMASK(22, 21)
+#define DW_SPI_CTRLR0_SPI_FRF_STD_SPI		0x0
 
 /* Bit fields in CTRLR1 */
 #define DW_SPI_NDF_MASK				GENMASK(15, 0)
@@ -135,6 +141,7 @@ struct dw_spi_cfg {
 	u8 dfs;
 	u32 ndf;
 	u32 freq;
+	u8 spi_frf;
 };
 
 struct dw_spi;
-- 
2.25.1

[PATCH v1 02/11] spi: dw: update NDF while using enhanced spi mode

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

If the transfer of Transmit only mode is using dual/quad/octal SPI then
NDF needs to be updated with the number of data frames.
If the Transmit FIFO goes empty in-between, DWC_ssi masks the serial
clock and wait for rest of the data until the programmed amount of
frames are transferred successfully.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 86e3e7487bc7..bd41e1b4dba7 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -348,6 +348,9 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
 	    cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
 		dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
+	else if (cfg->tmode == DW_SPI_CTRLR0_TMOD_TO &&
+		 dws->caps & DW_SPI_CAP_EMODE)
+		dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf);
 
 	/* Note DW APB SSI clock divider doesn't support odd numbers */
 	clk_div = (DIV_ROUND_UP(dws->max_freq, cfg->freq) + 1) & 0xfffe;
-- 
2.25.1

Re: [PATCH v1 02/11] spi: dw: update NDF while using enhanced spi mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] For Transmit Only (`TMOD_TO`) transfers, `cfg.ndf` is never populated and defaults to 0, causing the hardware to be incorrectly programmed with 0 data frames.
- [Medium] Missing clamp of `op->data.nbytes` for `SPI_MEM_DATA_OUT` leads to `DW_SPI_CTRLR1` register overflow for large transmit-only operations.
- [Low] The macro `DW_SPI_CAP_EMODE` is used but never defined anywhere in the kernel tree, resulting in a compilation error.
--

--- Patch [2]: [PATCH v1 02/11] spi: dw: update NDF while using enhanced spi mode ---
commit b0d60b5fc76ca13c62d210b00ff881436c76397e
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: update NDF while using enhanced spi mode

If the transfer of Transmit only mode is using dual/quad/octal SPI then
NDF needs to be updated with the number of data frames. If the Transmit FIFO
goes empty in-between, DWC_ssi masks the serial clock and wait for rest of
the data until the programmed amount of frames are transferred successfully.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 2c52209c90955..8d4ad4035d7c9 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -348,6 +348,9 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>  	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
>  	    cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
>  		dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
> +	else if (cfg->tmode == DW_SPI_CTRLR0_TMOD_TO &&
> +		 dws->caps & DW_SPI_CAP_EMODE)

[Severity: Low]
Is DW_SPI_CAP_EMODE defined anywhere in the kernel tree?  It does not appear
to be declared, which results in a compilation error when building this file.

> +		dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf);

[Severity: High]
Does this correctly program the NDF register for outbound transfers?  Since
cfg.ndf is now used here for TMOD_TO, it seems it might inadvertently be zero.

In dw_spi_exec_mem_op(), cfg is zero-initialized and cfg.ndf is only populated
for SPI_MEM_DATA_IN:

drivers/spi/spi-dw-core.c:dw_spi_exec_mem_op() {
    ...
	if (op->data.dir == SPI_MEM_DATA_IN) {
		cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
		cfg.ndf = op->data.nbytes;
	} else {
		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
	}
    ...
	dw_spi_update_config(dws, mem->spi, &cfg);
    ...
}

Could this result in dw_writel() writing 0 data frames to DW_SPI_CTRLR1,
breaking the Enhanced SPI write functionality?

[Severity: Medium]
Since NDF is now used to bound write transfers in enhanced mode, should the
size be clamped for SPI_MEM_DATA_OUT to prevent register overflow?

Looking at dw_spi_adjust_mem_op_size(), the size is only clamped for reads:

drivers/spi/spi-dw-core.c:dw_spi_adjust_mem_op_size() {
	if (op->data.dir == SPI_MEM_DATA_IN)
		op->data.nbytes = clamp_val(op->data.nbytes, 0, DW_SPI_NDF_MASK + 1);

	return 0;
}

Could a large transmit operation pass through and overflow the 16-bit NDF
register when it is written to DW_SPI_CTRLR1 here?

>  
>  	/* Note DW APB SSI clock divider doesn't support odd numbers */
>  	clk_div = (DIV_ROUND_UP(dws->max_freq, cfg->freq) + 1) & 0xfffe;

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

[PATCH v1 03/11] spi: dw: update SPI_CTRLR0 register

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

If the SPI transfer is being done in enhanced mode then SPI_CTRLR0
register needs to be updated to mention the instruction length, address
length, address and instruction transfer format, wait cycles. And, we
also need to enable clock stretching.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 15 ++++++++++++---
 drivers/spi/spi-dw.h      | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index bd41e1b4dba7..fabfdf4ef604 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -313,7 +313,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 }
 
 void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
-			  struct dw_spi_cfg *cfg)
+			  struct dw_spi_cfg *cfg, struct dw_spi_enh_cfg *enh_cfg)
 {
 	struct dw_spi_chip_data *chip = spi_get_ctldata(spi);
 	u32 cr0 = chip->cr0;
@@ -366,6 +366,15 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 		dw_writel(dws, DW_SPI_RX_SAMPLE_DLY, chip->rx_sample_dly);
 		dws->cur_rx_sample_dly = chip->rx_sample_dly;
 	}
+
+	if (enh_cfg) {
+		cr0 = DW_SPI_ENH_CTRLR0_CLK_STRETCH_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);
+	}
 }
 EXPORT_SYMBOL_NS_GPL(dw_spi_update_config, "SPI_DW_CORE");
 
@@ -451,7 +460,7 @@ static int dw_spi_transfer_one(struct spi_controller *ctlr,
 
 	dw_spi_enable_chip(dws, 0);
 
-	dw_spi_update_config(dws, spi, &cfg);
+	dw_spi_update_config(dws, spi, &cfg, NULL);
 
 	transfer->effective_speed_hz = dws->current_freq;
 
@@ -718,7 +727,7 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	dw_spi_enable_chip(dws, 0);
 
-	dw_spi_update_config(dws, mem->spi, &cfg);
+	dw_spi_update_config(dws, mem->spi, &cfg, NULL);
 
 	dw_spi_mask_intr(dws, 0xff);
 
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 19cf1b1a5d4f..16a8c7ab7364 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -63,6 +63,7 @@
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
 #define DW_SPI_RX_SAMPLE_DLY		0xf0
+#define DW_SPI_SPI_CTRLR0		0xf4
 #define DW_SPI_CS_OVERRIDE		0xf4
 
 /* Bit fields in CTRLR0 (DWC APB SSI) */
@@ -127,6 +128,13 @@
 #define DW_SPI_DMACR_RDMAE			BIT(0)
 #define DW_SPI_DMACR_TDMAE			BIT(1)
 
+/* Bit fields in SPI_CTRLR0 */
+#define DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN	BIT(30)
+#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_ADDR_L_MASK		GENMASK(5, 2)
+#define DW_SPI_ENH_CTRLR0_TRANS_TYPE_MASK	GENMASK(1, 0)
+
 /* Mem/DMA operations helpers */
 #define DW_SPI_WAIT_RETRIES			5
 #define DW_SPI_BUF_SIZE \
@@ -144,6 +152,13 @@ struct dw_spi_cfg {
 	u8 spi_frf;
 };
 
+struct dw_spi_enh_cfg {
+	u8 wait_c;
+	u8 inst_l;
+	u8 addr_l;
+	u8 trans_t;
+};
+
 struct dw_spi;
 struct dw_spi_dma_ops {
 	int (*dma_init)(struct device *dev, struct dw_spi *dws);
@@ -294,7 +309,7 @@ static inline void dw_spi_shutdown_chip(struct dw_spi *dws)
 
 extern void dw_spi_set_cs(struct spi_device *spi, bool enable);
 extern void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
-				 struct dw_spi_cfg *cfg);
+				 struct dw_spi_cfg *cfg, struct dw_spi_enh_cfg *enh_cfg);
 extern int dw_spi_check_status(struct dw_spi *dws, bool raw);
 extern int dw_spi_add_controller(struct device *dev, struct dw_spi *dws);
 extern void dw_spi_remove_controller(struct dw_spi *dws);
-- 
2.25.1

Re: [PATCH v1 03/11] spi: dw: update SPI_CTRLR0 register

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced `enh_cfg` parameter in `dw_spi_update_config()` is never populated or passed by any caller, rendering the enhanced mode configuration logic effectively dead code.
--

--- Patch [3]: [PATCH v1 03/11] spi: dw: update SPI_CTRLR0 register ---
commit b571b86cd59a7cddd50fbc1933264b4589cd6f16
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: update SPI_CTRLR0 register

This commit updates the SPI DesignWare driver to configure the SPI_CTRLR0
register for enhanced mode transfers. It introduces a new dw_spi_enh_cfg
structure and passes it to dw_spi_update_config() to set up instruction 
lengths, address lengths, and wait cycles.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 8d4ad4035d7c9..6befe47929b4c 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
[ ... ]
> @@ -451,7 +460,7 @@ static int dw_spi_transfer_one(struct spi_controller *ctlr,
>  
>  	dw_spi_enable_chip(dws, 0);
>  
> -	dw_spi_update_config(dws, spi, &cfg);
> +	dw_spi_update_config(dws, spi, &cfg, NULL);

[Severity: Medium]
Is this intended to always be NULL here? By passing NULL unconditionally, 
the new enhanced configuration block in dw_spi_update_config() will never 
execute for this path.

[ ... ]
> @@ -718,7 +727,7 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  
>  	dw_spi_enable_chip(dws, 0);
>  
> -	dw_spi_update_config(dws, mem->spi, &cfg);
> +	dw_spi_update_config(dws, mem->spi, &cfg, NULL);

[Severity: Medium]
Similarly in dw_spi_exec_mem_op(), the enh_cfg parameter is also 
unconditionally passed as NULL. Is there a missing piece that was supposed 
to extract the enhanced mode configurations from the memory operation and 
pass them along? 

Since all current callers provide NULL, the newly added SPI_CTRLR0 register 
update logic inside dw_spi_update_config() appears to be permanently 
unreachable.

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

[PATCH v1 04/11] spi: dw: add check for support of enhanced spi

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

Before doing the mem op, spi controller will be queried about the
buswidths it supports. Add the dual/quad/octal if the controller
has the DW_SPI_CAP_EMODE capability.
The DW_SPI_CAP_EMODE capability will be enabled in a later patch.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 31 ++++++++++++++++++++++++++++++-
 drivers/spi/spi-dw.h      |  1 +
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index fabfdf4ef604..0d7c88d2c74d 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -521,6 +521,32 @@ static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	return 0;
 }
 
+static bool dw_spi_supports_enh_mem_op(struct spi_mem *mem,
+				       const struct spi_mem_op *op)
+{
+	if (op->addr.nbytes != 0 && op->addr.buswidth != 1 &&
+	    op->addr.buswidth != op->data.buswidth)
+		return false;
+
+	if (op->addr.nbytes >= 8)
+		return false;
+
+	if (op->cmd.buswidth != 1 && op->cmd.buswidth != op->addr.buswidth &&
+	    op->cmd.buswidth != op->data.buswidth)
+		return false;
+
+	if (op->dummy.nbytes && !op->dummy.buswidth)
+		return false;
+
+	if (op->dummy.nbytes != 0 && op->data.dir == SPI_MEM_DATA_OUT)
+		return false;
+
+	if (op->dummy.nbytes != 0 && op->dummy.nbytes / op->dummy.buswidth > 4)
+		return false;
+
+	return spi_mem_default_supports_op(mem, op);
+}
+
 static bool dw_spi_supports_mem_op(struct spi_mem *mem,
 				   const struct spi_mem_op *op)
 {
@@ -803,7 +829,10 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
 	if (!dws->mem_ops.exec_op && !(dws->caps & DW_SPI_CAP_CS_OVERRIDE) &&
 	    !dws->set_cs) {
 		dws->mem_ops.adjust_op_size = dw_spi_adjust_mem_op_size;
-		dws->mem_ops.supports_op = dw_spi_supports_mem_op;
+		if (dws->caps & DW_SPI_CAP_EMODE)
+			dws->mem_ops.supports_op = dw_spi_supports_enh_mem_op;
+		else
+			dws->mem_ops.supports_op = dw_spi_supports_mem_op;
 		dws->mem_ops.exec_op = dw_spi_exec_mem_op;
 		if (!dws->max_mem_freq)
 			dws->max_mem_freq = dws->max_freq;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 16a8c7ab7364..81a433ab759b 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -35,6 +35,7 @@
 /* DW SPI controller capabilities */
 #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
 #define DW_SPI_CAP_DFS32		BIT(1)
+#define DW_SPI_CAP_EMODE		BIT(2)
 
 /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
 #define DW_SPI_CTRLR0			0x00
-- 
2.25.1

Re: [PATCH v1 04/11] spi: dw: add check for support of enhanced spi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The dummy cycle check uses integer division incorrectly, allowing wait cycle configurations that silently overflow the 5-bit hardware wait cycle register.
- [High] The buswidth validation logic incorrectly rejects valid NO_DATA operations (e.g., QPI Erase) while accepting unsupported mixed-buswidth operations (e.g., 4-1-4).
--

--- Patch [4]: [PATCH v1 04/11] spi: dw: add check for support of enhanced spi ---
commit fe456b0bd9838cdf376b52d9ec69265dbf1ef090
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: add check for support of enhanced spi

This commit queries the spi controller for buswidths it supports before doing
a mem op. It adds dual/quad/octal mode checks if the controller has the
DW_SPI_CAP_EMODE capability.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 6befe47929b4c..010a665b17b76 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -521,6 +521,32 @@ static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>  	return 0;
>  }
>  
> +static bool dw_spi_supports_enh_mem_op(struct spi_mem *mem,
> +				       const struct spi_mem_op *op)
> +{
> +	if (op->addr.nbytes != 0 && op->addr.buswidth != 1 &&
> +	    op->addr.buswidth != op->data.buswidth)
> +		return false;

[Severity: High]
Does this logic incorrectly reject valid operations that have no data phase,
like Quad SPI Erase?

If an operation has no data phase (op->data.nbytes == 0), the SPI core
initializes op->data.buswidth to 0. For an erase command over QSPI
(e.g., 1-4-0), op->addr.buswidth != op->data.buswidth evaluates to 4 != 0,
which erroneously returns false.

> +
> +	if (op->addr.nbytes >= 8)
> +		return false;
> +
> +	if (op->cmd.buswidth != 1 && op->cmd.buswidth != op->addr.buswidth &&
> +	    op->cmd.buswidth != op->data.buswidth)
> +		return false;

[Severity: High]
Can this logical AND chain accidentally accept unsupported configurations?

For an operation like 4-1-4, op->cmd.buswidth != op->data.buswidth evaluates
as 4 != 4 (false), which short-circuits the AND chain and bypasses the
rejection.

> +
> +	if (op->dummy.nbytes && !op->dummy.buswidth)
> +		return false;
> +
> +	if (op->dummy.nbytes != 0 && op->data.dir == SPI_MEM_DATA_OUT)
> +		return false;
> +
> +	if (op->dummy.nbytes != 0 && op->dummy.nbytes / op->dummy.buswidth > 4)
> +		return false;

[Severity: High]
Does this integer division silently allow configurations that overflow the
5-bit hardware wait cycle register?

Due to truncation, a configuration with op->dummy.nbytes = 19 and
op->dummy.buswidth = 4 will evaluate as 4 > 4 (false) and be accepted.

This corresponds to 38 SPI clocks ((19 * 8) / 4). Since the hardware wait
cycle register (DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK) has a maximum limit of
31 cycles, programming 38 will be bitwise-ANDed by FIELD_PREP() to 6 cycles.
This could result in sampling data out of sync and corrupting the read.

> +
> +	return spi_mem_default_supports_op(mem, op);
> +}

[ ... ]

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

[PATCH v1 05/11] spi: dw: Introduce enhanced single/dual/quad/octal spi

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

If the spi transfer is using enhanced single/dual/quad/octal spi mode,
then we need to update the SPI_CTRLR0 register. The SPI_CTRLR0 register
will be updated in dw_spi_update_config() via the values in
dw_spi_enh_cfg.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 91 +++++++++++++++++++++++++++++++++++++--
 drivers/spi/spi-dw.h      |  9 ++++
 2 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 0d7c88d2c74d..9f3ee1d78c05 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -815,6 +815,89 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	return ret;
 }
 
+static void dw_spi_init_enh_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
+{
+	dws->n_bytes = 1;
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		dws->rx = op->data.buf.in;
+		dws->rx_len = op->data.nbytes;
+		dws->tx = NULL;
+		dws->tx_len = 0;
+	} else if (op->data.dir == SPI_MEM_DATA_OUT) {
+		dws->tx_len = op->data.nbytes;
+		dws->tx = (void *)op->data.buf.out;
+		dws->rx = NULL;
+		dws->rx_len = 0;
+	} else {
+		dws->rx = NULL;
+		dws->rx_len = 0;
+		dws->tx = NULL;
+		dws->tx_len = 0;
+	}
+}
+
+static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+	struct spi_controller *ctlr = mem->spi->controller;
+	struct dw_spi *dws = spi_controller_get_devdata(ctlr);
+	struct dw_spi_enh_cfg enh_cfg;
+	struct dw_spi_cfg cfg;
+
+	switch (op->data.buswidth) {
+	case 0:
+	case 1:
+		cfg.spi_frf = DW_SPI_CTRLR0_SPI_FRF_STD_SPI;
+		break;
+	case 2:
+		cfg.spi_frf = DW_SPI_CTRLR0_SPI_FRF_DUAL_SPI;
+		break;
+	case 4:
+		cfg.spi_frf = DW_SPI_CTRLR0_SPI_FRF_QUAD_SPI;
+		break;
+	case 8:
+		cfg.spi_frf = DW_SPI_CTRLR0_SPI_FRF_OCT_SPI;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	dw_spi_init_enh_mem_buf(dws, op);
+
+	cfg.dfs = 8;
+	cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
+	cfg.ndf = op->data.nbytes;
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		cfg.tmode = DW_SPI_CTRLR0_TMOD_RO;
+	else
+		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
+
+	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.addr_l = op->addr.nbytes << 1;
+	if (op->cmd.nbytes == 2)
+		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L16;
+	else if (op->cmd.nbytes == 1)
+		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L8;
+	else
+		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L0;
+
+	enh_cfg.wait_c = (op->dummy.nbytes * (BITS_PER_BYTE / op->dummy.buswidth));
+
+	dw_spi_enable_chip(dws, 0);
+
+	dw_spi_update_config(dws, mem->spi, &cfg, &enh_cfg);
+
+	dw_spi_enable_chip(dws, 1);
+
+	return 0;
+}
+
 /*
  * Initialize the default memory operations if a glue layer hasn't specified
  * custom ones. Direct mapping operations will be preserved anyway since DW SPI
@@ -829,11 +912,13 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
 	if (!dws->mem_ops.exec_op && !(dws->caps & DW_SPI_CAP_CS_OVERRIDE) &&
 	    !dws->set_cs) {
 		dws->mem_ops.adjust_op_size = dw_spi_adjust_mem_op_size;
-		if (dws->caps & DW_SPI_CAP_EMODE)
+		if (dws->caps & DW_SPI_CAP_EMODE) {
+			dws->mem_ops.exec_op = dw_spi_exec_enh_mem_op;
 			dws->mem_ops.supports_op = dw_spi_supports_enh_mem_op;
-		else
+		} else {
+			dws->mem_ops.exec_op = dw_spi_exec_mem_op;
 			dws->mem_ops.supports_op = dw_spi_supports_mem_op;
-		dws->mem_ops.exec_op = dw_spi_exec_mem_op;
+		}
 		if (!dws->max_mem_freq)
 			dws->max_mem_freq = dws->max_freq;
 	}
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 81a433ab759b..2dae81c15423 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -102,6 +102,9 @@
 #define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22)
 #define DW_PSSI_CTRLR0_SPI_FRF_MASK		GENMASK(22, 21)
 #define DW_SPI_CTRLR0_SPI_FRF_STD_SPI		0x0
+#define DW_SPI_CTRLR0_SPI_FRF_DUAL_SPI		0x1
+#define DW_SPI_CTRLR0_SPI_FRF_QUAD_SPI		0x2
+#define DW_SPI_CTRLR0_SPI_FRF_OCT_SPI		0x3
 
 /* Bit fields in CTRLR1 */
 #define DW_SPI_NDF_MASK				GENMASK(15, 0)
@@ -133,7 +136,13 @@
 #define DW_SPI_ENH_CTRLR0_CLK_STRETCH_EN	BIT(30)
 #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
+#define DW_SPI_ENH_CTRLR0_INST_L_INST_L8	0x2
+#define DW_SPI_ENH_CTRLR0_INST_L_INST_L16	0x3
 #define DW_SPI_ENH_CTRLR0_ADDR_L_MASK		GENMASK(5, 2)
+#define DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT0	0x0
+#define DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT1	0x1
+#define DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT2	0x2
 #define DW_SPI_ENH_CTRLR0_TRANS_TYPE_MASK	GENMASK(1, 0)
 
 /* Mem/DMA operations helpers */
-- 
2.25.1

Re: [PATCH v1 05/11] spi: dw: Introduce enhanced single/dual/quad/octal spi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Divide-by-zero in wait cycle calculation causes kernel panic on commands without dummy cycles.
- [Critical] Enhanced memory operations completely fail to perform actual data transfers, causing uninitialized memory reads and silently dropped writes.
- [High] `dw_spi_exec_enh_mem_op` ignores the operation-specific maximum frequency, violating the SPI memory API contract.
- [High] Calculated wait cycles can exceed the 5-bit hardware register limit and are silently truncated to 0, corrupting read timing.
- [High] Unclamped data length for enhanced mode write operations leads to 16-bit hardware register truncation.
--

--- Patch [5]: [PATCH v1 05/11] spi: dw: Introduce enhanced single/dual/quad/octal spi ---
commit 0f1cbd98fc45570180f08cb46b29e531be3e743b
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: Introduce enhanced single/dual/quad/octal spi

This commit introduces support for enhanced single/dual/quad/octal SPI modes
in the DesignWare SPI controller driver. It adds a new function to execute
enhanced memory operations and updates the SPI_CTRLR0 register configuration
when these modes are utilized.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 010a665b17b76..58159e8269f53 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -815,6 +815,89 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)

[ ... ]

> +static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
> +{
> +	struct spi_controller *ctlr = mem->spi->controller;
> +	struct dw_spi *dws = spi_controller_get_devdata(ctlr);
> +	struct dw_spi_enh_cfg enh_cfg;
> +	struct dw_spi_cfg cfg;
> +
> +	switch (op->data.buswidth) {

[ ... ]

> +	dw_spi_init_enh_mem_buf(dws, op);
> +
> +	cfg.dfs = 8;
> +	cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);

[Severity: High]
Does this code ignore the operation-specific maximum frequency?

The spi_mem_op struct specifies op->max_freq which dictates the maximum
safe clock speed for this specific command. 

Using mem->spi->max_speed_hz instead might cause operations with stricter
frequency limits to execute at the higher overall bus speed, potentially
leading to hardware timing violations.

> +	cfg.ndf = op->data.nbytes;
> +	if (op->data.dir == SPI_MEM_DATA_IN)
> +		cfg.tmode = DW_SPI_CTRLR0_TMOD_RO;
> +	else
> +		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;

[Severity: High]
Can enhanced transmit-only mode operations suffer from size truncation?

Enhanced Transmit Only mode uses DW_SPI_CTRLR1 to count frames to transmit,
and this is a 16-bit register. 

While dw_spi_adjust_mem_op_size() clamps SPI_MEM_DATA_IN, it does not
clamp SPI_MEM_DATA_OUT operations. If an outbound transfer is large,
such as exactly 65536 bytes, the unclamped length will be truncated when
written to the 16-bit DW_SPI_CTRLR1 register.

> +
> +	if (op->data.buswidth == op->addr.buswidth &&
> +	    op->data.buswidth == op->cmd.buswidth)
> +		enh_cfg.trans_t = DW_SPI_ENH_CTRLR0_TRANS_TYPE_TT2;

[ ... ]

> +	enh_cfg.addr_l = op->addr.nbytes << 1;
> +	if (op->cmd.nbytes == 2)
> +		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L16;
> +	else if (op->cmd.nbytes == 1)
> +		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L8;
> +	else
> +		enh_cfg.inst_l = DW_SPI_ENH_CTRLR0_INST_L_INST_L0;
> +
> +	enh_cfg.wait_c = (op->dummy.nbytes * (BITS_PER_BYTE / op->dummy.buswidth));

[Severity: Critical]
Can this cause a divide-by-zero crash if the operation has no dummy phase?

For operations like READ ID or standard read/write without dummies, both
op->dummy.nbytes and op->dummy.buswidth are initialized to 0. 

Since there is no check before the division, this will cause a kernel panic
whenever such a command is sent in enhanced mode.

[Severity: High]
Additionally, could this calculation result in a value that exceeds the
hardware register limit?

If op->dummy.nbytes is 4 and op->dummy.buswidth is 1, enh_cfg.wait_c
evaluates to 32. 

Later, dw_spi_update_config() applies a 5-bit mask to this value:

cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK, enh_cfg->wait_c);

Since 32 requires 6 bits, FIELD_PREP will truncate it to 0, incorrectly
configuring the controller for 0 wait cycles instead of 32.

> +
> +	dw_spi_enable_chip(dws, 0);
> +
> +	dw_spi_update_config(dws, mem->spi, &cfg, &enh_cfg);
> +
> +	dw_spi_enable_chip(dws, 1);
> +
> +	return 0;

[Severity: Critical]
Does this function complete without actually performing the SPI data
transfers?

The function enables the SPI chip and then immediately returns 0. It
appears to omit the steps to write the command, address, or data to the
data register, and does not read from the RX FIFO. 

Will this cause read operations to return uninitialized buffer memory and
write operations to silently drop all data?

> +}
> +
>  /*
>   * Initialize the default memory operations if a glue layer hasn't specified
>   * custom ones. Direct mapping operations will be preserved anyway since DW SPI

[ ... ]

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

[PATCH v1 06/11] spi: dw: send cmd and addr to start the spi transfer

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

In enhanced spi mode, read or write will start by sending the cmd
and address (if present).

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 9f3ee1d78c05..9f0f7e0b93a1 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -836,6 +836,19 @@ static void dw_spi_init_enh_mem_buf(struct dw_spi *dws, const struct spi_mem_op
 	}
 }
 
+static void dw_spi_enh_write_cmd_addr(struct dw_spi *dws, const struct spi_mem_op *op)
+{
+	/* Send cmd as 32 bit value */
+	dw_write_io_reg(dws, DW_SPI_DR, op->cmd.opcode);
+	if (op->addr.nbytes) {
+		dw_write_io_reg(dws, DW_SPI_DR, lower_32_bits(op->addr.val));
+		if (op->addr.nbytes > 4) {
+			/* address more than 32bit */
+			dw_write_io_reg(dws, DW_SPI_DR, upper_32_bits(op->addr.val));
+		}
+	}
+}
+
 static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct spi_controller *ctlr = mem->spi->controller;
@@ -895,6 +908,8 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
 
 	dw_spi_enable_chip(dws, 1);
 
+	dw_spi_enh_write_cmd_addr(dws, op);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH v1 06/11] spi: dw: send cmd and addr to start the spi transfer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Incomplete implementation of enhanced SPI memory operations causes data corruption.
- [Critical] Divide-by-zero kernel panic when calculating wait cycles for dummy-less SPI memory operations.
--

--- Patch [6]: [PATCH v1 06/11] spi: dw: send cmd and addr to start the spi transfer ---
commit 81aa9a8a81c58018d2abaeeaaa7a7b9aed04510d
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: send cmd and addr to start the spi transfer

In enhanced spi mode, read or write will start by sending the cmd
and address (if present).

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 58159e8269f53..6a52860654280 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c

[ ... ]

> @@ -895,6 +908,8 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
>  
>  	dw_spi_enable_chip(dws, 1);
>  
> +	dw_spi_enh_write_cmd_addr(dws, op);
> +
>  	return 0;
>  }

[Severity: Critical]
Does this code return 0 (success) without ever transferring data (e.g., via
polling or DMA) or tearing down the transfer?

The dw_spi_exec_enh_mem_op() function configures the SPI controller, enables
the chip, and writes the command and address, but returns 0 immediately. It
omits the data transfer phase (writing outbound data to or reading inbound
data from the SPI registers).

Does this skip waiting for the operation to complete or disabling the chip
select, leaving caller buffers uninitialized on reads or writes silently
dropped?

[Severity: Critical]
When calculating wait cycles for dummy-less SPI memory operations in
dw_spi_exec_enh_mem_op(), could this cause a divide-by-zero regression?

If an enhanced SPI memory operation is executed without any dummy cycles
(op->dummy.nbytes == 0), op->dummy.buswidth is initialized to 0.

Earlier in dw_spi_exec_enh_mem_op(), enh_cfg.wait_c is calculated as:

    enh_cfg.wait_c = (op->dummy.nbytes * (BITS_PER_BYTE / op->dummy.buswidth));

Due to the explicit parentheses, does the division evaluate first and
unconditionally crash the kernel?

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

[PATCH v1 07/11] spi: dw: use irq handler for enhanced spi

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

Introduce the interrupt handler for enhanced spi to read or write based
on the generated irq. Also, use the xfer_completion from spi_controller
to wait for a timeout or completion from irq handler.

In enhanced mode we need to calculate RXFTLR based on the length of data
we are expecting to receive or the fifo length.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 93 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 9f0f7e0b93a1..532441da235e 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -248,6 +248,34 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t dw_spi_enh_handler(struct dw_spi *dws)
+{
+	u16 irq_status = dw_readl(dws, DW_SPI_ISR);
+
+	if (dw_spi_check_status(dws, false)) {
+		spi_finalize_current_transfer(dws->ctlr);
+		return IRQ_HANDLED;
+	}
+
+	if (irq_status & DW_SPI_INT_RXFI) {
+		dw_reader(dws);
+		if (dws->rx_len <= dw_readl(dws, DW_SPI_RXFTLR))
+			dw_writel(dws, DW_SPI_RXFTLR, dws->rx_len - 1);
+	}
+
+	if (irq_status & DW_SPI_INT_TXEI)
+		dw_writer(dws);
+
+	if (!dws->tx_len && dws->rx_len) {
+		dw_spi_mask_intr(dws, DW_SPI_INT_TXEI);
+	} else if (!dws->rx_len && !dws->tx_len) {
+		dw_spi_mask_intr(dws, 0xff);
+		spi_finalize_current_transfer(dws->ctlr);
+	}
+
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t dw_spi_irq(int irq, void *dev_id)
 {
 	struct spi_controller *ctlr = dev_id;
@@ -257,8 +285,15 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
 	if (!irq_status)
 		return IRQ_NONE;
 
-	if (!ctlr->cur_msg) {
+	if (!ctlr->cur_msg && dws->transfer_handler ==
+	    dw_spi_transfer_handler) {
+		dw_spi_mask_intr(dws, 0xff);
+		return IRQ_HANDLED;
+	}
+	if (dws->transfer_handler == dw_spi_enh_handler &&
+	    !dws->rx_len && !dws->tx_len) {
 		dw_spi_mask_intr(dws, 0xff);
+		spi_finalize_current_transfer(ctlr);
 		return IRQ_HANDLED;
 	}
 
@@ -399,6 +434,34 @@ static void dw_spi_irq_setup(struct dw_spi *dws)
 	dw_spi_umask_intr(dws, imask);
 }
 
+static void dw_spi_enh_irq_setup(struct dw_spi *dws)
+{
+	u16 level;
+	u8 imask;
+
+	/*
+	 * Originally Tx and Rx data lengths match. Rx FIFO Threshold level
+	 * will be adjusted at the final stage of the IRQ-based SPI transfer
+	 * execution so not to lose the leftover of the incoming data.
+	 */
+	level = min_t(unsigned int, dws->fifo_len / 2, dws->tx_len);
+	dw_writel(dws, DW_SPI_TXFTLR, level);
+
+	/*
+	 * In enhanced mode if we are reading then tx_len is 0 as we
+	 * have nothing to transmit. Calculate DW_SPI_RXFTLR with
+	 * rx_len.
+	 */
+	level = min_t(unsigned int, dws->fifo_len / 2, dws->rx_len);
+	dw_writel(dws, DW_SPI_RXFTLR, level - 1);
+
+	dws->transfer_handler = dw_spi_enh_handler;
+
+	imask = DW_SPI_INT_TXEI | DW_SPI_INT_TXOI |
+		DW_SPI_INT_RXUI | DW_SPI_INT_RXOI | DW_SPI_INT_RXFI;
+	dw_spi_umask_intr(dws, imask);
+}
+
 /*
  * The iterative procedure of the poll-based transfer is simple: write as much
  * as possible to the Tx FIFO, wait until the pending to receive data is ready
@@ -855,6 +918,7 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
 	struct dw_spi *dws = spi_controller_get_devdata(ctlr);
 	struct dw_spi_enh_cfg enh_cfg;
 	struct dw_spi_cfg cfg;
+	unsigned long long ms;
 
 	switch (op->data.buswidth) {
 	case 0:
@@ -906,9 +970,36 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
 
 	dw_spi_update_config(dws, mem->spi, &cfg, &enh_cfg);
 
+	dw_spi_mask_intr(dws, 0xff);
+	reinit_completion(&ctlr->xfer_completion);
 	dw_spi_enable_chip(dws, 1);
 
 	dw_spi_enh_write_cmd_addr(dws, op);
+	dw_spi_set_cs(mem->spi, false);
+
+	udelay(5);
+
+	dw_spi_enh_irq_setup(dws);
+
+	/* Use timeout calculation from spi_transfer_wait() */
+	ms = 8LL * MSEC_PER_SEC * (dws->rx_len ? dws->rx_len : dws->tx_len);
+	do_div(ms, dws->current_freq);
+
+	/*
+	 * Increase it twice and add 200 ms tolerance, use
+	 * predefined maximum in case of overflow.
+	 */
+	ms += ms + 200;
+	if (ms > UINT_MAX)
+		ms = UINT_MAX;
+
+	ms = wait_for_completion_timeout(&ctlr->xfer_completion,
+					 msecs_to_jiffies(ms));
+
+	dw_spi_stop_mem_op(dws, mem->spi);
+
+	if (ms == 0)
+		return -EIO;
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH v1 07/11] spi: dw: use irq handler for enhanced spi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Integer underflow when writing to the DW_SPI_RXFTLR hardware register causes 0xFFFFFFFF to be written.
- [High] A hardcoded 5 microsecond delay before unmasking interrupts can cause RX FIFO overflow during high-speed SPI reads.
- [High] Enhanced SPI memory operations for write and no-data commands are prematurely aborted, discarding data still in the TX FIFO.
- [High] Hardware errors (FIFO underflows/overflows) during enhanced SPI memory operations are silently ignored, reporting success.
- [High] Missing interrupt mask on the timeout path of `dw_spi_exec_enh_mem_op` leads to a Use-After-Free and interrupt storm.
--

--- Patch [7]: [PATCH v1 07/11] spi: dw: use irq handler for enhanced spi ---
commit 788453296e7040a4cdd9daf4c44b6d7c00eb97d0
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: use irq handler for enhanced spi

This commit introduces an interrupt handler for enhanced SPI to perform
reads and writes based on generated IRQs. It also utilizes the
xfer_completion from spi_controller to wait for a timeout or completion
from the new IRQ handler.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -248,6 +248,34 @@ static irqreturn_t dw_spi_transfer_handler(struct dw_spi *dws)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t dw_spi_enh_handler(struct dw_spi *dws)
> +{
> +	u16 irq_status = dw_readl(dws, DW_SPI_ISR);
> +
> +	if (dw_spi_check_status(dws, false)) {
> +		spi_finalize_current_transfer(dws->ctlr);
> +		return IRQ_HANDLED;
> +	}

[Severity: High]
Does this silently ignore hardware errors during memory operations?

When a hardware error like an RX FIFO overflow occurs, dw_spi_check_status()
attempts to record the error by setting dws->ctlr->cur_msg->status.
However, memory operations bypass the standard spi_message queue, so cur_msg
is NULL and the error code is discarded.

The handler then signals completion, and the caller sees a successful wait
without knowing the data might be corrupted.

> +
> +	if (irq_status & DW_SPI_INT_RXFI) {
> +		dw_reader(dws);
> +		if (dws->rx_len <= dw_readl(dws, DW_SPI_RXFTLR))
> +			dw_writel(dws, DW_SPI_RXFTLR, dws->rx_len - 1);
> +	}

[Severity: High]
Can this cause an integer underflow when writing to the hardware register?

When a read operation finishes and dws->rx_len reaches 0, the check evaluates
to true, and this writes 0xFFFFFFFF to DW_SPI_RXFTLR.

> +
> +	if (irq_status & DW_SPI_INT_TXEI)
> +		dw_writer(dws);
> +
> +	if (!dws->tx_len && dws->rx_len) {
> +		dw_spi_mask_intr(dws, DW_SPI_INT_TXEI);
> +	} else if (!dws->rx_len && !dws->tx_len) {
> +		dw_spi_mask_intr(dws, 0xff);
> +		spi_finalize_current_transfer(dws->ctlr);
> +	}
> +
> +	return IRQ_HANDLED;
> +}

[Severity: High]
Does this prematurely abort write operations?

For write-only operations like flashing memory, dws->tx_len drops to 0 as soon
as all bytes are pushed into the TX FIFO. This signals completion immediately,
before the hardware actually transmits the bytes over the wire.

The upstream caller will then disable the SPI chip, which flushes the FIFOs
and discards the trailing bytes.

[ ... ]

> @@ -399,6 +434,34 @@ static void dw_spi_irq_setup(struct dw_spi *dws)
>  	dw_spi_umask_intr(dws, imask);
>  }
>  
> +static void dw_spi_enh_irq_setup(struct dw_spi *dws)
> +{
> +	u16 level;
> +	u8 imask;
> +
> +	/*
> +	 * Originally Tx and Rx data lengths match. Rx FIFO Threshold level
> +	 * will be adjusted at the final stage of the IRQ-based SPI transfer
> +	 * execution so not to lose the leftover of the incoming data.
> +	 */
> +	level = min_t(unsigned int, dws->fifo_len / 2, dws->tx_len);
> +	dw_writel(dws, DW_SPI_TXFTLR, level);
> +
> +	/*
> +	 * In enhanced mode if we are reading then tx_len is 0 as we
> +	 * have nothing to transmit. Calculate DW_SPI_RXFTLR with
> +	 * rx_len.
> +	 */
> +	level = min_t(unsigned int, dws->fifo_len / 2, dws->rx_len);
> +	dw_writel(dws, DW_SPI_RXFTLR, level - 1);

[Severity: High]
Will this write 0xFFFFFFFF to DW_SPI_RXFTLR during TX-only operations?

For any TX-only memory operation like a flash write, dws->rx_len is 0. This
results in level becoming 0, causing an unconditional write of 0xFFFFFFFF
to the hardware register.

[ ... ]

> @@ -906,9 +970,36 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
>  
>  	dw_spi_update_config(dws, mem->spi, &cfg, &enh_cfg);
>  
> +	dw_spi_mask_intr(dws, 0xff);
> +	reinit_completion(&ctlr->xfer_completion);
>  	dw_spi_enable_chip(dws, 1);
>  
>  	dw_spi_enh_write_cmd_addr(dws, op);
> +	dw_spi_set_cs(mem->spi, false);
> +
> +	udelay(5);
> +
> +	dw_spi_enh_irq_setup(dws);

[Severity: High]
Can this delay cause an RX FIFO overflow?

The hardware transfer begins when CS is asserted, but the CPU blocks on this
udelay(5) before unmasking the interrupts.

During these 5 microseconds, the hardware is actively running and clocking in
data, but the CPU is prevented from draining the RX FIFO. At higher SPI clock
speeds with smaller FIFOs, this can result in the hardware receiving more data
than the FIFO can hold within 5us, leading to an RX overflow and data loss.

> +
> +	/* Use timeout calculation from spi_transfer_wait() */
> +	ms = 8LL * MSEC_PER_SEC * (dws->rx_len ? dws->rx_len : dws->tx_len);
> +	do_div(ms, dws->current_freq);
> +
> +	/*
> +	 * Increase it twice and add 200 ms tolerance, use
> +	 * predefined maximum in case of overflow.
> +	 */
> +	ms += ms + 200;
> +	if (ms > UINT_MAX)
> +		ms = UINT_MAX;
> +
> +	ms = wait_for_completion_timeout(&ctlr->xfer_completion,
> +					 msecs_to_jiffies(ms));
> +
> +	dw_spi_stop_mem_op(dws, mem->spi);
> +
> +	if (ms == 0)
> +		return -EIO;

[Severity: High]
Does this timeout path leave interrupts unmasked and cause a use-after-free?

If wait_for_completion_timeout() times out, this returns -EIO without
masking the interrupts via dw_spi_mask_intr().

Calling dw_spi_stop_mem_op() disables and re-enables the SPI chip, which
flushes the TX FIFO. Because the TXEI interrupt was never masked, re-enabling
the chip instantly triggers the interrupt.

The unmasked dw_spi_enh_handler() will fire asynchronously and call
dw_writer() to push more data, dereferencing dws->tx. Since this function
already returned -EIO, the upstream caller likely freed or popped the buffer
off the stack.

>  
>  	return 0;
>  }

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

[PATCH v1 08/11] spi: dw: adjust size of mem_op

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

In enhanced mode adjust the size of the data that can be sent or received
as this will then be used to set the NDF.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 532441da235e..0dbf250a101b 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -576,6 +576,13 @@ static int dw_spi_target_abort(struct spi_controller *ctlr)
 	return 0;
 }
 
+static int dw_spi_adjust_enh_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
+{
+	op->data.nbytes = clamp_val(op->data.nbytes, 0, DW_SPI_NDF_MASK + 1);
+
+	return 0;
+}
+
 static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 {
 	if (op->data.dir == SPI_MEM_DATA_IN)
@@ -1017,13 +1024,14 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
 {
 	if (!dws->mem_ops.exec_op && !(dws->caps & DW_SPI_CAP_CS_OVERRIDE) &&
 	    !dws->set_cs) {
-		dws->mem_ops.adjust_op_size = dw_spi_adjust_mem_op_size;
 		if (dws->caps & DW_SPI_CAP_EMODE) {
 			dws->mem_ops.exec_op = dw_spi_exec_enh_mem_op;
 			dws->mem_ops.supports_op = dw_spi_supports_enh_mem_op;
+			dws->mem_ops.adjust_op_size = dw_spi_adjust_enh_mem_op_size;
 		} else {
 			dws->mem_ops.exec_op = dw_spi_exec_mem_op;
 			dws->mem_ops.supports_op = dw_spi_supports_mem_op;
+			dws->mem_ops.adjust_op_size = dw_spi_adjust_mem_op_size;
 		}
 		if (!dws->max_mem_freq)
 			dws->max_mem_freq = dws->max_freq;
-- 
2.25.1

Re: [PATCH v1 08/11] spi: dw: adjust size of mem_op

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] A guaranteed divide-by-zero exception occurs in `dw_spi_exec_enh_mem_op()` when executing SPI operations that lack a dummy phase.
- [High] Clamping memory operation size to 65536 overflows the 16-bit NDF hardware register for Transmit-Only operations in enhanced mode.
- [High] Enhanced SPI mem ops disable the controller before the hardware finishes transmitting, causing trailing data loss.
- [High] Incorrect dummy cycle bounds check in Enhanced SPI mode allows `wait_c` overflow, leading to zero dummy cycles.
- [High] Incorrect FIFO write order for addresses larger than 32 bits in Enhanced SPI mode causes endianness reversal.
- [High] A hardcoded 5µs delay (`udelay(5)`) after asserting Chip Select leaves the RX FIFO susceptible to unhandled overflow on high-speed SPI buses.
--

--- Patch [8]: [PATCH v1 08/11] spi: dw: adjust size of mem_op ---
commit 1cc8ff7449ec8b3ec7ea070e7ac5dce9adb45f0c
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: adjust size of mem_op
    
This patch adjusts the size of the data that can be sent or received
in enhanced mode, utilizing the new bounds to set the NDF register
appropriately.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -576,6 +576,13 @@ static int dw_spi_target_abort(struct spi_controller *ctlr)
>  	return 0;
>  }
>  
> +static int dw_spi_adjust_enh_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> +	op->data.nbytes = clamp_val(op->data.nbytes, 0, DW_SPI_NDF_MASK + 1);

[Severity: High]
Does this clamp operation cause an overflow when written to the hardware?

If op->data.nbytes is exactly 65536 (DW_SPI_NDF_MASK + 1), writing this
value to the DW_SPI_CTRLR1 register, which is a 16-bit register, will set bits
15:0 to 0. This silently truncates maximum-length 65536-byte block transfers
down to 0 frames, which could cause data corruption on large writes.

> +
> +	return 0;
> +}
> +

[ ... ]

> @@ -1017,13 +1024,14 @@ static void dw_spi_init_mem_ops(struct dw_spi *dws)
>  {
>  	if (!dws->mem_ops.exec_op && !(dws->caps & DW_SPI_CAP_CS_OVERRIDE) &&
>  	    !dws->set_cs) {
> -		dws->mem_ops.adjust_op_size = dw_spi_adjust_mem_op_size;
>  		if (dws->caps & DW_SPI_CAP_EMODE) {
>  			dws->mem_ops.exec_op = dw_spi_exec_enh_mem_op;

[Severity: Critical]
Does this execution path trigger a divide-by-zero exception when operations
lack a dummy phase?

In dw_spi_exec_enh_mem_op(), the dummy cycles are calculated like this:

drivers/spi/spi-dw-core.c:dw_spi_exec_enh_mem_op() {
    ...
    enh_cfg.wait_c = (op->dummy.nbytes * (BITS_PER_BYTE / op->dummy.buswidth));
    ...
}

When an operation has no dummy cycles, the SPI core leaves op->dummy.buswidth
initialized to 0. Due to C operator precedence, the division happens before the
multiplication, which triggers a divide-by-zero exception unconditionally on
basic commands like Read ID.


[Severity: High]
Can this sequence cause trailing data loss by disabling the controller
prematurely?

When the IRQ handler signals transfer completion:

drivers/spi/spi-dw-core.c:dw_spi_enh_handler() {
    ...
    spi_finalize_current_transfer(dws->ctlr);
    ...
}

The main thread wakes up and immediately stops the operation:

drivers/spi/spi-dw-core.c:dw_spi_exec_enh_mem_op() {
    ...
    dw_spi_stop_mem_op(dws, mem->spi);
    ...
}

This disables the controller (dw_spi_enable_chip(dws, 0)) without waiting
for the BUSY bit to clear. Doing so clears internal FIFOs and abruptly aborts
ongoing transmissions on the wire.


[Severity: High]
Does the bounds check for dummy cycles allow an overflow that results in
zero dummy cycles being executed?

In dw_spi_supports_enh_mem_op(), the check allows up to 32 dummy cycles:

drivers/spi/spi-dw-core.c:dw_spi_supports_enh_mem_op() {
    ...
    if (op->dummy.nbytes != 0 && op->dummy.nbytes / op->dummy.buswidth > 4)
    ...
}

However, the WAIT_CYCLES field in SPI_CTRLR0 is 5 bits wide. Passing
wait_c = 32 to FIELD_PREP() in dw_spi_update_config() with a 5-bit
mask silently truncates the value to 0:

drivers/spi/spi-dw-core.c:dw_spi_update_config() {
    ...
    cr0 |= FIELD_PREP(DW_SPI_ENH_CTRLR0_WAIT_CYCLE_MASK, enh_cfg->wait_c);
    ...
}

This would cause the hardware to execute 0 dummy cycles instead of 32,
destroying data synchronization on subsequent reads.


[Severity: High]
Does the FIFO write order for addresses larger than 32 bits cause an
endianness reversal?

In dw_spi_enh_write_cmd_addr(), the lower 32 bits are written before the
upper 32 bits:

drivers/spi/spi-dw-core.c:dw_spi_enh_write_cmd_addr() {
    ...
    dw_write_io_reg(dws, DW_SPI_DR, lower_32_bits(op->addr.val));
    if (op->addr.nbytes > 4) {
        /* address more than 32bit */
        dw_write_io_reg(dws, DW_SPI_DR, upper_32_bits(op->addr.val));
    }
    ...
}

Since the controller transmits FIFO words in the order they are written, the
lower 32 bits are sent over the wire first. The SPI protocol mandates sending
the highest byte first, which could result in accessing incorrect addresses on
the flash device.


[Severity: High]
Can the hardcoded 5 microsecond delay leave the RX FIFO susceptible to
overflow on high-speed SPI buses?

In dw_spi_exec_enh_mem_op(), the CPU sleeps while the hardware is active:

drivers/spi/spi-dw-core.c:dw_spi_exec_enh_mem_op() {
    ...
    dw_spi_enh_write_cmd_addr(dws, op, mem);

    udelay(5);

    dw_spi_enh_irq_setup(dws);
    ...
}

Asserting the chip select inherently starts the clocking of SPI data. On a fast
SPI bus, the hardware will receive many bytes during this 5 microsecond delay.
Because RX FIFO depths are limited, the FIFO could overflow while the CPU
sleeps before the interrupts are unmasked, failing the transfer.

>  			dws->mem_ops.supports_op = dw_spi_supports_enh_mem_op;
> +			dws->mem_ops.adjust_op_size = dw_spi_adjust_enh_mem_op_size;
>  		} else {

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

[PATCH v1 09/11] spi: dw: detect enhanced spi mode

[Changhuang Liang]

From: Sudip Mukherjee <sudip.mukherjee@sifive.com>

All the SSI controllers supporting enhanced spi modes might not support
all the three dual or quad or octal modes. Detect the modes that are
supported and finally enable the DW_SPI_CAP_EMODE capability which will
start using all the enhanced spi functions that has been added.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
Co-developed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 62 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 0dbf250a101b..0abdee82eaab 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -1083,6 +1083,64 @@ static void dw_spi_cleanup(struct spi_device *spi)
 	spi_set_ctldata(spi, NULL);
 }
 
+static u16 detect_enh_mode(struct dw_spi *dws)
+{
+	u32 tmp_spi_ctrlr0, tmp_ctrlr0;
+	u32 tmp_val, frf_shift;
+	u16 mode = 0;
+
+	if (dw_spi_ver_is_ge(dws, HSSI, 103A))
+		frf_shift = __bf_shf(DW_HSSI_CTRLR0_SPI_FRF_MASK);
+	else if (dw_spi_ver_is_ge(dws, PSSI, 400A))
+		frf_shift = __bf_shf(DW_PSSI_CTRLR0_SPI_FRF_MASK);
+	else
+		return 0;
+
+	tmp_ctrlr0 = dw_readl(dws, DW_SPI_CTRLR0);
+	tmp_spi_ctrlr0 = dw_readl(dws, DW_SPI_SPI_CTRLR0);
+	dw_spi_enable_chip(dws, 0);
+
+	/* test dual mode */
+	tmp_val = DW_SPI_CTRLR0_SPI_FRF_DUAL_SPI << frf_shift;
+	dw_writel(dws, DW_SPI_CTRLR0, tmp_val);
+	if ((tmp_val & dw_readl(dws, DW_SPI_CTRLR0)) == tmp_val)
+		mode |= SPI_TX_DUAL | SPI_RX_DUAL;
+
+	/* test quad mode */
+	tmp_val = DW_SPI_CTRLR0_SPI_FRF_QUAD_SPI << frf_shift;
+	dw_writel(dws, DW_SPI_CTRLR0, tmp_val);
+	if ((tmp_val & dw_readl(dws, DW_SPI_CTRLR0)) == tmp_val)
+		mode |= SPI_TX_QUAD | SPI_RX_QUAD;
+
+	/* test octal mode */
+	tmp_val = DW_SPI_CTRLR0_SPI_FRF_OCT_SPI << frf_shift;
+	dw_writel(dws, DW_SPI_CTRLR0, tmp_val);
+	if ((tmp_val & dw_readl(dws, DW_SPI_CTRLR0)) == tmp_val)
+		mode |= SPI_TX_OCTAL | SPI_RX_OCTAL;
+
+	if (!mode)
+		goto disable_enh;
+
+	/* 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)
+		/*
+		 * If clock stretching is not enabled then do not use
+		 * enhanced mode.
+		 */
+		goto disable_enh;
+
+	dws->caps |= DW_SPI_CAP_EMODE;
+
+disable_enh:
+	dw_writel(dws, DW_SPI_CTRLR0, tmp_ctrlr0);
+	dw_writel(dws, DW_SPI_SPI_CTRLR0, tmp_spi_ctrlr0);
+	dw_spi_enable_chip(dws, 1);
+
+	return mode;
+}
+
 /* Restart the controller, disable all interrupts, clean rx fifo */
 static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
 {
@@ -1162,6 +1220,9 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
 		dws->caps |= DW_SPI_CAP_DFS32;
 	}
 
+	dws->ctlr->mode_bits = SPI_CPOL | SPI_CPHA;
+	dws->ctlr->mode_bits |= detect_enh_mode(dws);
+
 	/* enable HW fixup for explicit CS deselect for Amazon's alpine chip */
 	if (dws->caps & DW_SPI_CAP_CS_OVERRIDE)
 		dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF);
@@ -1206,7 +1267,6 @@ int dw_spi_add_controller(struct device *dev, struct dw_spi *dws)
 
 	dw_spi_init_mem_ops(dws);
 
-	ctlr->mode_bits = SPI_CPOL | SPI_CPHA;
 	if (dws->caps & DW_SPI_CAP_DFS32)
 		ctlr->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
 	else
-- 
2.25.1

Re: [PATCH v1 09/11] spi: dw: detect enhanced spi mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] detect_enh_mode() leaks detected multi-lane modes into ctlr->mode_bits on clock stretch test failure.
- [High] Advertising multi-bit SPI modes globally without supporting them in dw_spi_transfer_one() causes standard multi-lane transfers to corrupt data.
- [High] The register writability test falsely detects OCTAL mode support on QUAD-only hardware.
- [High] dw_spi_hw_init() blindly overwrites ctlr->mode_bits, wiping out dynamically established capabilities like SPI_LOOP after a suspend/resume cycle.
--

--- Patch [9]: [PATCH v1 09/11] spi: dw: detect enhanced spi mode ---
commit 0f885f7a3380b812c7026e7502b6a3b2baf64278
Author: Sudip Mukherjee <sudip.mukherjee@sifive.com>

spi: dw: detect enhanced spi mode

All the SSI controllers supporting enhanced spi modes might not support
all the three dual or quad or octal modes. Detect the modes that are
supported and finally enable the DW_SPI_CAP_EMODE capability which will
start using all the enhanced spi functions that has been added.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 526b54385cd3e..ef1abb540f069 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
[ ... ]
> @@ -1083,6 +1083,64 @@ static void dw_spi_cleanup(struct spi_device *spi)
[ ... ]
> +static u16 detect_enh_mode(struct dw_spi *dws)
> +{
[ ... ]
> +	/* test octal mode */
> +	tmp_val = DW_SPI_CTRLR0_SPI_FRF_OCT_SPI << frf_shift;
> +	dw_writel(dws, DW_SPI_CTRLR0, tmp_val);
> +	if ((tmp_val & dw_readl(dws, DW_SPI_CTRLR0)) == tmp_val)
> +		mode |= SPI_TX_OCTAL | SPI_RX_OCTAL;

[Severity: High]
Can this writability test falsely detect Octal mode support on Quad-only
hardware?

If the SPI_FRF field is 2 bits wide, Quad mode would require both physical
flip-flops to exist. When the driver tests for Octal mode by writing a value
that sets both bits, the readback will confirm both bits are set. This would
hold the value and trick the driver into assuming the IP supports internal
Octal logic when it may only be synthesized for Quad.

> +
> +	if (!mode)
> +		goto disable_enh;
> +
> +	/* 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)
> +		/*
> +		 * If clock stretching is not enabled then do not use
> +		 * enhanced mode.
> +		 */
> +		goto disable_enh;

[Severity: High]
Does this error path leak the detected multi-lane modes?

When the clock stretching test fails, the code jumps to the disable_enh label
without clearing the local mode variable. The non-zero mode bitmask is then
returned and unconditionally bitwise-ORed into ctlr->mode_bits by the caller.

Could this falsely advertise multi-lane capabilities to the SPI subsystem
when the controller does not actually support enhanced mode?

> +
> +	dws->caps |= DW_SPI_CAP_EMODE;
> +
> +disable_enh:
> +	dw_writel(dws, DW_SPI_CTRLR0, tmp_ctrlr0);
> +	dw_writel(dws, DW_SPI_SPI_CTRLR0, tmp_spi_ctrlr0);
> +	dw_spi_enable_chip(dws, 1);
> +
> +	return mode;
> +}
[ ... ]
> @@ -1162,6 +1220,9 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
[ ... ]
>  		dws->caps |= DW_SPI_CAP_DFS32;
>  	}
>  
> +	dws->ctlr->mode_bits = SPI_CPOL | SPI_CPHA;

[Severity: High]
Does this direct assignment overwrite dynamically established capabilities?

During initial probe, dw_spi_add_controller() sets capabilities like:
    ctlr->mode_bits |= SPI_LOOP;

When the system resumes, dw_spi_resume_controller() calls dw_spi_hw_init():

drivers/spi/spi-dw-core.c:dw_spi_resume_controller() {
    ...
    dw_spi_hw_init(&dws->ctlr->dev, dws);
    ...
}

Will this blind assignment wipe out flags like SPI_LOOP after a
suspend/resume cycle?

> +	dws->ctlr->mode_bits |= detect_enh_mode(dws);

[Severity: High]
Does advertising these multi-bit SPI modes globally cause standard multi-lane
transfers to corrupt data?

By adding these multi-lane modes to the controller's mode_bits, they are
exposed to standard (non-mem_ops) SPI transfers. If a user-space application
or kernel driver initiates a standard spi_transfer with tx_nbits > 1, the SPI
core will accept it. 

However, looking at dw_spi_transfer_one(), the standard transfer handler
appears to ignore multi-lane requests and executes everything in 1-bit
standard mode:

drivers/spi/spi-dw-core.c:dw_spi_transfer_one() {
    ...
    .spi_frf = DW_SPI_CTRLR0_SPI_FRF_STD_SPI,
    ...
}

Will this result in data corruption and protocol mismatch on the SPI bus?

> +
>  	/* enable HW fixup for explicit CS deselect for Amazon's alpine chip */
>  	if (dws->caps & DW_SPI_CAP_CS_OVERRIDE)
>  		dw_writel(dws, DW_SPI_CS_OVERRIDE, 0xF);

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

[PATCH v1 10/11] spi: dt-bindings: snps,dw-apb-ssi: Add starfive,jhb100-sfc

[Changhuang Liang]

Add a new compatible string "starfive,jhb100-sfc" for the StarFive
JHB100 SPI Flash Controller, it based on the Synopsys DesignWare
SSI version 2.00a but with minor modifications.

Due to these minor modifications, it only supports access for flash
memory and requires a system controller register to configure the
address mode filter for SPI NOR flash devices.

The starfive,sfc-filter-syscon property is required to provide a phandle
to the system controller that manages switching between 3-byte and
4-byte addressing modes, essential for supporting SPI NOR flash devices
with different address width requirements.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index 4458316326fc..f13cb963db88 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -33,6 +33,17 @@ allOf:
     else:
       properties:
         amd,pensando-elba-syscon: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jhb100-sfc
+    then:
+      required:
+        - starfive,sfc-filter-syscon
+    else:
+      properties:
+        starfive,sfc-filter-syscon: false
 
 properties:
   compatible:
@@ -68,6 +79,8 @@ properties:
         const: amd,pensando-elba-spi
       - description: Canaan Kendryte K210 SoS SPI Controller
         const: canaan,k210-spi
+      - description: StarFive JHB100 SoC SPI Flash Controller
+        const: starfive,jhb100-sfc
       - description: Renesas RZ/N1 SPI Controller
         items:
           - const: renesas,r9a06g032-spi # RZ/N1D
@@ -139,6 +152,15 @@ properties:
       Block address to control SPI chip-selects. The Elba SoC system controller
       provides an interface to override the native DWC SSI CS control.
 
+  starfive,sfc-filter-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandle to the system controller register that controls the SPI NOR flash
+      address mode filter. This system controller interface provides additional
+      configuration to switch between 3-byte and 4-byte addressing modes, which
+      is required when accessing SPI NOR flash devices with different address
+      width requirements.
+
 patternProperties:
   "@[0-9a-f]$":
     type: object
-- 
2.25.1

Re: [PATCH v1 10/11] spi: dt-bindings: snps,dw-apb-ssi: Add starfive,jhb100-sfc

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

[PATCH v1 11/11] spi: dw: Add support for StarFive JHB100 SoC SFC

[Changhuang Liang]

Add support for the StarFive JHB100 SoC SPI Flash Controller (SFC),
which is based on the Synopsys DesignWare SSI version 2.00a but with
some customizations.

The JHB100 SFC controller has the following special features:
1. Separate registers for instruction and address (DW_SPI_JHB100_INST
   and DW_SPI_JHB100_ADDR) instead of using the common data register.
2. A filter interrupt mask register (DW_SPI_JHB100_FILTER_IMR),
   which is default masked to disable filter interrupts as they
   are not used.
3. Requires a system controller phandle "starfive,sfc-filter-syscon"
   to configure 3-byte/4-byte address mode switching per chip select.
4. Different Set CS and Enable Controller Timing.

A new quirk flag DW_SPI_QUIRK_JHB100 is introduced to handle these
differences in the enhanced SPI memory operation path. The controller
uses the HSSI initialization path (DW_HSSI_ID) and shares the same
interrupt masking logic.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/spi/spi-dw-core.c | 37 ++++++++++++++++++++++++----------
 drivers/spi/spi-dw-mmio.c | 42 +++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-dw.h      | 18 +++++++++++++++++
 3 files changed, 86 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 0abdee82eaab..d7698802019a 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -906,16 +906,30 @@ static void dw_spi_init_enh_mem_buf(struct dw_spi *dws, const struct spi_mem_op
 	}
 }
 
-static void dw_spi_enh_write_cmd_addr(struct dw_spi *dws, const struct spi_mem_op *op)
+static void dw_spi_enh_write_cmd_addr(struct dw_spi *dws, const struct spi_mem_op *op,
+				      struct spi_mem *mem)
 {
-	/* Send cmd as 32 bit value */
-	dw_write_io_reg(dws, DW_SPI_DR, op->cmd.opcode);
-	if (op->addr.nbytes) {
-		dw_write_io_reg(dws, DW_SPI_DR, lower_32_bits(op->addr.val));
-		if (op->addr.nbytes > 4) {
-			/* address more than 32bit */
-			dw_write_io_reg(dws, DW_SPI_DR, upper_32_bits(op->addr.val));
+	if (dws->quirk_flags & DW_SPI_QUIRK_JHB100) {
+		dw_write_io_reg(dws, DW_SPI_JHB100_INST, op->cmd.opcode);
+		if (op->addr.nbytes)
+			dw_write_io_reg(dws, DW_SPI_JHB100_ADDR, op->addr.val);
+
+		dw_spi_set_cs(mem->spi, false);
+		dw_spi_enable_chip(dws, 1);
+	} else {
+		dw_spi_enable_chip(dws, 1);
+
+		/* Send cmd as 32 bit value */
+		dw_write_io_reg(dws, DW_SPI_DR, op->cmd.opcode);
+		if (op->addr.nbytes) {
+			dw_write_io_reg(dws, DW_SPI_DR, lower_32_bits(op->addr.val));
+			if (op->addr.nbytes > 4) {
+				/* address more than 32bit */
+				dw_write_io_reg(dws, DW_SPI_DR, upper_32_bits(op->addr.val));
+			}
 		}
+
+		dw_spi_set_cs(mem->spi, false);
 	}
 }
 
@@ -979,10 +993,11 @@ static int dw_spi_exec_enh_mem_op(struct spi_mem *mem, const struct spi_mem_op *
 
 	dw_spi_mask_intr(dws, 0xff);
 	reinit_completion(&ctlr->xfer_completion);
-	dw_spi_enable_chip(dws, 1);
 
-	dw_spi_enh_write_cmd_addr(dws, op);
-	dw_spi_set_cs(mem->spi, false);
+	if (dws->set_addr_nbyte)
+		dws->set_addr_nbyte(mem->spi, op->addr.nbytes);
+
+	dw_spi_enh_write_cmd_addr(dws, op, mem);
 
 	udelay(5);
 
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 603e81a92c57..236ac5fa9cd0 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -48,6 +48,8 @@ struct dw_spi_mmio {
 #define SPARX5_FORCE_ENA			0xa4
 #define SPARX5_FORCE_VAL			0xa8
 
+#define JHB100_ADDRMODE_CS			0x00
+
 struct dw_spi_mscc {
 	struct regmap       *syscon;
 	void __iomem        *spi_mst; /* Not sparx5 */
@@ -310,6 +312,45 @@ static int dw_spi_elba_init(struct platform_device *pdev,
 	return 0;
 }
 
+static void dw_spi_jhb100_set_addr_nbyte(struct spi_device *spi, u8 nbyte)
+{
+	struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
+	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
+	struct regmap *syscon = dwsmmio->priv;
+
+	if (nbyte == 3) {
+		regmap_update_bits(syscon, JHB100_ADDRMODE_CS,
+				   BIT(spi_get_chipselect(spi, 0)),
+				   0);
+	} else if (nbyte == 4) {
+		regmap_update_bits(syscon, JHB100_ADDRMODE_CS,
+				   BIT(spi_get_chipselect(spi, 0)),
+				   BIT(spi_get_chipselect(spi, 0)));
+	}
+}
+
+static int dw_spi_jhb100_init(struct platform_device *pdev,
+			      struct dw_spi_mmio *dwsmmio)
+{
+	struct regmap *syscon;
+
+	syscon = syscon_regmap_lookup_by_phandle(dev_of_node(&pdev->dev),
+						 "starfive,sfc-filter-syscon");
+	if (IS_ERR(syscon))
+		return dev_err_probe(&pdev->dev, PTR_ERR(syscon),
+				     "syscon regmap lookup failed\n");
+
+	dwsmmio->priv = syscon;
+
+	dwsmmio->dws.set_addr_nbyte = dw_spi_jhb100_set_addr_nbyte;
+	dwsmmio->dws.ip = DW_HSSI_ID;
+	dwsmmio->dws.quirk_flags = DW_SPI_QUIRK_JHB100;
+
+	dw_spi_jhb100_mask_intr(&dwsmmio->dws, 0xff);
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	int (*init_func)(struct platform_device *pdev,
@@ -447,6 +488,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
 	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
 	{ .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
+	{ .compatible = "starfive,jhb100-sfc", .data = dw_spi_jhb100_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 2dae81c15423..a913c850ba1a 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -67,6 +67,11 @@
 #define DW_SPI_SPI_CTRLR0		0xf4
 #define DW_SPI_CS_OVERRIDE		0xf4
 
+/* Register offsets (StarFive JHB100 DWC SSI IP-cores) */
+#define DW_SPI_JHB100_INST		0x1000
+#define DW_SPI_JHB100_ADDR		0x1004
+#define DW_SPI_JHB100_FILTER_IMR	0x1008
+
 /* Bit fields in CTRLR0 (DWC APB SSI) */
 #define DW_PSSI_CTRLR0_DFS_MASK			GENMASK(3, 0)
 #define DW_PSSI_CTRLR0_DFS32_MASK		GENMASK(20, 16)
@@ -199,6 +204,7 @@ struct dw_spi {
 	u32			num_cs;		/* chip select lines */
 	u16			bus_num;
 	void (*set_cs)(struct spi_device *spi, bool enable);
+	void (*set_addr_nbyte)(struct spi_device *spi, u8 nbyte);
 
 	/* Current message transfer state info */
 	void			*tx;
@@ -228,6 +234,9 @@ struct dw_spi {
 	const struct dw_spi_dma_ops *dma_ops;
 	struct completion	dma_completion;
 
+#define DW_SPI_QUIRK_JHB100		BIT(0)
+	u32			quirk_flags;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs;
 	struct debugfs_regset32 regset;
@@ -296,6 +305,15 @@ static inline void dw_spi_umask_intr(struct dw_spi *dws, u32 mask)
 	dw_writel(dws, DW_SPI_IMR, new_mask);
 }
 
+/* Disable JHB100 SPI filter IRQ bits */
+static inline void dw_spi_jhb100_mask_intr(struct dw_spi *dws, u32 mask)
+{
+	u32 new_mask;
+
+	new_mask = dw_readl(dws, DW_SPI_JHB100_FILTER_IMR) & ~mask;
+	dw_writel(dws, DW_SPI_JHB100_FILTER_IMR, new_mask);
+}
+
 /*
  * This disables the SPI controller, interrupts, clears the interrupts status
  * and CS, then re-enables the controller back. Transmit and receive FIFO
-- 
2.25.1

Re: [PATCH v1 11/11] spi: dw: Add support for StarFive JHB100 SoC SFC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The JHB100 quirk does not properly validate or handle address lengths other than 3 or 4 bytes, leading to syscon state desynchronization and silent address truncation.
- [High] The JHB100 filter interrupt mask register is masked during initialization but is not restored on system resume.
--

--- Patch [11]: [PATCH v1 11/11] spi: dw: Add support for StarFive JHB100 SoC SFC ---
commit 3a943425e81f5dc20e515a24b6937c56ee3c08e8
Author: Changhuang Liang <changhuang.liang@starfivetech.com>

spi: dw: Add support for StarFive JHB100 SoC SFC

Add support for the StarFive JHB100 SoC SPI Flash Controller (SFC). It
introduces customizations over the Synopsys DW SSI v2.00a, including
separate instruction/address registers, a filter interrupt mask register,
and requires a syscon phandle for address mode switching.

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index ef1abb540f069..c4e53c7456b26 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
[ ... ]
> -static void dw_spi_enh_write_cmd_addr(struct dw_spi *dws, const struct spi_mem_op *op)
> +static void dw_spi_enh_write_cmd_addr(struct dw_spi *dws, const struct spi_mem_op *op,
> +				      struct spi_mem *mem)
>  {
> -	/* Send cmd as 32 bit value */
> -	dw_write_io_reg(dws, DW_SPI_DR, op->cmd.opcode);
> -	if (op->addr.nbytes) {
> -		dw_write_io_reg(dws, DW_SPI_DR, lower_32_bits(op->addr.val));
> -		if (op->addr.nbytes > 4) {
> -			/* address more than 32bit */
> -			dw_write_io_reg(dws, DW_SPI_DR, upper_32_bits(op->addr.val));
> +	if (dws->quirk_flags & DW_SPI_QUIRK_JHB100) {
> +		dw_write_io_reg(dws, DW_SPI_JHB100_INST, op->cmd.opcode);
> +		if (op->addr.nbytes)
> +			dw_write_io_reg(dws, DW_SPI_JHB100_ADDR, op->addr.val);

[Severity: High]
If op->addr.nbytes is greater than 4, does this 32-bit register write via
dw_write_io_reg() silently truncate the 64-bit op->addr.val address value?

The SPI core can issue mem_op structures with address lengths up to 7 bytes 
(which dw_spi_supports_enh_mem_op allows). Could standard SPI flash
operations utilizing these non-standard address lengths experience data
corruption or communication failure due to this silent dropping of upper bits?

[ ... ]
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 603e81a92c57a..236ac5fa9cd06 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
[ ... ]
> +static void dw_spi_jhb100_set_addr_nbyte(struct spi_device *spi, u8 nbyte)
> +{
> +	struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
> +	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
> +	struct regmap *syscon = dwsmmio->priv;
> +
> +	if (nbyte == 3) {
> +		regmap_update_bits(syscon, JHB100_ADDRMODE_CS,
> +				   BIT(spi_get_chipselect(spi, 0)),
> +				   0);
> +	} else if (nbyte == 4) {
> +		regmap_update_bits(syscon, JHB100_ADDRMODE_CS,
> +				   BIT(spi_get_chipselect(spi, 0)),
> +				   BIT(spi_get_chipselect(spi, 0)));
> +	}
> +}

[Severity: High]
What happens if nbyte is something other than 3 or 4 (like 1, 2, or 5)?

Does this silently ignore unsupported address lengths instead of rejecting
them, potentially leaving the syscon hardware in a stale or misconfigured
state during transfers?

> +
> +static int dw_spi_jhb100_init(struct platform_device *pdev,
> +			      struct dw_spi_mmio *dwsmmio)
> +{
> +	struct regmap *syscon;
> +
> +	syscon = syscon_regmap_lookup_by_phandle(dev_of_node(&pdev->dev),
> +						 "starfive,sfc-filter-syscon");
> +	if (IS_ERR(syscon))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(syscon),
> +				     "syscon regmap lookup failed\n");
> +
> +	dwsmmio->priv = syscon;
> +
> +	dwsmmio->dws.set_addr_nbyte = dw_spi_jhb100_set_addr_nbyte;
> +	dwsmmio->dws.ip = DW_HSSI_ID;
> +	dwsmmio->dws.quirk_flags = DW_SPI_QUIRK_JHB100;
> +
> +	dw_spi_jhb100_mask_intr(&dwsmmio->dws, 0xff);

[Severity: High]
Since this applies the interrupt mask during probe initialization, is there
corresponding logic needed to restore this register during PM resume?

If the SPI controller's power domain is lost during suspend,
DW_SPI_JHB100_FILTER_IMR might revert to its hardware reset value on resume.

The mmio suspend/resume path (dw_spi_mmio_resume) calls
dw_spi_resume_controller(), which ultimately executes dw_spi_hw_init(). 
However, that function resets standard registers and lacks any awareness of
this quirk-specific DW_SPI_JHB100_FILTER_IMR register.

If the default reset state leaves the filter interrupts unmasked, could this
trigger an unhandled interrupt storm locking up the CPU after the system
resumes?

> +
> +	return 0;
> +}

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