[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