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

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

[Jisheng Zhang]

This series tries to improve the cadence-xspi cpu usage and potential
performance if the allocated dmaengine is powerful.

But no matter the dmaengine performance is powerful or not, the cpu
usage is reduced a lot.

For example, on a dual CA55 platform, test with looping the following
cmd:
dd if=/dev/mtd0 of=/dev/null bs=8192

Before the patch:
cpu usage is nearly 50%, which means one CA55 is full loaded with
the sdma io copy.

After the patch:
cpu usage is between 0 ~ 1%

patch1 documents the dt-binding
patch2 adds the memcpy dmaengine support 
patch3 uses writel_relaxed for cmd regs.

Jisheng Zhang (3):
  spi: dt-bindings: cdns,xspi: Add DMA properties
  spi: cadence-xspi: Support dmaengine for sdma io copy
  spi: cadence-xspi: Use writel_relaxed() for cmd regs

 .../devicetree/bindings/spi/cdns,xspi.yaml    |  13 ++
 drivers/spi/spi-cadence-xspi.c                | 149 +++++++++++++++++-
 2 files changed, 155 insertions(+), 7 deletions(-)

-- 
2.55.0

[PATCH 1/3] spi: dt-bindings: cdns,xspi: Add DMA properties

[Jisheng Zhang]

Add dmas and dma-names to describe DMA memcpy channel for the candence
XSPI controller.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 .../devicetree/bindings/spi/cdns,xspi.yaml          | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index 38a5795589de..b19c2e051e18 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -40,6 +40,19 @@ properties:
   interrupts:
     maxItems: 1
 
+  dmas:
+    maxItems: 1
+    description: |
+      Optional DMA memcpy channel for transfers between memory and the Slave
+      DMA data port. The channel is shared by reads and writes.
+
+  dma-names:
+    const: memcpy
+
+dependencies:
+  dmas: [dma-names]
+  dma-names: [dmas]
+
 required:
   - compatible
   - reg
-- 
2.55.0

[PATCH 2/3] spi: cadence-xspi: Support dmaengine for sdma io copy

[Jisheng Zhang]

use dmaengine to do sdma io copy, performance improvement depends on
the memcpy dmaengine, maybe significant, maybe trivial, but no matter
the dmaengine performance is powerful or not, the cpu usage is reduced
a lot.

For example, on a dual CA55 platform, test with looping the following
cmd:
dd if=/dev/mtd0 of=/dev/null bs=8192

Before the patch:
cpu usage is nearly 50%, which means one CA55 is full loaded with
the sdma io copy.

After the patch:
cpu usage is between 0 ~ 1%

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/spi/spi-cadence-xspi.c | 139 ++++++++++++++++++++++++++++++++-
 1 file changed, 137 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 3687853e4ce4..d9996ec42edd 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -4,6 +4,8 @@
 
 #include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
@@ -19,12 +21,14 @@
 #include <linux/bitfield.h>
 #include <linux/limits.h>
 #include <linux/log2.h>
+#include <linux/mm.h>
 #include <linux/bitrev.h>
 #include <linux/util_macros.h>
 
 #define CDNS_XSPI_MAGIC_NUM_VALUE	0x6522
 #define CDNS_XSPI_MAX_BANKS		8
 #define CDNS_XSPI_NAME			"cadence-xspi"
+#define CDNS_XSPI_DMA_TIMEOUT_MS		5000
 
 /*
  * Note: below are additional auxiliary registers to
@@ -334,6 +338,10 @@ struct cdns_xspi_dev {
 	int cur_cs;
 	unsigned int sdmasize;
 	unsigned int sdma_xfer_len;
+	struct dma_chan *dma_chan;
+	dma_addr_t dma_addr;
+	struct completion dma_complete;
+	bool dma_error;
 
 	struct completion cmd_complete;
 	struct completion auto_cmd_complete;
@@ -355,6 +363,122 @@ struct cdns_xspi_dev {
 	int current_xfer_qword;
 };
 
+static void cdns_xspi_dma_cleanup(void *data)
+{
+	struct cdns_xspi_dev *cdns_xspi = data;
+
+	dma_unmap_resource(dmaengine_get_dma_device(cdns_xspi->dma_chan),
+			   cdns_xspi->dma_addr, cdns_xspi->sdmasize,
+			   DMA_BIDIRECTIONAL, 0);
+}
+
+static int cdns_xspi_dma_init(struct cdns_xspi_dev *cdns_xspi,
+			      phys_addr_t sdma_phys)
+{
+	struct device *dev = cdns_xspi->dev;
+	struct dma_chan *chan;
+	struct device *dma_dev;
+	dma_addr_t dma_addr;
+
+	/* The Marvell overlay has additional SDMA access-width restrictions. */
+	if (cdns_xspi->driver_data->mrvl_hw_overlay)
+		return 0;
+
+	chan = devm_dma_request_chan(dev, "memcpy");
+	if (IS_ERR(chan)) {
+		if (PTR_ERR(chan) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_dbg(dev, "DMA unavailable, using CPU transfers\n");
+		return 0;
+	}
+	if (!dma_has_cap(DMA_MEMCPY, chan->device->cap_mask)) {
+		dev_dbg(dev, "DMA channel does not support memcpy, using CPU transfers\n");
+		return 0;
+	}
+
+	/* Map the aperture once; only the RAM buffer changes between transfers. */
+	dma_dev = dmaengine_get_dma_device(chan);
+	dma_addr = dma_map_resource(dma_dev, sdma_phys, cdns_xspi->sdmasize,
+				    DMA_BIDIRECTIONAL, 0);
+	if (dma_mapping_error(dma_dev, dma_addr)) {
+		dev_dbg(dev, "Failed to map SDMA aperture, using CPU transfers\n");
+		return 0;
+	}
+
+	cdns_xspi->dma_chan = chan;
+	cdns_xspi->dma_addr = dma_addr;
+	init_completion(&cdns_xspi->dma_complete);
+
+	return devm_add_action_or_reset(dev, cdns_xspi_dma_cleanup, cdns_xspi);
+}
+
+static void cdns_xspi_dma_callback(void *data, const struct dmaengine_result *result)
+{
+	struct cdns_xspi_dev *cdns_xspi = data;
+
+	cdns_xspi->dma_error = result->result != DMA_TRANS_NOERROR || result->residue;
+	complete(&cdns_xspi->dma_complete);
+}
+
+/* -EAGAIN means no DMA was issued, so the caller can use the CPU path. */
+static int cdns_xspi_dma_transfer(struct cdns_xspi_dev *cdns_xspi,
+				  size_t len, bool read)
+{
+	struct dma_chan *chan = cdns_xspi->dma_chan;
+	dma_addr_t port_addr = cdns_xspi->dma_addr;
+	enum dma_data_direction dir = read ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	void *buf = read ? cdns_xspi->in_buffer : (void *)cdns_xspi->out_buffer;
+	struct dma_async_tx_descriptor *desc;
+	struct device *dma_dev;
+	dma_addr_t buf_addr;
+	dma_cookie_t cookie;
+	int ret = -EAGAIN;
+
+	if (!chan || len < cdns_xspi->dma_data_width ||
+	    len > cdns_xspi->sdmasize || !virt_addr_valid(buf))
+		return -EAGAIN;
+
+	dma_dev = dmaengine_get_dma_device(chan);
+	buf_addr = dma_map_single(dma_dev, buf, len, dir);
+	if (dma_mapping_error(dma_dev, buf_addr))
+		return -EAGAIN;
+	if (!is_dma_copy_aligned(chan->device, port_addr, buf_addr, len))
+		goto unmap;
+
+	desc = dmaengine_prep_dma_memcpy(chan, read ? buf_addr : port_addr,
+					 read ? port_addr : buf_addr, len,
+					 DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	if (!desc)
+		goto unmap;
+
+	reinit_completion(&cdns_xspi->dma_complete);
+	cdns_xspi->dma_error = false;
+	desc->callback_result = cdns_xspi_dma_callback;
+	desc->callback_param = cdns_xspi;
+	cookie = dmaengine_submit(desc);
+	if (dma_submit_error(cookie)) {
+		dmaengine_terminate_sync(chan);
+		goto unmap;
+	}
+	dma_async_issue_pending(chan);
+
+	if (!wait_for_completion_timeout(&cdns_xspi->dma_complete,
+					msecs_to_jiffies(CDNS_XSPI_DMA_TIMEOUT_MS))) {
+		dmaengine_terminate_sync(chan);
+		dev_err(cdns_xspi->dev, "SDMA %s DMA timeout\n", read ? "read" : "write");
+		ret = -ETIMEDOUT;
+	} else if (cdns_xspi->dma_error) {
+		dmaengine_terminate_sync(chan);
+		ret = -EIO;
+	} else {
+		ret = 0;
+	}
+
+unmap:
+	dma_unmap_single(dma_dev, buf_addr, len, dir);
+	return ret;
+}
+
 static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
 {
 	u32 ctrl_stat;
@@ -502,6 +626,7 @@ static int cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
 {
 	u32 sdma_size, sdma_trd_info;
 	u8 sdma_dir;
+	int ret;
 
 	sdma_size = readl(cdns_xspi->iobase + CDNS_XSPI_SDMA_SIZE_REG);
 	sdma_trd_info = readl(cdns_xspi->iobase + CDNS_XSPI_SDMA_TRD_INFO_REG);
@@ -513,6 +638,10 @@ static int cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
 			sdma_size, cdns_xspi->sdma_xfer_len);
 		return -EINVAL;
 	}
+	ret = cdns_xspi_dma_transfer(cdns_xspi, sdma_size,
+				     sdma_dir == CDNS_XSPI_SDMA_DIR_READ);
+	if (ret != -EAGAIN)
+		return ret;
 
 	switch (sdma_dir) {
 	case CDNS_XSPI_SDMA_DIR_READ:
@@ -543,6 +672,8 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
 	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
 
+	reinit_completion(&cdns_xspi->cmd_complete);
+	reinit_completion(&cdns_xspi->sdma_complete);
 	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
 	cdns_xspi->sdma_error = false;
 
@@ -580,9 +711,9 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 			return -EIO;
 		}
 		ret = cdns_xspi->sdma_handler(cdns_xspi);
-		if (ret) {
+		if (ret || cdns_xspi->sdma_error) {
 			cdns_xspi->set_interrupts_handler(cdns_xspi, false);
-			return ret;
+			return ret ?: -EIO;
 		}
 	}
 
@@ -1277,6 +1408,10 @@ static int cdns_xspi_probe(struct platform_device *pdev)
 	}
 	cdns_xspi->sdmasize = resource_size(res);
 
+	ret = cdns_xspi_dma_init(cdns_xspi, res->start);
+	if (ret)
+		return ret;
+
 	cdns_xspi->auxbase = devm_platform_ioremap_resource_byname(pdev, "aux");
 	if (IS_ERR(cdns_xspi->auxbase)) {
 		cdns_xspi->auxbase = devm_platform_ioremap_resource(pdev, 2);
-- 
2.55.0

[PATCH 3/3] spi: cadence-xspi: Use writel_relaxed() for cmd regs

[Jisheng Zhang]

Use writel_relaxed() for all command register writes except the last
one in cdns_xspi_trigger_command(). The ordering guarantees provided
by the final writel() ensure that all preceding writes are completed
before the command is triggered.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/spi/spi-cadence-xspi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index d9996ec42edd..72852849ba27 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -494,11 +494,11 @@ static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
 static void cdns_xspi_trigger_command(struct cdns_xspi_dev *cdns_xspi,
 				      u32 cmd_regs[6])
 {
-	writel(cmd_regs[5], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_5);
-	writel(cmd_regs[4], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_4);
-	writel(cmd_regs[3], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_3);
-	writel(cmd_regs[2], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_2);
-	writel(cmd_regs[1], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_1);
+	writel_relaxed(cmd_regs[5], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_5);
+	writel_relaxed(cmd_regs[4], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_4);
+	writel_relaxed(cmd_regs[3], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_3);
+	writel_relaxed(cmd_regs[2], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_2);
+	writel_relaxed(cmd_regs[1], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_1);
 	writel(cmd_regs[0], cdns_xspi->iobase + CDNS_XSPI_CMD_REG_0);
 }
 
-- 
2.55.0

Re: [PATCH 2/3] spi: cadence-xspi: Support dmaengine for sdma io copy

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] spi: cadence-xspi: unreleased DMA channel on fallback in
  cdns_xspi_dma_init()
--

commit a84278759465bccce36b68c10064eeefdfafaa8b
Author: Jisheng Zhang <jszhang@kernel.org>

spi: cadence-xspi: Support dmaengine for sdma io copy

This commit adds dmaengine memcpy support for SDMA I/O transfers in the
Cadence xSPI controller driver, falling back to CPU transfers when DMA is
unavailable.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 3687853e4ce4e..d9996ec42edda 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c

[ ... ]

> @@ -355,6 +363,122 @@ struct cdns_xspi_dev {
>  	int current_xfer_qword;
>  };
>  
> +static void cdns_xspi_dma_cleanup(void *data)
> +{
> +	struct cdns_xspi_dev *cdns_xspi = data;
> +
> +	dma_unmap_resource(dmaengine_get_dma_device(cdns_xspi->dma_chan),
> +			   cdns_xspi->dma_addr, cdns_xspi->sdmasize,
> +			   DMA_BIDIRECTIONAL, 0);
> +}
> +
> +static int cdns_xspi_dma_init(struct cdns_xspi_dev *cdns_xspi,
> +			      phys_addr_t sdma_phys)
> +{
> +	struct device *dev = cdns_xspi->dev;
> +	struct dma_chan *chan;
> +	struct device *dma_dev;
> +	dma_addr_t dma_addr;
> +
> +	/* The Marvell overlay has additional SDMA access-width restrictions. */
> +	if (cdns_xspi->driver_data->mrvl_hw_overlay)
> +		return 0;
> +
> +	chan = devm_dma_request_chan(dev, "memcpy");
> +	if (IS_ERR(chan)) {
> +		if (PTR_ERR(chan) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_dbg(dev, "DMA unavailable, using CPU transfers\n");
> +		return 0;
> +	}
> +	if (!dma_has_cap(DMA_MEMCPY, chan->device->cap_mask)) {
> +		dev_dbg(dev, "DMA channel does not support memcpy, using CPU transfers\n");
> +		return 0;
> +	}
> +
> +	/* Map the aperture once; only the RAM buffer changes between transfers. */
> +	dma_dev = dmaengine_get_dma_device(chan);
> +	dma_addr = dma_map_resource(dma_dev, sdma_phys, cdns_xspi->sdmasize,
> +				    DMA_BIDIRECTIONAL, 0);
> +	if (dma_mapping_error(dma_dev, dma_addr)) {
> +		dev_dbg(dev, "Failed to map SDMA aperture, using CPU transfers\n");
> +		return 0;
> +	}

[Severity: Medium]
Does cdns_xspi_dma_init() leave chan allocated if the DMA_MEMCPY check or
dma_map_resource() fails?  Since cdns_xspi_dma_init() returns 0 to fall
back to CPU transfers, cdns_xspi_probe() succeeds and devres keeps the
unused DMA channel exclusively claimed for the lifetime of the bound
device.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107072044.17574-1-jszhang@kernel.org?part=2

Re: [PATCH 1/3] spi: dt-bindings: cdns,xspi: Add DMA properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] spi: dt-bindings: cdns,xspi: misuse of dmas property for
  memcpy channel
--

commit bdaf74eae564e5ab96700c90ad51b7fb6c7c0da6
Author: Jisheng Zhang <jszhang@kernel.org>

spi: dt-bindings: cdns,xspi: Add DMA properties

This commit adds dmas and dma-names properties to the Cadence XSPI
device tree binding to describe a DMA memcpy channel.

> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index 38a5795589de2..b19c2e051e189 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -40,6 +40,19 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  dmas:
> +    maxItems: 1
> +    description: |
> +      Optional DMA memcpy channel for transfers between memory and the Slave
> +      DMA data port. The channel is shared by reads and writes.
> +
> +  dma-names:
> +    const: memcpy
> +
> +dependencies:
> +  dmas: [dma-names]
> +  dma-names: [dmas]

[Severity: Medium]
Should a software-triggered DMA_MEMCPY channel be described with dmas and
dma-names in the device tree binding?

The dmas property in dma-common.yaml describes hardware DMA request lines
routed between a peripheral and a DMA controller. The Cadence XSPI Slave
DMA data port is a memory-mapped aperture driven by the controller's SDMA
trigger interrupt and memory-to-memory DMA_MEMCPY transfers, with no
hardware DMA request signal.

Requesting this channel through devm_dma_request_chan(dev, "memcpy") goes
through of_dma_request_slave_channel() and the DMA controller's
of_dma_xlate() callback, which expects hardware slave request specifiers
in #dma-cells and fails or requires fabricated specifiers on controllers
that validate slave request IDs or only expose DMA_MEMCPY channels.

Other drivers with memory-mapped ports and DMA_MEMCPY transfers, including
the sibling Cadence HPNFC driver in
drivers/mtd/nand/raw/cadence-nand-controller.c and
drivers/spi/spi-cadence-quadspi.c, allocate the channel in the driver via
dma_request_chan_by_mask() with DMA_MEMCPY instead of adding dmas =
"memcpy" to the binding. If a platform does wire a hardware DMA request
line to the SPI controller, spi-controller.yaml names a shared channel
"rx-tx" rather than "memcpy".

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107072044.17574-1-jszhang@kernel.org?part=1