[PATCH] dmaengine: mmp_pdma: fix wrong extended DRCMR base for SpacemiT K3

[PATCH] dmaengine: mmp_pdma: fix wrong extended DRCMR base for SpacemiT K3

[Troy Mitchell]

K3 PDMA shares the same DRCMR layout as K1, where the extended DRCMR
base address is 0x1100. Commit 6587b8661a0b ("dmaengine: mmp_pdma: add
SpacemiT K3 support") incorrectly defined DRCMR_EXT_BASE_K3 as 0x1000,
causing all DRCMR accesses for channels >= 64 to be off by 0x100.

Drop the bogus DRCMR_EXT_BASE_K3 macro and reuse DRCMR_EXT_BASE_DEFAULT
for the K3 ops.

Fixes: 6587b8661a0b ("dmaengine: mmp_pdma: add SpacemiT K3 support")
Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
This is a minimal fix for the wrong DRCMR_EXT_BASE_K3 value introduced
by commit 6587b8661a0b ("dmaengine: mmp_pdma: add SpacemiT K3 support").
K3 PDMA shares the same extended DRCMR base (0x1100) as K1, so the K3
ops now reuses DRCMR_EXT_BASE_DEFAULT.

I deliberately kept the per-ops drcmr_ext_base field and the
DRCMR_EXT_BASE_DEFAULT macro to preserve the extensibility that was
introduced together with the helper refactor, in case a future PDMA
variant ends up using a different extended base. If reviewers prefer
to drop this abstraction entirely and collapse it back to a single
constant now that all known users share the same value, I am happy to
send a v2 that removes the drcmr_ext_base ops field as a cleanup.
---
 drivers/dma/mmp_pdma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 386e85cd4882..78e3e07e681d 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -52,7 +52,6 @@
 #define DCSR_EORINTR	BIT(9)	/* The end of Receive */
 
 #define DRCMR_BASE		0x0100
-#define DRCMR_EXT_BASE_K3	0x1000
 #define DRCMR_EXT_BASE_DEFAULT	0x1100
 #define DRCMR_REQ_LIMIT		64
 #define DRCMR_MAPVLD	BIT(7)	/* Map Valid (read / write) */
@@ -1219,7 +1218,7 @@ static const struct mmp_pdma_ops spacemit_k3_pdma_ops = {
 	.get_desc_dst_addr = get_desc_dst_addr_64,
 	.run_bits = (DCSR_RUN | DCSR_LPAEEN | DCSR_EORIRQEN | DCSR_EORSTOPEN),
 	.dma_width = 64,
-	.drcmr_ext_base = DRCMR_EXT_BASE_K3,
+	.drcmr_ext_base = DRCMR_EXT_BASE_DEFAULT,
 };
 
 static const struct of_device_id mmp_pdma_dt_ids[] = {

---
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
change-id: 20260615-k3-pdma-fix-drcmr-base-326536770427

Best regards,
--  
Troy Mitchell <troy.mitchell@linux.spacemit.com>

Re: [PATCH] dmaengine: mmp_pdma: fix wrong extended DRCMR base for SpacemiT K3

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The interrupt handler `mmp_pdma_chan_handler` dereferences `phy->vchan` without checking if it is NULL, leading to a kernel panic.
- [High] Disabling a DMA channel by clearing all `run_bits` simultaneously drops 64-bit addressing capabilities, potentially corrupting DMA transfers while the hardware halts.
- [High] The driver destroys the DMA descriptor pool without synchronously killing the channel's tasklet, allowing a running tasklet to access freed resources.
--

commit 8ee69ab96e073f81f565ee82dce7689d911272ac
Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>

dmaengine: mmp_pdma: fix wrong extended DRCMR base for SpacemiT K3

K3 PDMA shares the same DRCMR layout as K1, where the extended DRCMR
base address is 0x1100. Commit 6587b8661a0b ("dmaengine: mmp_pdma: add
SpacemiT K3 support") incorrectly defined DRCMR_EXT_BASE_K3 as 0x1000,
causing all DRCMR accesses for channels >= 64 to be off by 0x100. Drop the
bogus DRCMR_EXT_BASE_K3 macro and reuse DRCMR_EXT_BASE_DEFAULT for the K3 ops.

> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 386e85cd4882a..78e3e07e681df 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c

[ ... ]

> @@ -1219,7 +1218,7 @@ static const struct mmp_pdma_ops spacemit_k3_pdma_ops = {
>  	.get_desc_dst_addr = get_desc_dst_addr_64,
>  	.run_bits = (DCSR_RUN | DCSR_LPAEEN | DCSR_EORIRQEN | DCSR_EORSTOPEN),

[Severity: High]
This is a pre-existing issue, but can clearing all run_bits simultaneously
corrupt DMA transfers?

In disable_chan(), the driver stops the DMA by clearing pdev->ops->run_bits
from the DCSR register:

drivers/dma/mmp_pdma.c:disable_chan() {
    ...
	if (phy->vchan) {
		struct mmp_pdma_device *pdev;

		pdev = to_mmp_pdma_dev(phy->vchan->chan.device);
		writel(dcsr & ~pdev->ops->run_bits, phy->base + reg);
    ...
}

Since a DMA channel does not halt instantaneously when the RUN bit is
cleared, simultaneously clearing DCSR_LPAEEN strips the upper 32 bits from
the DMA addresses of any remaining in-flight bursts.

This causes the final bursts to overwrite unintended memory in the lower 4GB.

Should only DCSR_RUN be toggled when stopping a channel?

>  	.dma_width = 64,
> -	.drcmr_ext_base = DRCMR_EXT_BASE_K3,
> +	.drcmr_ext_base = DRCMR_EXT_BASE_DEFAULT,
>  };

[Severity: High]
This is a pre-existing issue, but can this code cause a kernel panic due
to a NULL pointer dereference?

In mmp_pdma_probe(), IRQs are requested for all channels while phy->vchan
is initialized to NULL. It is only set later when a client allocates a
channel.

If the hardware has pending interrupts from boot, or a spurious interrupt
fires, mmp_pdma_chan_handler() executes this:

drivers/dma/mmp_pdma.c:mmp_pdma_chan_handler() {
    ...
	if (clear_chan_irq(phy) != 0)
		return IRQ_NONE;

	tasklet_schedule(&phy->vchan->tasklet);
    ...
}

Since clear_chan_irq() returns 0 even if vchan is NULL, does the
unconditional dereference of phy->vchan->tasklet trigger a crash?


[Severity: High]
This is a pre-existing issue, but does this code allow a tasklet to access
freed resources?

In mmp_pdma_free_chan_resources(), the descriptor pool is destroyed:

drivers/dma/mmp_pdma.c:mmp_pdma_free_chan_resources() {
    ...
	dma_pool_destroy(chan->desc_pool);
	chan->desc_pool = NULL;
	chan->idle = true;
    ...
}

However, the driver never calls tasklet_kill(&chan->tasklet) to synchronize
with the tasklet.

If dma_do_tasklet() is running concurrently on another CPU, it may block
until mmp_pdma_free_chan_resources() completes. Once it resumes, it attempts
to free descriptors using dma_pool_free() on the destroyed pool.

Can this lead to a use-after-free or a kernel oops?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-k3-pdma-fix-drcmr-base-v1-1-ee1af124199f@linux.spacemit.com?part=1

Re: [PATCH] dmaengine: mmp_pdma: fix wrong extended DRCMR base for SpacemiT K3

[Troy Mitchell]

On Mon Jun 15, 2026 at 10:53 AM CST, Troy Mitchell wrote:
> K3 PDMA shares the same DRCMR layout as K1, where the extended DRCMR
> base address is 0x1100. Commit 6587b8661a0b ("dmaengine: mmp_pdma: add
> SpacemiT K3 support") incorrectly defined DRCMR_EXT_BASE_K3 as 0x1000,
> causing all DRCMR accesses for channels >= 64 to be off by 0x100.
>
> Drop the bogus DRCMR_EXT_BASE_K3 macro and reuse DRCMR_EXT_BASE_DEFAULT
> for the K3 ops.
>
> Fixes: 6587b8661a0b ("dmaengine: mmp_pdma: add SpacemiT K3 support")
To clarify: the previous version was not untested; it simply hadn't been
tested with SPI DMA specifically. This issue only affects DMA for SPI
devices, rather than all DMA-capable peripherals

                                          - Troy

Re: [PATCH] dmaengine: mmp_pdma: fix wrong extended DRCMR base for SpacemiT K3

[Guodong Xu]

Hi Troy,

Thanks for the patch.

On 2026-06-15 10:53, Troy Mitchell wrote:
>K3 PDMA shares the same DRCMR layout as K1, where the extended DRCMR

K1 never touches DMA request numbers higher than 45. It's not K1 who needs
the extended DRCMR. PXAxx needs them.

>base address is 0x1100. Commit 6587b8661a0b ("dmaengine: mmp_pdma: add
>SpacemiT K3 support") incorrectly defined DRCMR_EXT_BASE_K3 as 0x1000,

The SpacemiT K3 datasheet still 'incorrectly' carries the base 0x1000.
Refer to [1], Section 16.1.4.6 DMA Connectivity & Assignments
which still states 0x1000 as the base:

0x1004 Request for SSP0 TxReq 0xD4040000
0x1008 Request for SSP0 RxReq 0xD4040000

I would recommend you double check with the SpacemiT team to make sure.
It's better SpacemiT can fix both the online document and downloadable PDF
as well.

[1]: https://www.spacemit.com/community/document/info?lang=en&nodepath=hardware/key_stone/k3/k3_docs/k3_usermanual/16_peripherals/dma.md

>causing all DRCMR accesses for channels >= 64 to be off by 0x100.
>
>Drop the bogus DRCMR_EXT_BASE_K3 macro and reuse DRCMR_EXT_BASE_DEFAULT
>for the K3 ops.

>
>Fixes: 6587b8661a0b ("dmaengine: mmp_pdma: add SpacemiT K3 support")
>Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>---
>This is a minimal fix for the wrong DRCMR_EXT_BASE_K3 value introduced
>by commit 6587b8661a0b ("dmaengine: mmp_pdma: add SpacemiT K3 support").
>K3 PDMA shares the same extended DRCMR base (0x1100) as K1, so the K3
>ops now reuses DRCMR_EXT_BASE_DEFAULT.

Before we flip it, could you please confirm your source fo truth:
did you confirm 0x1100 on k3 silicon with a real DMA transaction,
or from the datasheet? It would help if you could share your
test setup.

BR,
Guodong Xu

>Best regards,
>--
>Troy Mitchell <troy.mitchell@linux.spacemit.com>
>
>
>_______________________________________________
>linux-riscv mailing list
>linux-riscv@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-riscv