* [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations
2026-02-20 19:43 [PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support Max Hsu
@ 2026-02-20 19:43 ` Max Hsu
2026-02-20 20:10 ` Frank Li
2026-02-20 19:43 ` [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts Max Hsu
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Max Hsu @ 2026-02-20 19:43 UTC (permalink / raw)
To: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations
2026-02-20 19:43 ` [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations Max Hsu
@ 2026-02-20 20:10 ` Frank Li
2026-02-20 22:26 ` Conor Dooley
0 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2026-02-20 20:10 UTC (permalink / raw)
To: Max Hsu
Cc: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations
2026-02-20 20:10 ` Frank Li
@ 2026-02-20 22:26 ` Conor Dooley
0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-02-20 22:26 UTC (permalink / raw)
To: Frank Li
Cc: Max Hsu, Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li,
Green Wan, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Palmer Debbelt, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts
2026-02-20 19:43 [PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support Max Hsu
2026-02-20 19:43 ` [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations Max Hsu
@ 2026-02-20 19:43 ` Max Hsu
2026-02-20 20:26 ` Frank Li
2026-02-20 19:43 ` [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers Max Hsu
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Max Hsu @ 2026-02-20 19:43 UTC (permalink / raw)
To: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
Max Hsu, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts
2026-02-20 19:43 ` [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts Max Hsu
@ 2026-02-20 20:26 ` Frank Li
0 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2026-02-20 20:26 UTC (permalink / raw)
To: Max Hsu
Cc: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
stable
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers
2026-02-20 19:43 [PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support Max Hsu
2026-02-20 19:43 ` [PATCH 1/5] dmaengine: sf-pdma: add missing PDMA base offset to register calculations Max Hsu
2026-02-20 19:43 ` [PATCH 2/5] dmaengine: sf-pdma: fix race between done and error interrupts Max Hsu
@ 2026-02-20 19:43 ` Max Hsu
2026-02-20 20:28 ` Frank Li
2026-02-20 19:43 ` [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support Max Hsu
2026-02-20 19:43 ` [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node Max Hsu
4 siblings, 1 reply; 13+ messages in thread
From: Max Hsu @ 2026-02-20 19:43 UTC (permalink / raw)
To: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
Max Hsu, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers
2026-02-20 19:43 ` [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers Max Hsu
@ 2026-02-20 20:28 ` Frank Li
0 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2026-02-20 20:28 UTC (permalink / raw)
To: Max Hsu
Cc: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
stable
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support
2026-02-20 19:43 [PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support Max Hsu
` (2 preceding siblings ...)
2026-02-20 19:43 ` [PATCH 3/5] dmaengine: sf-pdma: fix NULL pointer dereference in error and done handlers Max Hsu
@ 2026-02-20 19:43 ` Max Hsu
2026-02-20 22:28 ` Conor Dooley
2026-02-20 19:43 ` [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node Max Hsu
4 siblings, 1 reply; 13+ messages in thread
From: Max Hsu @ 2026-02-20 19:43 UTC (permalink / raw)
To: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support
2026-02-20 19:43 ` [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support Max Hsu
@ 2026-02-20 22:28 ` Conor Dooley
0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-02-20 22:28 UTC (permalink / raw)
To: Max Hsu
Cc: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-riscv,
dmaengine, linux-kernel, Paul Walmsley, devicetree
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node
2026-02-20 19:43 [PATCH 0/5] dmaengine: sf-pdma: critical fixes and FU740 support Max Hsu
` (3 preceding siblings ...)
2026-02-20 19:43 ` [PATCH 4/5] dt-bindings: dma: sifive,fu540-c000-pdma: add fu740 support Max Hsu
@ 2026-02-20 19:43 ` Max Hsu
2026-02-20 20:30 ` Frank Li
4 siblings, 1 reply; 13+ messages in thread
From: Max Hsu @ 2026-02-20 19:43 UTC (permalink / raw)
To: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree,
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node
2026-02-20 19:43 ` [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node Max Hsu
@ 2026-02-20 20:30 ` Frank Li
2026-02-20 22:27 ` Conor Dooley
0 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2026-02-20 20:30 UTC (permalink / raw)
To: Max Hsu
Cc: Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li, Green Wan,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Palmer Debbelt,
Conor Dooley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] riscv: dts: sifive: fu740: add PDMA device node
2026-02-20 20:30 ` Frank Li
@ 2026-02-20 22:27 ` Conor Dooley
0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-02-20 22:27 UTC (permalink / raw)
To: Frank Li
Cc: Max Hsu, Paul Walmsley, Samuel Holland, Vinod Koul, Frank Li,
Green Wan, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Palmer Debbelt, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
linux-riscv, dmaengine, linux-kernel, Paul Walmsley, devicetree
[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread