[PATCH 0/3] spi: cadence-xspi: add ACMD mode support

[PATCH 0/3] spi: cadence-xspi: add ACMD mode support

[Nuno Sá]

The Cadence xSPI driver currently drives every operation through STIG
mode, where the CPU builds each command and drains the data. The
controller also has an auto command (ACMD) mode where it runs whole
sequences on its own and can move data with its internal DMA, which
pays off for bulk reads.

Whether ACMD and the internal DMA can be used depends on the SoC
integration (external SRAM, the DMA bus requester interface being
wired), which the controller registers do not expose. Hence two new
devicetree properties: cdns,has-acmd and cdns,has-internal-dma.

For now, ACMD is only used for reads of at least 2048 bytes with an
address phase, using the internal DMA, and only when both properties
are set. Smaller reads and all other operations stay on STIG.

NOR and NAND are handled the same way: NAND operations are still issued
one by one, not combined into a single ACMD sequence, so no NAND
geometry is needed. The real gain is on large reads which, for NAND,
means chips supporting buffer/sequential read mode.

Another attempt for ACMD was sent in [1] but that relies on knowing the
NAND geometry which brings much more complexity given that it needs a
proper interface implemented in the spi-nand core. So we agreed on
starting with this simpler approach and then build on top of it if we
agree it pays off (mostly on cpu load).

Some stats:

ACMD
-----

SPI-NOR - is25wx01g (only reads > 2k are done in ACMD )
-------

flash_speed -c 5 -d /dev/mtd4
not NAND flash, assume page size is 512 bytes.
scanning for bad eraseblocks
scanned 5 eraseblocks, 0 are bad
testing eraseblock write speed
eraseblock write speed is 1054 KiB/s
testing eraseblock read speed
eraseblock read speed is 106666 KiB/s
testing page write speed
page write speed is 1035 KiB/s
testing page read speed
page read speed is 15238 KiB/s

SPI-NAND - w25n08lw (all reads are ACMD given that page size is 4k)
--------

flash_speed -c 5 -d /dev/mtd5
scanning for bad eraseblocks
scanned 5 eraseblocks, 0 are bad
testing eraseblock write speed
eraseblock write speed is 6497 KiB/s
testing eraseblock read speed
eraseblock read speed is 44137 KiB/s
testing page write speed
page write speed is 6336 KiB/s
testing page read speed
page read speed is 16842 KiB/s

STIG
----

SPI-NOR
-------

flash_speed -c 5 -d /dev/mtd4
not NAND flash, assume page size is 512 bytes.
scanning for bad eraseblocks
scanned 5 eraseblocks, 0 are bad
testing eraseblock write speed
eraseblock write speed is 1054 KiB/s
testing eraseblock read speed
eraseblock read speed is 64000 KiB/s
testing page write speed
page write speed is 1042 KiB/s
testing page read speed
page read speed is 14883 KiB/s

SPI-NAND (Cont mode)
--------

flash_speed -c 5 -d /dev/mtd5
scanning for bad eraseblocks
scanned 5 eraseblocks, 0 are bad
testing eraseblock write speed
eraseblock write speed is 6564 KiB/s
testing eraseblock read speed
eraseblock read speed is 18823 KiB/s
testing page write speed
page write speed is 6400 KiB/s
testing page read speed
page read speed is 18028 KiB/s

For now, we have a static threshold to decide to use ACMD or not. I do
understand that's not ideal. It might depend on sclk and number of lanes
so not sure if that value will be accurate for everybody. So, open to
suggestions if someone has something in mind regarding this.

[1]: https://lore.kernel.org/linux-spi/20260921093701.1341766-1-fei.xie@horizon.auto/

---
Nuno Sá (3):
      spi: cadence-xspi: rename sdma_error to xfer_error
      spi: dt-bindings: cdns,xspi: add ACMD and internal DMA properties
      spi: cadence-xspi: add ACMD mode support

 .../devicetree/bindings/spi/cdns,xspi.yaml         |  18 ++
 drivers/spi/spi-cadence-xspi.c                     | 269 +++++++++++++++++++--
 2 files changed, 273 insertions(+), 14 deletions(-)
---
base-commit: 7da2aca123754c0e06092bbe179d58078b58d8d4
change-id: 20261027-xspi-cadence-add-acmd-3c0e342cb4f4
--

Thanks!
- Nuno Sá

[PATCH 1/3] spi: cadence-xspi: rename sdma_error to xfer_error

[Nuno Sá]

Rename the sdma_error flag to xfer_error given that a following patch
will support ACMD mode. And so, let's keep the name more generic.

No functional change intended.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/spi/spi-cadence-xspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 3687853e4ce4..905292fa50a2 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -338,7 +338,7 @@ struct cdns_xspi_dev {
 	struct completion cmd_complete;
 	struct completion auto_cmd_complete;
 	struct completion sdma_complete;
-	bool sdma_error;
+	bool xfer_error;
 
 	void *in_buffer;
 	const void *out_buffer;
@@ -544,7 +544,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
 
 	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
-	cdns_xspi->sdma_error = false;
+	cdns_xspi->xfer_error = false;
 
 	memset(cmd_regs, 0, sizeof(cmd_regs));
 	cmd_regs[1] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase);
@@ -575,7 +575,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 		cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
 
 		wait_for_completion(&cdns_xspi->sdma_complete);
-		if (cdns_xspi->sdma_error) {
+		if (cdns_xspi->xfer_error) {
 			cdns_xspi->set_interrupts_handler(cdns_xspi, false);
 			return -EIO;
 		}
@@ -703,7 +703,7 @@ static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)
 		if (irq_status & CDNS_XSPI_SDMA_ERROR) {
 			dev_err(cdns_xspi->dev,
 				"Slave DMA transaction error\n");
-			cdns_xspi->sdma_error = true;
+			cdns_xspi->xfer_error = true;
 			complete(&cdns_xspi->sdma_complete);
 		}
 

-- 
2.56.0

[PATCH 2/3] spi: dt-bindings: cdns,xspi: add ACMD and internal DMA properties

[Nuno Sá]

The Cadence xSPI controller supports an auto command (ACMD) mode where
high level sequences (read, program, erase) are handled by the
controller and data can be moved by its internal DMA. Whether ACMD can
be used, and whether the internal DMA bus requester interface is wired,
depends on how the IP is integrated (e.g. external SRAM, interconnect
wiring) and cannot be discovered from the controller.

Add the cdns,has-acmd and cdns,has-internal-dma boolean properties to
describe that. The internal DMA is only used by ACMD, so properly
reflect that.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index 38a5795589de..9708ede79730 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -40,11 +40,29 @@ properties:
   interrupts:
     maxItems: 1
 
+  cdns,has-acmd:
+    type: boolean
+    description:
+      The controller supports the auto command (ACMD) mode. The IP may need
+      external hardware (like SRAM) at SoC integration to fully leverage
+      ACMD.
+
+  cdns,has-internal-dma:
+    type: boolean
+    description:
+      The controller internal DMA bus requester interface is wired at SoC
+      integration, so the controller can move ACMD data to and from memory
+      on its own.
+
 required:
   - compatible
   - reg
   - interrupts
 
+dependencies:
+  cdns,has-internal-dma:
+    - cdns,has-acmd
+
 allOf:
   - $ref: spi-controller.yaml#
   - if:

-- 
2.56.0

Re: [PATCH 2/3] spi: dt-bindings: cdns,xspi: add ACMD and internal DMA properties

[Conor Dooley]

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

On Thu, Oct 29, 2026 at 04:03:17PM +0000, Nuno Sá wrote:
> The Cadence xSPI controller supports an auto command (ACMD) mode where
> high level sequences (read, program, erase) are handled by the
> controller and data can be moved by its internal DMA. Whether ACMD can
> be used, and whether the internal DMA bus requester interface is wired,
> depends on how the IP is integrated (e.g. external SRAM, interconnect
> wiring) and cannot be discovered from the controller.
> 
> Add the cdns,has-acmd and cdns,has-internal-dma boolean properties to
> describe that. The internal DMA is only used by ACMD, so properly
> reflect that.

Why would these properties be needed? What the controller support surely
can be determined from the specific compatible for the SoC integration?

> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index 38a5795589de..9708ede79730 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -40,11 +40,29 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  cdns,has-acmd:
> +    type: boolean
> +    description:
> +      The controller supports the auto command (ACMD) mode. The IP may need
> +      external hardware (like SRAM) at SoC integration to fully leverage
> +      ACMD.
> +
> +  cdns,has-internal-dma:
> +    type: boolean
> +    description:
> +      The controller internal DMA bus requester interface is wired at SoC
> +      integration, so the controller can move ACMD data to and from memory
> +      on its own.
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
>  
> +dependencies:
> +  cdns,has-internal-dma:
> +    - cdns,has-acmd
> +
>  allOf:
>    - $ref: spi-controller.yaml#
>    - if:
> 
> -- 
> 2.56.0
> 

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

[PATCH 3/3] spi: cadence-xspi: add ACMD mode support

[Nuno Sá]

So far every operation goes through STIG mode, where the CPU builds
each command and drains the data. The controller also has an auto
command (ACMD) mode where it runs a whole read sequence on its own and,
with the internal DMA, writes the data straight to memory.

Use ACMD with the internal DMA for reads with an address phase of at
least CDNS_XSPI_ACMD_MIN_BYTES (2048). Below that, the per-command setup
costs more than the saved PIO drain. Smaller reads and all other
operations stay on STIG. Such reads are not clamped to the SDMA size
anymore, since ACMD does not go through the SDMA port.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/spi/spi-cadence-xspi.c | 261 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 251 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 905292fa50a2..b2c1ba86fa14 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -4,6 +4,7 @@
 
 #include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
@@ -63,6 +64,7 @@
 #define CDNS_XSPI_INIT_LEGACY			BIT(9)
 #define CDNS_XSPI_INIT_FAIL			BIT(8)
 #define CDNS_XSPI_CTRL_BUSY			BIT(7)
+#define CDNS_XSPI_ACMD_ENGINE_BUSY		BIT(2)
 
 /* Controller interrupt status register */
 #define CDNS_XSPI_INTR_STATUS_REG		0x0110
@@ -103,6 +105,44 @@
 #define CDNS_XSPI_SDMA_TRD_INFO_REG		0x0244
 #define CDNS_XSPI_SDMA_DIR			BIT(8)
 
+#define CDNS_XSPI_ACMD_THRD_0			0
+#define CDNS_XSPI_ACMD_THRD_0_MASK		BIT(CDNS_XSPI_ACMD_THRD_0)
+
+/* Command status pointer register, selects the thread CMD_STAT refers to */
+#define CDNS_XSPI_CMD_STATUS_PTR_REG		0x0040
+#define CDNS_XSPI_CMD_STATUS_PTR_SEL		GENMASK(2, 0)
+
+/* CMD0 layout for ACMD PIO sequences */
+#define CDNS_XSPI_ACMD_MODE			GENMASK(31, 30)
+#define CDNS_XSPI_ACMD_MODE_PIO			1
+#define CDNS_XSPI_ACMD_TRD_NUM			GENMASK(26, 24)
+#define CDNS_XSPI_ACMD_BANK			GENMASK(22, 20)
+#define CDNS_XSPI_ACMD_DMA_SEL			BIT(19)
+#define CDNS_XSPI_ACMD_INT			BIT(18)
+#define CDNS_XSPI_ACMD_CMD_TYPE			GENMASK(15, 0)
+
+/* ACMD sequence types. Any other value is reserved. */
+#define CDNS_XSPI_ACMD_TYPE_READ		0x2200
+
+/* Common sequence configuration */
+#define CDNS_XSPI_SEQ_GCTL0_REG			0x0390
+#define CDNS_XSPI_SEQ_GCTL0_TYP			GENMASK(24, 23)
+#define CDNS_XSPI_SEQ_GCTL0_TYP_PROFILE_1	0
+#define CDNS_XSPI_SEQ_GCTL0_PSIZ_RD		GENMASK(3, 0)
+#define CDNS_XSPI_SEQ_GCTL0_PSIZ_RD_UNLIMITED	0xf
+
+/* Read sequence configuration */
+#define CDNS_XSPI_READ_SEQ_CFG0_REG		0x0430
+#define CDNS_XSPI_READ_SEQ_CFG0_CMD_VALUE	GENMASK(7, 0)
+#define CDNS_XSPI_READ_SEQ_CFG0_CMD_IOS		GENMASK(9, 8)
+#define CDNS_XSPI_READ_SEQ_CFG0_ADDR_CNT	GENMASK(14, 12)
+#define CDNS_XSPI_READ_SEQ_CFG0_ADDR_IOS	GENMASK(17, 16)
+#define CDNS_XSPI_READ_SEQ_CFG0_DAT_IOS		GENMASK(21, 20)
+#define CDNS_XSPI_READ_SEQ_CFG0_DMY_CNT		GENMASK(29, 24)
+#define CDNS_XSPI_READ_SEQ_CFG1_REG		0x0434
+
+#define CDNS_XSPI_ACMD_MIN_BYTES		2048
+
 /* Controller features register */
 #define CDNS_XSPI_CTRL_FEATURES_REG		0x0F04
 #define CDNS_XSPI_NUM_BANKS			GENMASK(25, 24)
@@ -139,6 +179,8 @@
 /* STIG command status fields */
 #define CDNS_XSPI_CMD_STATUS_COMPLETED		BIT(15)
 #define CDNS_XSPI_CMD_STATUS_FAILED		BIT(14)
+/* Bit 4 is defined in ACMD mode only */
+#define CDNS_XSPI_CMD_STATUS_DEVICE_ERROR	BIT(4)
 #define CDNS_XSPI_CMD_STATUS_DQS_ERROR		BIT(3)
 #define CDNS_XSPI_CMD_STATUS_CRC_ERROR		BIT(2)
 #define CDNS_XSPI_CMD_STATUS_BUS_ERROR		BIT(1)
@@ -335,6 +377,9 @@ struct cdns_xspi_dev {
 	unsigned int sdmasize;
 	unsigned int sdma_xfer_len;
 
+	int cur_work_mode;
+	bool has_acmd;
+
 	struct completion cmd_complete;
 	struct completion auto_cmd_complete;
 	struct completion sdma_complete;
@@ -367,6 +412,18 @@ static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
 					  100, 1000);
 }
 
+static int cdns_xspi_wait_for_acmd_idle(struct cdns_xspi_dev *cdns_xspi)
+{
+	u32 ctrl_status;
+
+	return readl_relaxed_poll_timeout(cdns_xspi->iobase +
+					  CDNS_XSPI_CTRL_STATUS_REG,
+					  ctrl_status,
+					  ((ctrl_status &
+					    CDNS_XSPI_ACMD_ENGINE_BUSY) == 0),
+					  100, 1000);
+}
+
 static void cdns_xspi_trigger_command(struct cdns_xspi_dev *cdns_xspi,
 				      u32 cmd_regs[6])
 {
@@ -378,13 +435,19 @@ static void cdns_xspi_trigger_command(struct cdns_xspi_dev *cdns_xspi,
 	writel(cmd_regs[0], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
 }
 
-static int cdns_xspi_check_command_status(struct cdns_xspi_dev *cdns_xspi)
+static int cdns_xspi_check_command_status(struct cdns_xspi_dev *cdns_xspi,
+					  bool acmd)
 {
 	int ret = 0;
 	u32 cmd_status = readl(cdns_xspi->iobase + CDNS_XSPI_CMD_STATUS_REG);
 
 	if (cmd_status & CDNS_XSPI_CMD_STATUS_COMPLETED) {
 		if ((cmd_status & CDNS_XSPI_CMD_STATUS_FAILED) != 0) {
+			if (acmd && cmd_status & CDNS_XSPI_CMD_STATUS_DEVICE_ERROR) {
+				dev_err(cdns_xspi->dev,
+					"Device reported an error\n");
+				ret = -EPROTO;
+			}
 			if (cmd_status & CDNS_XSPI_CMD_STATUS_DQS_ERROR) {
 				dev_err(cdns_xspi->dev,
 					"Incorrect DQS pulses detected\n");
@@ -427,6 +490,39 @@ static void cdns_xspi_set_interrupts(struct cdns_xspi_dev *cdns_xspi,
 	writel(intr_enable, cdns_xspi->iobase + CDNS_XSPI_INTR_ENABLE_REG);
 }
 
+static void cdns_xspi_set_thread_error_interrupt(struct cdns_xspi_dev *cdns_xspi,
+						 bool enabled)
+{
+	u32 intr_enable;
+
+	intr_enable = readl(cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_EN);
+	if (enabled)
+		intr_enable |= CDNS_XSPI_ACMD_THRD_0_MASK;
+	else
+		intr_enable &= ~CDNS_XSPI_ACMD_THRD_0_MASK;
+	writel(intr_enable, cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_EN);
+}
+
+static void cdns_xspi_acmd_init(struct cdns_xspi_dev *cdns_xspi)
+{
+	u32 seq_cfg;
+
+	/*
+	 * For now, ACMD relies on the profile 1 sequence with no read page
+	 * limit (so SPI NAND continuous reads work). The read is fully
+	 * described by READ_SEQ_CFG0: no mode bits (they replace its dummy
+	 * count) nor opcode extension.
+	 */
+	seq_cfg = readl(cdns_xspi->iobase + CDNS_XSPI_SEQ_GCTL0_REG);
+	FIELD_MODIFY(CDNS_XSPI_SEQ_GCTL0_TYP, &seq_cfg,
+		     CDNS_XSPI_SEQ_GCTL0_TYP_PROFILE_1);
+	FIELD_MODIFY(CDNS_XSPI_SEQ_GCTL0_PSIZ_RD, &seq_cfg,
+		     CDNS_XSPI_SEQ_GCTL0_PSIZ_RD_UNLIMITED);
+	writel(seq_cfg, cdns_xspi->iobase + CDNS_XSPI_SEQ_GCTL0_REG);
+
+	writel(0, cdns_xspi->iobase + CDNS_XSPI_READ_SEQ_CFG1_REG);
+}
+
 static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
 {
 	u32 ctrl_ver;
@@ -447,6 +543,9 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
 	cdns_xspi->dma_data_width = (ctrl_features & CDNS_XSPI_DMA_DATA_WIDTH) ? 8 : 4;
 	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
 
+	if (cdns_xspi->has_acmd)
+		cdns_xspi_acmd_init(cdns_xspi);
+
 	return 0;
 }
 
@@ -527,6 +626,24 @@ static int cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
 	return 0;
 }
 
+static int cdns_xspi_set_work_mode(struct cdns_xspi_dev *cdns_xspi, int mode)
+{
+	int ret;
+
+	if (cdns_xspi->cur_work_mode == mode)
+		return 0;
+
+	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
+	if (ret < 0)
+		return ret;
+
+	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, mode),
+	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+	cdns_xspi->cur_work_mode = mode;
+
+	return 0;
+}
+
 static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 				       const struct spi_mem_op *op,
 				       bool data_phase)
@@ -536,12 +653,9 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 	int ret;
 	int dummybytes = op->dummy.nbytes;
 
-	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
-	if (ret < 0)
-		return -EIO;
-
-	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
-	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+	ret = cdns_xspi_set_work_mode(cdns_xspi, CDNS_XSPI_WORK_MODE_STIG);
+	if (ret)
+		return ret;
 
 	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
 	cdns_xspi->xfer_error = false;
@@ -589,13 +703,112 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 	wait_for_completion(&cdns_xspi->cmd_complete);
 	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
 
-	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
+	cmd_status = cdns_xspi_check_command_status(cdns_xspi, false);
 	if (cmd_status)
 		return -EPROTO;
 
 	return 0;
 }
 
+static void cdns_xspi_acmd_setup_read_seq(struct cdns_xspi_dev *cdns_xspi,
+					  const struct spi_mem_op *op)
+{
+	u32 cfg0;
+
+	cfg0 = FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_CMD_VALUE, op->cmd.opcode) |
+	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_CMD_IOS,
+			  ilog2(op->cmd.buswidth)) |
+	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_ADDR_CNT, op->addr.nbytes) |
+	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_ADDR_IOS,
+			  ilog2(op->addr.buswidth)) |
+	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_DAT_IOS,
+			  ilog2(op->data.buswidth));
+
+	if (op->dummy.nbytes && op->dummy.buswidth)
+		cfg0 |= FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_DMY_CNT,
+				   (op->dummy.nbytes * 8) / op->dummy.buswidth);
+
+	writel(cfg0, cdns_xspi->iobase + CDNS_XSPI_READ_SEQ_CFG0_REG);
+}
+
+static int cdns_xspi_acmd_check_status(struct cdns_xspi_dev *cdns_xspi)
+{
+	writel(FIELD_PREP(CDNS_XSPI_CMD_STATUS_PTR_SEL, CDNS_XSPI_ACMD_THRD_0),
+	       cdns_xspi->iobase + CDNS_XSPI_CMD_STATUS_PTR_REG);
+
+	return cdns_xspi_check_command_status(cdns_xspi, true);
+}
+
+static int cdns_xspi_acmd_read(struct cdns_xspi_dev *cdns_xspi,
+			       const struct spi_mem_op *op)
+{
+	dma_addr_t dma_addr;
+	u32 cmd_regs[6];
+	int ret;
+
+	ret = cdns_xspi_set_work_mode(cdns_xspi, CDNS_XSPI_WORK_MODE_ACMD);
+	if (ret)
+		return ret;
+
+	ret = cdns_xspi_wait_for_acmd_idle(cdns_xspi);
+	if (ret)
+		return ret;
+
+	cdns_xspi_acmd_setup_read_seq(cdns_xspi, op);
+
+	dma_addr = dma_map_single(cdns_xspi->dev, op->data.buf.in,
+				  op->data.nbytes, DMA_FROM_DEVICE);
+	if (dma_mapping_error(cdns_xspi->dev, dma_addr))
+		return -ENOMEM;
+
+	cdns_xspi->xfer_error = false;
+	writel(CDNS_XSPI_ACMD_THRD_0_MASK,
+	       cdns_xspi->iobase + CDNS_XSPI_TRD_COMP_INTR_STATUS);
+	writel(CDNS_XSPI_ACMD_THRD_0_MASK,
+	       cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_STATUS);
+	cdns_xspi_set_thread_error_interrupt(cdns_xspi, true);
+	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
+
+	cmd_regs[1] = lower_32_bits(op->addr.val);
+	cmd_regs[5] = upper_32_bits(op->addr.val);
+	cmd_regs[2] = lower_32_bits(dma_addr);
+	cmd_regs[3] = upper_32_bits(dma_addr);
+	cmd_regs[4] = op->data.nbytes - 1;
+	cmd_regs[0] = FIELD_PREP(CDNS_XSPI_ACMD_MODE,
+				 CDNS_XSPI_ACMD_MODE_PIO) |
+		      FIELD_PREP(CDNS_XSPI_ACMD_TRD_NUM, CDNS_XSPI_ACMD_THRD_0) |
+		      FIELD_PREP(CDNS_XSPI_ACMD_BANK, cdns_xspi->cur_cs) |
+		      CDNS_XSPI_ACMD_DMA_SEL | CDNS_XSPI_ACMD_INT |
+		      FIELD_PREP(CDNS_XSPI_ACMD_CMD_TYPE,
+				 CDNS_XSPI_ACMD_TYPE_READ);
+
+	reinit_completion(&cdns_xspi->auto_cmd_complete);
+	cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
+	wait_for_completion(&cdns_xspi->auto_cmd_complete);
+
+	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
+	cdns_xspi_set_thread_error_interrupt(cdns_xspi, false);
+	dma_unmap_single(cdns_xspi->dev, dma_addr, op->data.nbytes,
+			 DMA_FROM_DEVICE);
+
+	if (cdns_xspi->xfer_error)
+		return -EIO;
+
+	return cdns_xspi_acmd_check_status(cdns_xspi);
+}
+
+static bool cdns_xspi_acmd_can_read(const struct cdns_xspi_dev *cdns_xspi,
+				    const struct spi_mem_op *op)
+{
+	if (!cdns_xspi->has_acmd)
+		return false;
+
+	if (op->data.dir != SPI_MEM_DATA_IN || !op->addr.nbytes)
+		return false;
+
+	return op->data.nbytes >= CDNS_XSPI_ACMD_MIN_BYTES;
+}
+
 static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
 			    struct spi_mem *mem,
 			    const struct spi_mem_op *op)
@@ -605,6 +818,9 @@ static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
 	if (cdns_xspi->cur_cs != spi_get_chipselect(mem->spi, 0))
 		cdns_xspi->cur_cs = spi_get_chipselect(mem->spi, 0);
 
+	if (cdns_xspi_acmd_can_read(cdns_xspi, op))
+		return cdns_xspi_acmd_read(cdns_xspi, op);
+
 	return cdns_xspi_send_stig_command(cdns_xspi, op,
 					   (dir != SPI_MEM_NO_DATA));
 }
@@ -677,6 +893,9 @@ static int cdns_xspi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *
 	struct cdns_xspi_dev *cdns_xspi =
 		spi_controller_get_devdata(mem->spi->controller);
 
+	if (cdns_xspi_acmd_can_read(cdns_xspi, op))
+		return 0;
+
 	op->data.nbytes = clamp_val(op->data.nbytes, 0, cdns_xspi->sdmasize);
 
 	return 0;
@@ -691,8 +910,9 @@ static const struct spi_controller_mem_ops cadence_xspi_mem_ops = {
 static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)
 {
 	struct cdns_xspi_dev *cdns_xspi = dev;
-	u32 irq_status;
+	bool acmd_done = false;
 	irqreturn_t result = IRQ_NONE;
+	u32 irq_status;
 
 	irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
 	writel(irq_status, cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
@@ -721,11 +941,28 @@ static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)
 		writel(irq_status,
 		       cdns_xspi->iobase + CDNS_XSPI_TRD_COMP_INTR_STATUS);
 
-		complete(&cdns_xspi->auto_cmd_complete);
+		if (irq_status & CDNS_XSPI_ACMD_THRD_0_MASK)
+			acmd_done = true;
 
 		result = IRQ_HANDLED;
 	}
 
+	irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_STATUS);
+	if (irq_status) {
+		writel(irq_status,
+		       cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_STATUS);
+
+		if (irq_status & CDNS_XSPI_ACMD_THRD_0_MASK) {
+			cdns_xspi->xfer_error = true;
+			acmd_done = true;
+		}
+
+		result = IRQ_HANDLED;
+	}
+
+	if (acmd_done)
+		complete(&cdns_xspi->auto_cmd_complete);
+
 	return result;
 }
 
@@ -1258,6 +1495,10 @@ static int cdns_xspi_probe(struct platform_device *pdev)
 	if (ret)
 		return -ENODEV;
 
+	/* For now, ACMD is only supported together with the internal DMA */
+	cdns_xspi->has_acmd = device_property_read_bool(dev, "cdns,has-acmd") &&
+			      device_property_read_bool(dev, "cdns,has-internal-dma");
+
 	cdns_xspi->iobase = devm_platform_ioremap_resource_byname(pdev, "io");
 	if (IS_ERR(cdns_xspi->iobase)) {
 		cdns_xspi->iobase = devm_platform_ioremap_resource(pdev, 0);

-- 
2.56.0

Re: [PATCH 3/3] spi: cadence-xspi: add ACMD mode support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] spi: cadence-xspi: restore work mode and ACMD config in cdns_xspi_resume()
- [Medium] spi: cadence-xspi: xfer_error check in cdns_xspi_acmd_read() bypasses status check
- [Medium] spi: cadence-xspi: missing STIG mode in cdns_xspi_transfer_one_message_b0()
--

commit db673b09285a06730d352ef002e9477417c0c965
Author: Nuno Sá <nuno.sa@analog.com>

spi: cadence-xspi: add ACMD mode support

This commit adds auto command (ACMD) mode support using the internal DMA for
SPI memory read operations of at least 2048 bytes that include an address
phase. Smaller reads and other operations continue to use STIG mode.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 905292fa50a25..b2c1ba86fa149 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c

[ ... ]

> @@ -378,13 +435,19 @@ static void cdns_xspi_trigger_command(struct cdns_xspi_dev *cdns_xspi,
>  	writel(cmd_regs[0], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
>  }
>  
> -static int cdns_xspi_check_command_status(struct cdns_xspi_dev *cdns_xspi)
> +static int cdns_xspi_check_command_status(struct cdns_xspi_dev *cdns_xspi,
> +					  bool acmd)
>  {
>  	int ret = 0;
>  	u32 cmd_status = readl(cdns_xspi->iobase + CDNS_XSPI_CMD_STATUS_REG);
>  
>  	if (cmd_status & CDNS_XSPI_CMD_STATUS_COMPLETED) {
>  		if ((cmd_status & CDNS_XSPI_CMD_STATUS_FAILED) != 0) {
> +			if (acmd && cmd_status & CDNS_XSPI_CMD_STATUS_DEVICE_ERROR) {
> +				dev_err(cdns_xspi->dev,
> +					"Device reported an error\n");
> +				ret = -EPROTO;
> +			}
>  			if (cmd_status & CDNS_XSPI_CMD_STATUS_DQS_ERROR) {
>  				dev_err(cdns_xspi->dev,
>  					"Incorrect DQS pulses detected\n");

[ ... ]

> @@ -447,6 +543,9 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
>  	cdns_xspi->dma_data_width = (ctrl_features & CDNS_XSPI_DMA_DATA_WIDTH) ? 8 : 4;
>  	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
>  
> +	if (cdns_xspi->has_acmd)
> +		cdns_xspi_acmd_init(cdns_xspi);
> +
>  	return 0;
>  }
>  
> @@ -527,6 +626,24 @@ static int cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
>  	return 0;
>  }
>  
> +static int cdns_xspi_set_work_mode(struct cdns_xspi_dev *cdns_xspi, int mode)
> +{
> +	int ret;
> +
> +	if (cdns_xspi->cur_work_mode == mode)
> +		return 0;
> +
> +	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, mode),
> +	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
> +	cdns_xspi->cur_work_mode = mode;
> +
> +	return 0;
> +}

[Severity: High]
What happens across a system suspend and resume cycle when
cdns_xspi->cur_work_mode has been cached as CDNS_XSPI_WORK_MODE_STIG or
CDNS_XSPI_WORK_MODE_ACMD?

Previously, cdns_xspi_send_stig_command() wrote CDNS_XSPI_CTRL_CONFIG_REG
unconditionally on every command. With cdns_xspi_set_work_mode() skipping the
register write when cdns_xspi->cur_work_mode == mode, and cdns_xspi_acmd_init()
only running in cdns_xspi_controller_init() during probe, a controller reset
during system suspend will leave CDNS_XSPI_CTRL_CONFIG_REG in
CDNS_XSPI_WORK_MODE_DIRECT (0) and clear CDNS_XSPI_SEQ_GCTL0_REG and
CDNS_XSPI_READ_SEQ_CFG1_REG.

Looking at cdns_xspi_resume() in drivers/spi/spi-cadence-xspi.c:

static int cdns_xspi_resume(struct device *dev)
{
	struct cdns_xspi_dev *cdns_xspi = dev_get_drvdata(dev);

#ifdef CONFIG_64BIT
	if (cdns_xspi->driver_data->mrvl_hw_overlay) {
		cdns_mrvl_xspi_setup_clock(cdns_xspi, MRVL_DEFAULT_CLK);
		cdns_xspi_configure_phy(cdns_xspi);
	}
#endif

	cdns_xspi->set_interrupts_handler(cdns_xspi, false);

	return spi_controller_resume(cdns_xspi->host);
}

Should cdns_xspi_resume() reset cdns_xspi->cur_work_mode to
CDNS_XSPI_WORK_MODE_DIRECT and call cdns_xspi_acmd_init() when
cdns_xspi->has_acmd is set so that post-resume SPI operations do not skip
reprogramming CDNS_XSPI_CTRL_CONFIG_REG and the ACMD sequence registers?

> +
>  static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  				       const struct spi_mem_op *op,
>  				       bool data_phase)
> @@ -536,12 +653,9 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  	int ret;
>  	int dummybytes = op->dummy.nbytes;
>  
> -	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
> -	if (ret < 0)
> -		return -EIO;
> -
> -	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
> -	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
> +	ret = cdns_xspi_set_work_mode(cdns_xspi, CDNS_XSPI_WORK_MODE_STIG);
> +	if (ret)
> +		return ret;
>  
>  	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
>  	cdns_xspi->xfer_error = false;
> @@ -589,13 +703,112 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  	wait_for_completion(&cdns_xspi->cmd_complete);
>  	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
>  
> -	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
> +	cmd_status = cdns_xspi_check_command_status(cdns_xspi, false);
>  	if (cmd_status)
>  		return -EPROTO;
>  
>  	return 0;
>  }
>  
> +static void cdns_xspi_acmd_setup_read_seq(struct cdns_xspi_dev *cdns_xspi,
> +					  const struct spi_mem_op *op)
> +{
> +	u32 cfg0;
> +
> +	cfg0 = FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_CMD_VALUE, op->cmd.opcode) |
> +	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_CMD_IOS,
> +			  ilog2(op->cmd.buswidth)) |
> +	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_ADDR_CNT, op->addr.nbytes) |
> +	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_ADDR_IOS,
> +			  ilog2(op->addr.buswidth)) |
> +	       FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_DAT_IOS,
> +			  ilog2(op->data.buswidth));
> +
> +	if (op->dummy.nbytes && op->dummy.buswidth)
> +		cfg0 |= FIELD_PREP(CDNS_XSPI_READ_SEQ_CFG0_DMY_CNT,
> +				   (op->dummy.nbytes * 8) / op->dummy.buswidth);
> +
> +	writel(cfg0, cdns_xspi->iobase + CDNS_XSPI_READ_SEQ_CFG0_REG);
> +}
> +
> +static int cdns_xspi_acmd_check_status(struct cdns_xspi_dev *cdns_xspi)
> +{
> +	writel(FIELD_PREP(CDNS_XSPI_CMD_STATUS_PTR_SEL, CDNS_XSPI_ACMD_THRD_0),
> +	       cdns_xspi->iobase + CDNS_XSPI_CMD_STATUS_PTR_REG);
> +
> +	return cdns_xspi_check_command_status(cdns_xspi, true);
> +}
> +
> +static int cdns_xspi_acmd_read(struct cdns_xspi_dev *cdns_xspi,
> +			       const struct spi_mem_op *op)
> +{
> +	dma_addr_t dma_addr;
> +	u32 cmd_regs[6];
> +	int ret;
> +
> +	ret = cdns_xspi_set_work_mode(cdns_xspi, CDNS_XSPI_WORK_MODE_ACMD);
> +	if (ret)
> +		return ret;
> +
> +	ret = cdns_xspi_wait_for_acmd_idle(cdns_xspi);
> +	if (ret)
> +		return ret;
> +
> +	cdns_xspi_acmd_setup_read_seq(cdns_xspi, op);
> +
> +	dma_addr = dma_map_single(cdns_xspi->dev, op->data.buf.in,
> +				  op->data.nbytes, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(cdns_xspi->dev, dma_addr))
> +		return -ENOMEM;
> +
> +	cdns_xspi->xfer_error = false;
> +	writel(CDNS_XSPI_ACMD_THRD_0_MASK,
> +	       cdns_xspi->iobase + CDNS_XSPI_TRD_COMP_INTR_STATUS);
> +	writel(CDNS_XSPI_ACMD_THRD_0_MASK,
> +	       cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_STATUS);
> +	cdns_xspi_set_thread_error_interrupt(cdns_xspi, true);
> +	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
> +
> +	cmd_regs[1] = lower_32_bits(op->addr.val);
> +	cmd_regs[5] = upper_32_bits(op->addr.val);
> +	cmd_regs[2] = lower_32_bits(dma_addr);
> +	cmd_regs[3] = upper_32_bits(dma_addr);
> +	cmd_regs[4] = op->data.nbytes - 1;
> +	cmd_regs[0] = FIELD_PREP(CDNS_XSPI_ACMD_MODE,
> +				 CDNS_XSPI_ACMD_MODE_PIO) |
> +		      FIELD_PREP(CDNS_XSPI_ACMD_TRD_NUM, CDNS_XSPI_ACMD_THRD_0) |
> +		      FIELD_PREP(CDNS_XSPI_ACMD_BANK, cdns_xspi->cur_cs) |
> +		      CDNS_XSPI_ACMD_DMA_SEL | CDNS_XSPI_ACMD_INT |
> +		      FIELD_PREP(CDNS_XSPI_ACMD_CMD_TYPE,
> +				 CDNS_XSPI_ACMD_TYPE_READ);
> +
> +	reinit_completion(&cdns_xspi->auto_cmd_complete);
> +	cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
> +	wait_for_completion(&cdns_xspi->auto_cmd_complete);
> +
> +	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
> +	cdns_xspi_set_thread_error_interrupt(cdns_xspi, false);
> +	dma_unmap_single(cdns_xspi->dev, dma_addr, op->data.nbytes,
> +			 DMA_FROM_DEVICE);
> +
> +	if (cdns_xspi->xfer_error)
> +		return -EIO;
> +
> +	return cdns_xspi_acmd_check_status(cdns_xspi);
> +}

[Severity: Medium]
Does checking cdns_xspi->xfer_error and returning -EIO here prevent
cdns_xspi_acmd_check_status() from ever running when an ACMD command fails?

When thread 0 encounters an ACMD command error (setting
CDNS_XSPI_CMD_STATUS_FAILED in CDNS_XSPI_CMD_STATUS_REG due to
CDNS_XSPI_CMD_STATUS_DEVICE_ERROR, CDNS_XSPI_CMD_STATUS_DQS_ERROR,
CDNS_XSPI_CMD_STATUS_CRC_ERROR, CDNS_XSPI_CMD_STATUS_BUS_ERROR, or
CDNS_XSPI_CMD_STATUS_INV_SEQ_ERROR), the controller raises the thread 0 error
interrupt in CDNS_XSPI_TRD_ERR_INTR_STATUS.

In cdns_xspi_irq_handler(), that interrupt sets cdns_xspi->xfer_error = true
and completes cdns_xspi->auto_cmd_complete:

	irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_STATUS);
	if (irq_status) {
		writel(irq_status,
		       cdns_xspi->iobase + CDNS_XSPI_TRD_ERR_INTR_STATUS);

		if (irq_status & CDNS_XSPI_ACMD_THRD_0_MASK) {
			cdns_xspi->xfer_error = true;
			acmd_done = true;
		}

		result = IRQ_HANDLED;
	}

Because cdns_xspi_acmd_read() returns -EIO as soon as cdns_xspi->xfer_error is
true, cdns_xspi_acmd_check_status() -> cdns_xspi_check_command_status() is
bypassed on thread error interrupts, skipping all error logging (including the
new CDNS_XSPI_CMD_STATUS_DEVICE_ERROR check) and returning -EIO instead of
-EPROTO.

> +
> +static bool cdns_xspi_acmd_can_read(const struct cdns_xspi_dev *cdns_xspi,
> +				    const struct spi_mem_op *op)
> +{
> +	if (!cdns_xspi->has_acmd)
> +		return false;
> +
> +	if (op->data.dir != SPI_MEM_DATA_IN || !op->addr.nbytes)
> +		return false;
> +
> +	return op->data.nbytes >= CDNS_XSPI_ACMD_MIN_BYTES;
> +}
> +
>  static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
>  			    struct spi_mem *mem,
>  			    const struct spi_mem_op *op)
> @@ -605,6 +818,9 @@ static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
>  	if (cdns_xspi->cur_cs != spi_get_chipselect(mem->spi, 0))
>  		cdns_xspi->cur_cs = spi_get_chipselect(mem->spi, 0);
>  
> +	if (cdns_xspi_acmd_can_read(cdns_xspi, op))
> +		return cdns_xspi_acmd_read(cdns_xspi, op);
> +
>  	return cdns_xspi_send_stig_command(cdns_xspi, op,
>  					   (dir != SPI_MEM_NO_DATA));
>  }

[Severity: Medium]
Can cdns_xspi_transfer_one_message_b0() issue STIG commands while the
controller is still left in CDNS_XSPI_WORK_MODE_ACMD?

When mrvl_hw_overlay is enabled, marvell_xspi_mem_ops.exec_op calls
marvell_xspi_mem_op_execute() -> cdns_xspi_mem_op() -> cdns_xspi_acmd_read(),
which switches CDNS_XSPI_CTRL_CONFIG_REG to CDNS_XSPI_WORK_MODE_ACMD.

However, cdns_xspi_transfer_one_message_b0() in
drivers/spi/spi-cadence-xspi.c triggers STIG generic commands directly without
calling cdns_xspi_set_work_mode(cdns_xspi, CDNS_XSPI_WORK_MODE_STIG):

			if (current_transfer_len < 10) {
				cdns_xspi_prepare_generic(cs, txd, current_transfer_len,
							  false, cmd_regs);
				cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
				if (!cdns_xspi_is_stig_ready(cdns_xspi, true))
					return -EIO;
			} else {
				cdns_xspi_prepare_generic(cs, txd, 1, true, cmd_regs);
				cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
				cdns_xspi_prepare_transfer(cs, 1, current_transfer_len - 1,
							   cmd_regs);
				cdns_xspi_trigger_command(cdns_xspi, cmd_regs);

If a controller with mrvl_hw_overlay uses ACMD mode and then runs a regular SPI
transfer via cdns_xspi_transfer_one_message_b0(), won't writing STIG command
words to CDNS_XSPI_CMD_REG_0..5 while in ACMD mode be misinterpreted by the
hardware as an ACMD command?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-xspi-cadence-add-acmd-v1-0-53cd44690e04@analog.com?part=3

Re: [PATCH 3/3] spi: cadence-xspi: add ACMD mode support

[Mark Brown]

> So far every operation goes through STIG mode, where the CPU builds
> each command and drains the data. The controller also has an auto
> command (ACMD) mode where it runs a whole read sequence on its own and,
> with the internal DMA, writes the data straight to memory.

> +static int cdns_xspi_set_work_mode(struct cdns_xspi_dev *cdns_xspi, int mode)
> +{
> +	int ret;
> +
> +	if (cdns_xspi->cur_work_mode == mode)
> +		return 0;

We need some suspend/resume handling to invalidate the configuration
when the IP power goes so we set everything up again.  I'm not seeing
anything here.

> @@ -677,6 +893,9 @@ static int cdns_xspi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *
>  	struct cdns_xspi_dev *cdns_xspi =
>  		spi_controller_get_devdata(mem->spi->controller);
>  
> +	if (cdns_xspi_acmd_can_read(cdns_xspi, op))
> +		return 0;

That function has a minimum size but not a maximum size...

> +
>  	op->data.nbytes = clamp_val(op->data.nbytes, 0, cdns_xspi->sdmasize);

...which we have here?

-- 
Mark Brown <broonie@kernel.org>