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

[Conor Dooley]

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 at 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 at 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
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20260220/e743b5ec/attachment.sig>