[PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support

[PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support

[Max Hsu]

This series addresses critical bugs in the SiFive Platform DMA (PDMA)
driver and adds support for the FU740 SoC.

The first three patches fix serious issues in the existing sf-pdma driver:

1. Missing PDMA base offset (0x80000) in register calculations. While
   the hardware provides an alias at offset 0x0, the driver should use
   the canonical offset 0x80000 as documented in the specification to
   ensure consistency and maintainability.

2. Race condition between done and error interrupts on SMP systems. Per
   the FU540-C000 and FU740-C000 specs, both DONE and ERROR interrupt
   bits are set simultaneously when a DMA error occurs, which can cause
   concurrent execution of done_isr and err_isr on different CPUs,
   leading to undefined behavior.

3. NULL pointer dereferences in both error and done tasklets due to race
   conditions during channel termination. Both tasklets unconditionally
   dereference chan->desc, which can be NULL during legitimate scenarios
   like sf_pdma_terminate_all() or when interrupts fire after channel
   cleanup. The fix adds NULL checks in both tasklets, protected by
   vchan.lock to ensure atomicity.

These three fixes are tagged for stable as they address bugs present
since the driver's introduction in commit 6973886ad58e ("dmaengine:
sf-pdma: add platform DMA support for HiFive Unleashed A00").

The last two patches add FU740 support:

4. Add "sifive,fu740-c000-pdma" compatible string to the dt-bindings.

5. Add PDMA device node to the FU740 device tree to enable DMA support.

All patches have been tested on HiFive Unmatched (FU740-C000) hardware.

Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
Max Hsu (5):
      dmaengine: sf-pdma: add missing PDMA base offset to register calculations
      dmaengine: sf-pdma: fix race between done and error interrupts
      dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers
      dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support
      riscv: dts: sifive: fu740: add PDMA device node

 .../bindings/dma/sifive,fu540-c000-pdma.yaml       |  1 +
 arch/riscv/boot/dts/sifive/fu740-c000.dtsi         |  9 ++++
 drivers/dma/sf-pdma/sf-pdma.c                      | 63 +++++++++++++++++-----
 drivers/dma/sf-pdma/sf-pdma.h                      |  4 +-
 4 files changed, 63 insertions(+), 14 deletions(-)
---
base-commit: 8bf22c33e7a172fbc72464f4cc484d23a6b412ba
change-id: 20260111-pdma-0421b0e48a10

Best regards,
-- 
Max Hsu <max.hsu@sifive.com>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations

[Max Hsu]

The PDMA control registers start at offset 0x80000 from the PDMA base
address, according to the FU540-C000 v1p1 manual [1].

The current SF_PDMA_REG_BASE macro is missing this offset:
    Current:  pdma->membase + (PDMA_CHAN_OFFSET * ch)
    Correct:  pdma->membase + 0x80000 + (PDMA_CHAN_OFFSET * ch)

Fix by adding PDMA_BASE_OFFSET (0x80000) to the register address
calculation.

Link: https://www.sifive.com/document-file/freedom-u540-c000-manual [1]
Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
 drivers/dma/sf-pdma/sf-pdma.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
index 215e07183d7e..d33551eb2ee8 100644
--- a/drivers/dma/sf-pdma/sf-pdma.h
+++ b/drivers/dma/sf-pdma/sf-pdma.h
@@ -24,7 +24,7 @@
 
 #define PDMA_MAX_NR_CH					4
 
-#define PDMA_BASE_ADDR					0x3000000
+#define PDMA_BASE_OFFSET				0x80000
 #define PDMA_CHAN_OFFSET				0x1000
 
 /* Register Offset */
@@ -54,7 +54,7 @@
 /* Error Recovery */
 #define MAX_RETRY					1
 
-#define SF_PDMA_REG_BASE(ch)	(pdma->membase + (PDMA_CHAN_OFFSET * (ch)))
+#define SF_PDMA_REG_BASE(ch)	(pdma->membase + PDMA_BASE_OFFSET + (PDMA_CHAN_OFFSET * (ch)))
 
 struct pdma_regs {
 	/* read-write regs */

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts

[Max Hsu]

According to the FU540-C000 v1p5 [1] and FU740-C000 v1p7 [2] specs,
when a DMA transaction error occurs, the hardware sets both the
DONE and ERROR interrupt bits simultaneously.
On SMP systems, this can cause the done_isr and err_isr to execute
concurrently on different CPUs, leading to race conditions and
NULL pointer dereferences.

Fix by:
- In done_isr: abort if ERROR bit is set or DONE bit was already cleared
- In err_isr: clear both DONE and ERROR bits to prevent done_isr from
  processing the same transaction

Link: https://www.sifive.com/document-file/freedom-u540-c000-manual [1]
Link: https://www.sifive.com/document-file/freedom-u740-c000-manual [2]
Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
Cc: stable@vger.kernel.org
Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
 drivers/dma/sf-pdma/sf-pdma.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index 7ad3c29be146..ac7d3b127a24 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -346,9 +346,25 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 	struct sf_pdma_chan *chan = dev_id;
 	struct pdma_regs *regs = &chan->regs;
 	u64 residue;
+	u32 control_reg;
 
 	spin_lock(&chan->vchan.lock);
-	writel((readl(regs->ctrl)) & ~PDMA_DONE_STATUS_MASK, regs->ctrl);
+	control_reg = readl(regs->ctrl);
+	if (control_reg & PDMA_ERR_STATUS_MASK) {
+		spin_unlock(&chan->vchan.lock);
+		return IRQ_HANDLED;
+	}
+
+	/*
+	 * Check if DONE bit is still set. If not, the error ISR on another
+	 * CPU has already cleared it, so abort to avoid double-processing.
+	 */
+	if (!(control_reg & PDMA_DONE_STATUS_MASK)) {
+		spin_unlock(&chan->vchan.lock);
+		return IRQ_HANDLED;
+	}
+
+	writel((control_reg & ~PDMA_DONE_STATUS_MASK), regs->ctrl);
 	residue = readq(regs->residue);
 
 	if (!residue) {
@@ -375,7 +391,7 @@ static irqreturn_t sf_pdma_err_isr(int irq, void *dev_id)
 	struct pdma_regs *regs = &chan->regs;
 
 	spin_lock(&chan->lock);
-	writel((readl(regs->ctrl)) & ~PDMA_ERR_STATUS_MASK, regs->ctrl);
+	writel((readl(regs->ctrl)) & ~(PDMA_DONE_STATUS_MASK | PDMA_ERR_STATUS_MASK), regs->ctrl);
 	spin_unlock(&chan->lock);
 
 	tasklet_schedule(&chan->err_tasklet);

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers

[Max Hsu]

Fix NULL pointer dereferences in both the error and done tasklets that
can occur due to race conditions during channel termination or completion.

Both tasklets (sf_pdma_errbh_tasklet and sf_pdma_donebh_tasklet)
dereference chan->desc without checking if it's NULL. However,
chan->desc can be NULL in legitimate scenarios:

1. During sf_pdma_terminate_all(): The function sets chan->desc = NULL
   while holding vchan.lock, but interrupts for previously submitted
   transactions could fire after the lock is released, before the
   hardware is fully quiesced. These interrupts can schedule tasklets
   that will run with chan->desc = NULL.

2. During channel cleanup: Similar race condition during
   sf_pdma_free_chan_resources().

The fix adds NULL checks at the beginning of both tasklets, protected
by vchan.lock, using the same lock that terminate_all and
free_chan_resources use when setting chan->desc = NULL. This ensures
that either:
- The descriptor is valid and we can safely process it, or
- The descriptor was already freed and we safely skip processing

Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
Cc: stable@vger.kernel.org
Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
 drivers/dma/sf-pdma/sf-pdma.c | 43 +++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index ac7d3b127a24..70e4afcda52a 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -298,33 +298,56 @@ static void sf_pdma_free_desc(struct virt_dma_desc *vdesc)
 static void sf_pdma_donebh_tasklet(struct tasklet_struct *t)
 {
 	struct sf_pdma_chan *chan = from_tasklet(chan, t, done_tasklet);
+	struct sf_pdma_desc *desc;
 	unsigned long flags;
 
-	spin_lock_irqsave(&chan->lock, flags);
-	if (chan->xfer_err) {
-		chan->retries = MAX_RETRY;
-		chan->status = DMA_COMPLETE;
-		chan->xfer_err = false;
+	spin_lock_irqsave(&chan->vchan.lock, flags);
+	desc = chan->desc;
+	if (!desc) {
+		/*
+		 * The descriptor was already freed (e.g., by terminate_all
+		 * or completion on another CPU). Nothing to do.
+		 */
+		spin_unlock_irqrestore(&chan->vchan.lock, flags);
+		return;
 	}
-	spin_unlock_irqrestore(&chan->lock, flags);
 
-	spin_lock_irqsave(&chan->vchan.lock, flags);
-	list_del(&chan->desc->vdesc.node);
-	vchan_cookie_complete(&chan->desc->vdesc);
+	list_del(&desc->vdesc.node);
+	vchan_cookie_complete(&desc->vdesc);
 
 	chan->desc = sf_pdma_get_first_pending_desc(chan);
 	if (chan->desc)
 		sf_pdma_xfer_desc(chan);
 
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
+
+	spin_lock_irqsave(&chan->lock, flags);
+	if (chan->xfer_err) {
+		chan->retries = MAX_RETRY;
+		chan->status = DMA_COMPLETE;
+		chan->xfer_err = false;
+	}
+	spin_unlock_irqrestore(&chan->lock, flags);
 }
 
 static void sf_pdma_errbh_tasklet(struct tasklet_struct *t)
 {
 	struct sf_pdma_chan *chan = from_tasklet(chan, t, err_tasklet);
-	struct sf_pdma_desc *desc = chan->desc;
+	struct sf_pdma_desc *desc;
 	unsigned long flags;
 
+	spin_lock_irqsave(&chan->vchan.lock, flags);
+	desc = chan->desc;
+	if (!desc) {
+		/*
+		 * The descriptor was already freed (e.g., by terminate_all
+		 * or completion on another CPU). Nothing to do.
+		 */
+		spin_unlock_irqrestore(&chan->vchan.lock, flags);
+		return;
+	}
+	spin_unlock_irqrestore(&chan->vchan.lock, flags);
+
 	spin_lock_irqsave(&chan->lock, flags);
 	if (chan->retries <= 0) {
 		/* fail to recover */

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support

[Max Hsu]

Add "sifive,fu740-c000-pdma" compatible string.

Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
 Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml b/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
index 609e38901434..b6c49060bc6f 100644
--- a/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
+++ b/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
@@ -36,6 +36,7 @@ properties:
           - enum:
               - microchip,mpfs-pdma
               - sifive,fu540-c000-pdma
+              - sifive,fu740-c000-pdma
           - const: sifive,pdma0
     description:
       Should be "sifive,<chip>-pdma" and "sifive,pdma<version>".

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node

[Max Hsu]

The FU740 SoC includes a 4-channel Platform DMA (PDMA) controller.
Add the device node to enable DMA support.

Signed-off-by: Max Hsu <max.hsu@sifive.com>
---
 arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
index 6150f3397bff..30d0d6837c57 100644
--- a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
+++ b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
@@ -329,6 +329,15 @@ gpio: gpio@10060000 {
 			clocks = <&prci FU740_PRCI_CLK_PCLK>;
 			status = "disabled";
 		};
+		dma: dma-controller@3000000 {
+			compatible = "sifive,fu740-c000-pdma", "sifive,pdma0";
+			reg = <0x0 0x3000000 0x0 0x100000>;
+			interrupt-parent = <&plic0>;
+			interrupts = <11>, <12>, <13>, <14>, <15>, <16>, <17>, <18>;
+			dma-channels = <4>;
+			clocks = <&prci FU740_PRCI_CLK_PCLK>;
+			#dma-cells = <1>;
+		};
 		pcie@e00000000 {
 			compatible = "sifive,fu740-pcie";
 			#address-cells = <3>;

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations

[Frank Li]

On Sat, Feb 21, 2026 at 03:43:53AM +0800, Max Hsu wrote:
> The PDMA control registers start at offset 0x80000 from the PDMA base
> address, according to the FU540-C000 v1p1 manual [1].
>
> The current SF_PDMA_REG_BASE macro is missing this offset:
>     Current:  pdma->membase + (PDMA_CHAN_OFFSET * ch)
>     Correct:  pdma->membase + 0x80000 + (PDMA_CHAN_OFFSET * ch)

How it work at previous version? suppose it is tested when upstream this
driver?

>
> Fix by adding PDMA_BASE_OFFSET (0x80000) to the register address
> calculation.
>
> Link: https://www.sifive.com/document-file/freedom-u540-c000-manual [1]
> Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
> Signed-off-by: Max Hsu <max.hsu@sifive.com>
> ---
>  drivers/dma/sf-pdma/sf-pdma.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
> index 215e07183d7e..d33551eb2ee8 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.h
> +++ b/drivers/dma/sf-pdma/sf-pdma.h
> @@ -24,7 +24,7 @@
>
>  #define PDMA_MAX_NR_CH					4
>
> -#define PDMA_BASE_ADDR					0x3000000

This change belong to cleanup, don't mix to fixes into fix patch.

> +#define PDMA_BASE_OFFSET				0x80000
>  #define PDMA_CHAN_OFFSET				0x1000
>
>  /* Register Offset */
> @@ -54,7 +54,7 @@
>  /* Error Recovery */
>  #define MAX_RETRY					1
>
> -#define SF_PDMA_REG_BASE(ch)	(pdma->membase + (PDMA_CHAN_OFFSET * (ch)))
> +#define SF_PDMA_REG_BASE(ch)	(pdma->membase + PDMA_BASE_OFFSET + (PDMA_CHAN_OFFSET * (ch)))

why not set membase to pdma->membase + PDMA_BASE_OFFSET directly? are there
registers between pdma->membase and pdma->membase + PDMA_BASE_OFFSET?

Frank

>
>  struct pdma_regs {
>  	/* read-write regs */
>
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts

[Frank Li]

On Sat, Feb 21, 2026 at 03:43:54AM +0800, Max Hsu wrote:
> According to the FU540-C000 v1p5 [1] and FU740-C000 v1p7 [2] specs,
> when a DMA transaction error occurs, the hardware sets both the
> DONE and ERROR interrupt bits simultaneously.

Nit: need extra empty line between two paragraph.

> On SMP systems, this can cause the done_isr and err_isr to execute
> concurrently on different CPUs, leading to race conditions and
> NULL pointer dereferences.

On SMP systems, the race conditons and NULL pointer dereferences happen if
the done_isr and err_isr to execute concurrently on different CPUs

>
> Fix by:
> - In done_isr: abort if ERROR bit is set or DONE bit was already cleared
> - In err_isr: clear both DONE and ERROR bits to prevent done_isr from
>   processing the same transaction
>
> Link: https://www.sifive.com/document-file/freedom-u540-c000-manual [1]
> Link: https://www.sifive.com/document-file/freedom-u740-c000-manual [2]
> Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Hsu <max.hsu@sifive.com>
> ---
>  drivers/dma/sf-pdma/sf-pdma.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> index 7ad3c29be146..ac7d3b127a24 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.c
> +++ b/drivers/dma/sf-pdma/sf-pdma.c
> @@ -346,9 +346,25 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
>  	struct sf_pdma_chan *chan = dev_id;
>  	struct pdma_regs *regs = &chan->regs;
>  	u64 residue;
> +	u32 control_reg;
>
>  	spin_lock(&chan->vchan.lock);
> -	writel((readl(regs->ctrl)) & ~PDMA_DONE_STATUS_MASK, regs->ctrl);
> +	control_reg = readl(regs->ctrl);
> +	if (control_reg & PDMA_ERR_STATUS_MASK) {
> +		spin_unlock(&chan->vchan.lock);
> +		return IRQ_HANDLED;
> +	}
> +
> +	/*
> +	 * Check if DONE bit is still set. If not, the error ISR on another
> +	 * CPU has already cleared it, so abort to avoid double-processing.
> +	 */
> +	if (!(control_reg & PDMA_DONE_STATUS_MASK)) {
> +		spin_unlock(&chan->vchan.lock);
> +		return IRQ_HANDLED;
> +	}
> +
> +	writel((control_reg & ~PDMA_DONE_STATUS_MASK), regs->ctrl);
>  	residue = readq(regs->residue);
>
>  	if (!residue) {
> @@ -375,7 +391,7 @@ static irqreturn_t sf_pdma_err_isr(int irq, void *dev_id)
>  	struct pdma_regs *regs = &chan->regs;
>
>  	spin_lock(&chan->lock);
> -	writel((readl(regs->ctrl)) & ~PDMA_ERR_STATUS_MASK, regs->ctrl);
> +	writel((readl(regs->ctrl)) & ~(PDMA_DONE_STATUS_MASK | PDMA_ERR_STATUS_MASK), regs->ctrl);
>  	spin_unlock(&chan->lock);
>
>  	tasklet_schedule(&chan->err_tasklet);


ideally, it'd better handle by one function

sf_pdma_isr()
{
     stat = readl(regs->ctrl);
     writel(stat & ~(PDMA_DONE_STATUS_MASK | PDMA_ERR_STATUS_MASK), regs->ctrl);

     if (err)
	return err_handle();

     if (done)
       return done_handle();

     return IRQ_NONE;
}

Anyways, this also work

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers

[Frank Li]

On Sat, Feb 21, 2026 at 03:43:55AM +0800, Max Hsu wrote:
> Fix NULL pointer dereferences in both the error and done tasklets that
> can occur due to race conditions during channel termination or completion.
>
> Both tasklets (sf_pdma_errbh_tasklet and sf_pdma_donebh_tasklet)
> dereference chan->desc without checking if it's NULL. However,
> chan->desc can be NULL in legitimate scenarios:
>
> 1. During sf_pdma_terminate_all(): The function sets chan->desc = NULL
>    while holding vchan.lock, but interrupts for previously submitted
>    transactions could fire after the lock is released, before the
>    hardware is fully quiesced. These interrupts can schedule tasklets
>    that will run with chan->desc = NULL.
>
> 2. During channel cleanup: Similar race condition during
>    sf_pdma_free_chan_resources().
>
> The fix adds NULL checks at the beginning of both tasklets, protected
> by vchan.lock, using the same lock that terminate_all and
> free_chan_resources use when setting chan->desc = NULL. This ensures
> that either:
> - The descriptor is valid and we can safely process it, or
> - The descriptor was already freed and we safely skip processing
>
> Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
> Cc: stable@vger.kernel.org
> Signed-off-by: Max Hsu <max.hsu@sifive.com>
> ---
>  drivers/dma/sf-pdma/sf-pdma.c | 43 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> index ac7d3b127a24..70e4afcda52a 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.c
> +++ b/drivers/dma/sf-pdma/sf-pdma.c
> @@ -298,33 +298,56 @@ static void sf_pdma_free_desc(struct virt_dma_desc *vdesc)
>  static void sf_pdma_donebh_tasklet(struct tasklet_struct *t)
>  {
>  	struct sf_pdma_chan *chan = from_tasklet(chan, t, done_tasklet);
> +	struct sf_pdma_desc *desc;
>  	unsigned long flags;
>
> -	spin_lock_irqsave(&chan->lock, flags);
> -	if (chan->xfer_err) {
> -		chan->retries = MAX_RETRY;
> -		chan->status = DMA_COMPLETE;
> -		chan->xfer_err = false;
> +	spin_lock_irqsave(&chan->vchan.lock, flags);
> +	desc = chan->desc;
> +	if (!desc) {
> +		/*
> +		 * The descriptor was already freed (e.g., by terminate_all
> +		 * or completion on another CPU). Nothing to do.
> +		 */
> +		spin_unlock_irqrestore(&chan->vchan.lock, flags);
> +		return;

suggest use scoped_guard(spin_lock_irqsave)(&chan->lock)

Frank
>  	}
> -	spin_unlock_irqrestore(&chan->lock, flags);
>
> -	spin_lock_irqsave(&chan->vchan.lock, flags);
> -	list_del(&chan->desc->vdesc.node);
> -	vchan_cookie_complete(&chan->desc->vdesc);
> +	list_del(&desc->vdesc.node);
> +	vchan_cookie_complete(&desc->vdesc);
>
>  	chan->desc = sf_pdma_get_first_pending_desc(chan);
>  	if (chan->desc)
>  		sf_pdma_xfer_desc(chan);
>
>  	spin_unlock_irqrestore(&chan->vchan.lock, flags);
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +	if (chan->xfer_err) {
> +		chan->retries = MAX_RETRY;
> +		chan->status = DMA_COMPLETE;
> +		chan->xfer_err = false;
> +	}
> +	spin_unlock_irqrestore(&chan->lock, flags);
>  }
>
>  static void sf_pdma_errbh_tasklet(struct tasklet_struct *t)
>  {
>  	struct sf_pdma_chan *chan = from_tasklet(chan, t, err_tasklet);
> -	struct sf_pdma_desc *desc = chan->desc;
> +	struct sf_pdma_desc *desc;
>  	unsigned long flags;
>
> +	spin_lock_irqsave(&chan->vchan.lock, flags);
> +	desc = chan->desc;
> +	if (!desc) {
> +		/*
> +		 * The descriptor was already freed (e.g., by terminate_all
> +		 * or completion on another CPU). Nothing to do.
> +		 */
> +		spin_unlock_irqrestore(&chan->vchan.lock, flags);
> +		return;
> +	}
> +	spin_unlock_irqrestore(&chan->vchan.lock, flags);
> +
>  	spin_lock_irqsave(&chan->lock, flags);
>  	if (chan->retries <= 0) {
>  		/* fail to recover */
>
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node

[Frank Li]

On Sat, Feb 21, 2026 at 03:43:57AM +0800, Max Hsu wrote:
> The FU740 SoC includes a 4-channel Platform DMA (PDMA) controller.
> Add the device node to enable DMA support.
>
> Signed-off-by: Max Hsu <max.hsu@sifive.com>
> ---
>  arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
> index 6150f3397bff..30d0d6837c57 100644
> --- a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
> +++ b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
> @@ -329,6 +329,15 @@ gpio: gpio@10060000 {
>  			clocks = <&prci FU740_PRCI_CLK_PCLK>;
>  			status = "disabled";
>  		};
> +		dma: dma-controller@3000000 {

not sure sifive, generally require orderby hex address, and need empty
line between child node.

Frank
> +			compatible = "sifive,fu740-c000-pdma", "sifive,pdma0";
> +			reg = <0x0 0x3000000 0x0 0x100000>;
> +			interrupt-parent = <&plic0>;
> +			interrupts = <11>, <12>, <13>, <14>, <15>, <16>, <17>, <18>;
> +			dma-channels = <4>;
> +			clocks = <&prci FU740_PRCI_CLK_PCLK>;
> +			#dma-cells = <1>;
> +		};
>  		pcie@e00000000 {
>  			compatible = "sifive,fu740-pcie";
>  			#address-cells = <3>;
>
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations

[Conor Dooley]

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

On Fri, Feb 20, 2026 at 03:10:44PM -0500, Frank Li wrote:
> On Sat, Feb 21, 2026 at 03:43:53AM +0800, Max Hsu wrote:
> > The PDMA control registers start at offset 0x80000 from the PDMA base
> > address, according to the FU540-C000 v1p1 manual [1].
> >
> > The current SF_PDMA_REG_BASE macro is missing this offset:
> >     Current:  pdma->membase + (PDMA_CHAN_OFFSET * ch)
> >     Correct:  pdma->membase + 0x80000 + (PDMA_CHAN_OFFSET * ch)
> 
> How it work at previous version? suppose it is tested when upstream this
> driver?

Yeah, it's been tested alright. I'm pretty sceptical about this patch
being correct even if it does match the documentation as both mpfs and
the fu540 have dt nodes that look like:
		dma: dma-controller@3000000 {
			compatible = "sifive,fu540-c000-pdma", "sifive,pdma0";
			reg = <0x0 0x3000000 0x0 0x8000>;
			interrupt-parent = <&plic0>;
			interrupts = <23>, <24>, <25>, <26>, <27>, <28>, <29>,
				     <30>;
			dma-channels = <4>;
			#dma-cells = <1>;
		};
and on at least mpfs the pdma works. In fact, adding 0x8_0000 to membase
would lead to accessing outside of the reg region described by the
property.
The docs for PolarFire SoC (that's mpfs) don't mention this 0x80000
offset - see 3.1.10.1 of https://ww1.microchip.com/downloads/aemDocuments/documents/FPGA/ProductDocuments/ReferenceManuals/PolarFire_SoC_FPGA_MSS_Technical_Reference_Manual_VC.pdf

On the fu540 it may actually not work, I don't know as I have never
tried, but that's what the driver was originally written for. Given that
mpfs has the exact same setup I suspect it does work though...

The 0x8_0000 offset is very weird though, why offset like that and what's
in that region? The original documentation for the fu540 also doesn't
have the offset in it: https://pdos.csail.mit.edu/6.828/2025/readings/FU540-C000-v1.0.pdf
Why did this get changed? Should we also have changed this in our
documentation for mpfs and there are actually two interfaces for this IP
block, with the higher address being preferred?

Making this change blindly will break existing devicetrees, so it cannot
go in in this form.

If the fu740 requires this, then add a fu740 compatible and make this
offset fu740 specific. If it actually works with the current driver, I
vote for leaving this as-is with a comment explaining how it works.
For the fu540, could you test if the current driver and dts work? If
they don't, then we can consider more invasive changes.

mpfs works (and matches our docs), so I don't think we should make any
changes that affect it unless you can provide me with an explanation for
why the fu540 docs changed etc.

Cheers,
Conor.

> 
> >
> > Fix by adding PDMA_BASE_OFFSET (0x80000) to the register address
> > calculation.
> >
> > Link: https://www.sifive.com/document-file/freedom-u540-c000-manual [1]
> > Fixes: 6973886ad58e ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
> > Signed-off-by: Max Hsu <max.hsu@sifive.com>
> > ---
> >  drivers/dma/sf-pdma/sf-pdma.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/sf-pdma/sf-pdma.h b/drivers/dma/sf-pdma/sf-pdma.h
> > index 215e07183d7e..d33551eb2ee8 100644
> > --- a/drivers/dma/sf-pdma/sf-pdma.h
> > +++ b/drivers/dma/sf-pdma/sf-pdma.h
> > @@ -24,7 +24,7 @@
> >
> >  #define PDMA_MAX_NR_CH					4
> >
> > -#define PDMA_BASE_ADDR					0x3000000
> 
> This change belong to cleanup, don't mix to fixes into fix patch.
> 
> > +#define PDMA_BASE_OFFSET				0x80000
> >  #define PDMA_CHAN_OFFSET				0x1000
> >
> >  /* Register Offset */
> > @@ -54,7 +54,7 @@
> >  /* Error Recovery */
> >  #define MAX_RETRY					1
> >
> > -#define SF_PDMA_REG_BASE(ch)	(pdma->membase + (PDMA_CHAN_OFFSET * (ch)))
> > +#define SF_PDMA_REG_BASE(ch)	(pdma->membase + PDMA_BASE_OFFSET + (PDMA_CHAN_OFFSET * (ch)))
> 
> why not set membase to pdma->membase + PDMA_BASE_OFFSET directly? are there
> registers between pdma->membase and pdma->membase + PDMA_BASE_OFFSET?
> 
> Frank
> 
> >
> >  struct pdma_regs {
> >  	/* read-write regs */
> >
> > --
> > 2.43.0
> >

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

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node

[Conor Dooley]

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

On Fri, Feb 20, 2026 at 03:30:50PM -0500, Frank Li wrote:
> On Sat, Feb 21, 2026 at 03:43:57AM +0800, Max Hsu wrote:
> > The FU740 SoC includes a 4-channel Platform DMA (PDMA) controller.
> > Add the device node to enable DMA support.
> >
> > Signed-off-by: Max Hsu <max.hsu@sifive.com>
> > ---
> >  arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
> > index 6150f3397bff..30d0d6837c57 100644
> > --- a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
> > +++ b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
> > @@ -329,6 +329,15 @@ gpio: gpio@10060000 {
> >  			clocks = <&prci FU740_PRCI_CLK_PCLK>;
> >  			status = "disabled";
> >  		};
> > +		dma: dma-controller@3000000 {
> 
> not sure sifive, generally require orderby hex address, and need empty
> line between child node.

Correct on the ordering front, but the latter appears to be the style in
the file at the moment so I don't care about that.

> 
> Frank
> > +			compatible = "sifive,fu740-c000-pdma", "sifive,pdma0";
> > +			reg = <0x0 0x3000000 0x0 0x100000>;
> > +			interrupt-parent = <&plic0>;
> > +			interrupts = <11>, <12>, <13>, <14>, <15>, <16>, <17>, <18>;
> > +			dma-channels = <4>;
> > +			clocks = <&prci FU740_PRCI_CLK_PCLK>;
> > +			#dma-cells = <1>;
> > +		};
> >  		pcie@e00000000 {
> >  			compatible = "sifive,fu740-pcie";
> >  			#address-cells = <3>;
> >
> > --
> > 2.43.0
> >

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

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support

[Conor Dooley]

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

On Sat, Feb 21, 2026 at 03:43:56AM +0800, Max Hsu wrote:
> Add "sifive,fu740-c000-pdma" compatible string.
> 
> Signed-off-by: Max Hsu <max.hsu@sifive.com>
> ---
>  Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml b/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
> index 609e38901434..b6c49060bc6f 100644
> --- a/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
> +++ b/Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
> @@ -36,6 +36,7 @@ properties:
>            - enum:
>                - microchip,mpfs-pdma
>                - sifive,fu540-c000-pdma
> +              - sifive,fu740-c000-pdma

Based on the driver change and related discussion, I'm not sure that
this is the correct change to make.
This shouldn't be applied til the discussion on the driver is figured
out.

Cheers,
Conor.
pw-bot: not-applicable

>            - const: sifive,pdma0
>      description:
>        Should be "sifive,<chip>-pdma" and "sifive,pdma<version>".
> 
> -- 
> 2.43.0
> 

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

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv