* [PATCH v2 1/6] dmaengine: sun6i: fix physical channel index in interrupt handler
2026-10-23 18:42 [PATCH v2 0/6] dmaengine: sun6i: fix interrupt routing and channel lifetime James Hilliard
@ 2026-10-23 18:42 ` James Hilliard
2026-10-23 18:42 ` [PATCH v2 2/6] dmaengine: sun6i: handle the last partial interrupt bank James Hilliard
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: James Hilliard @ 2026-10-23 18:42 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine
Cc: Hongling Zeng, Chengfeng Ye, Nguyen Minh Tien, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, James Hilliard,
stable, Frank Li
From: Hongling Zeng <zenghongling@kylinos.cn>
The interrupt handler iterates over register banks in the outer loop,
but the physical channel lookup uses only the inner loop index. It
therefore handles every bank as if it belonged to the first channels.
For controllers with eight channels per bank, interrupts from channels
8 and above are routed to channels 0-7. On A733, each bank covers one
channel, so every interrupt is routed to channel 0. This can falsely
complete descriptors for unrelated channels and corrupt their data.
Use the absolute channel index, calculated from the bank number and the
controller's configured number of channels per bank.
Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
[james: use the configured bank size and describe the A733 case]
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 8752969366ff..9d94a37fcf10 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -603,7 +603,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
sun6i_write_irq_stat(sdev, i, status);
for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
- pchan = sdev->pchans + j;
+ pchan = sdev->pchans + i * sdev->cfg->num_channels_per_reg + j;
vchan = pchan->vchan;
if (vchan && (status & vchan->irq_type)) {
if (vchan->cyclic) {
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 2/6] dmaengine: sun6i: handle the last partial interrupt bank
2026-10-23 18:42 [PATCH v2 0/6] dmaengine: sun6i: fix interrupt routing and channel lifetime James Hilliard
2026-10-23 18:42 ` [PATCH v2 1/6] dmaengine: sun6i: fix physical channel index in interrupt handler James Hilliard
@ 2026-10-23 18:42 ` James Hilliard
2026-10-23 20:05 ` Frank Li
2026-10-23 18:42 ` [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling James Hilliard
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: James Hilliard @ 2026-10-23 18:42 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine
Cc: Hongling Zeng, Chengfeng Ye, Nguyen Minh Tien, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, James Hilliard,
Sashiko
The number of physical channels need not be a multiple of the number
covered by an interrupt-status register. H3 has twelve channels and
eight channels per bank. Rounding down the number of banks leaves its
channels 8 through 11 without completion handling.
Round up the number of banks to visit, and stop processing the last bank
at num_pchans so unused status bits cannot index past the physical-channel
array. Use the controller's configured bank size for both limits and
the physical-channel index.
Fixes: f008db8c00c1 ("dmaengine: sun6i: Add support for Allwinner H3 (sun8i) variant")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260706065650.34B131F00A3A@smtp.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 9d94a37fcf10..0a9645171180 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -587,12 +587,13 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
{
struct sun6i_dma_dev *sdev = dev_id;
+ u32 channels_per_reg = sdev->cfg->num_channels_per_reg;
struct sun6i_vchan *vchan;
struct sun6i_pchan *pchan;
int i, j, ret = IRQ_NONE;
u32 status;
- for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
+ for (i = 0; i < DIV_ROUND_UP(sdev->num_pchans, channels_per_reg); i++) {
status = sun6i_read_irq_stat(sdev, i);
if (!status)
continue;
@@ -602,8 +603,9 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
sun6i_write_irq_stat(sdev, i, status);
- for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
- pchan = sdev->pchans + i * sdev->cfg->num_channels_per_reg + j;
+ for (j = 0; j < channels_per_reg &&
+ i * channels_per_reg + j < sdev->num_pchans && status; j++) {
+ pchan = sdev->pchans + i * channels_per_reg + j;
vchan = pchan->vchan;
if (vchan && (status & vchan->irq_type)) {
if (vchan->cyclic) {
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/6] dmaengine: sun6i: handle the last partial interrupt bank
2026-10-23 18:42 ` [PATCH v2 2/6] dmaengine: sun6i: handle the last partial interrupt bank James Hilliard
@ 2026-10-23 20:05 ` Frank Li
0 siblings, 0 replies; 10+ messages in thread
From: Frank Li @ 2026-10-23 20:05 UTC (permalink / raw)
To: James Hilliard
Cc: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine, Hongling Zeng, Chengfeng Ye,
Nguyen Minh Tien, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, Sashiko
On Fri, Oct 23, 2026 at 12:42:36PM -0600, James Hilliard wrote:
> The number of physical channels need not be a multiple of the number
> covered by an interrupt-status register. H3 has twelve channels and
> eight channels per bank. Rounding down the number of banks leaves its
> channels 8 through 11 without completion handling.
>
> Round up the number of banks to visit, and stop processing the last bank
> at num_pchans so unused status bits cannot index past the physical-channel
> array. Use the controller's configured bank size for both limits and
> the physical-channel index.
>
> Fixes: f008db8c00c1 ("dmaengine: sun6i: Add support for Allwinner H3 (sun8i) variant")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/20260706065650.34B131F00A3A@smtp.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> drivers/dma/sun6i-dma.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 9d94a37fcf10..0a9645171180 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -587,12 +587,13 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
> static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
> {
> struct sun6i_dma_dev *sdev = dev_id;
> + u32 channels_per_reg = sdev->cfg->num_channels_per_reg;
> struct sun6i_vchan *vchan;
> struct sun6i_pchan *pchan;
> int i, j, ret = IRQ_NONE;
> u32 status;
>
> - for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
> + for (i = 0; i < DIV_ROUND_UP(sdev->num_pchans, channels_per_reg); i++) {
> status = sun6i_read_irq_stat(sdev, i);
> if (!status)
> continue;
> @@ -602,8 +603,9 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>
> sun6i_write_irq_stat(sdev, i, status);
>
> - for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
> - pchan = sdev->pchans + i * sdev->cfg->num_channels_per_reg + j;
> + for (j = 0; j < channels_per_reg &&
> + i * channels_per_reg + j < sdev->num_pchans && status; j++) {
look like check status first is a little better
for (j = 0; status && j < channels_per_reg &&
i * channels_per_reg + j < sdev->num_pchans; j++)
Frank
> + pchan = sdev->pchans + i * channels_per_reg + j;
> vchan = pchan->vchan;
> if (vchan && (status & vchan->irq_type)) {
> if (vchan->cyclic) {
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling
2026-10-23 18:42 [PATCH v2 0/6] dmaengine: sun6i: fix interrupt routing and channel lifetime James Hilliard
2026-10-23 18:42 ` [PATCH v2 1/6] dmaengine: sun6i: fix physical channel index in interrupt handler James Hilliard
2026-10-23 18:42 ` [PATCH v2 2/6] dmaengine: sun6i: handle the last partial interrupt bank James Hilliard
@ 2026-10-23 18:42 ` James Hilliard
2026-10-23 20:32 ` Frank Li
2026-10-23 18:42 ` [PATCH v2 4/6] dmaengine: sun6i: track paused virtual channels James Hilliard
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: James Hilliard @ 2026-10-23 18:42 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine
Cc: Hongling Zeng, Chengfeng Ye, Nguyen Minh Tien, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, James Hilliard,
Sashiko
The allocator changes vchan->phy and pchan->vchan under sdev->lock, but
termination reads the physical channel before taking either lock. The IRQ
handler also takes an owner snapshot without protecting it. Termination
can therefore miss a new assignment or detach an owner while an interrupt
is about to complete its descriptor. An IRQ status snapshot can survive
reassignment and be applied to the next transfer.
Use sdev->lock for physical-channel ownership, pending channels and the
shared interrupt registers, nesting vc.lock inside it for descriptor
lists and cookie updates. Allocate and start each pending channel in one
critical section. Read and acknowledge IRQ status under the same outer
lock, and protect cyclic notifications as well as ordinary completions.
Clear the active descriptor pointer before publishing finite completion
to virt-dma, whose tasklet may free it independently of the allocator.
An assigned channel without an active descriptor is ready for the
allocator to start its next transfer or release it. This replaces the
completed-descriptor pointer and keeps residue queries and termination
from consulting a descriptor already handed to virt-dma.
Mask and clear a stopped channel's IRQs before releasing its ownership,
and clear pending status before starting a new descriptor. Schedule the
allocator after termination so other pending channels can use the released
physical channel without depending on another device interrupt.
This also keeps process-context acquisitions IRQ-safe. Client callbacks
remain outside these locks in the virtual-channel completion tasklet.
Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260706065650.34B131F00A3A@smtp.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 124 ++++++++++++++++++++++++++----------------------
1 file changed, 66 insertions(+), 58 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 0a9645171180..a21acb9ecc27 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -198,7 +198,6 @@ struct sun6i_pchan {
void __iomem *base;
struct sun6i_vchan *vchan;
struct sun6i_desc *desc;
- struct sun6i_desc *done;
};
struct sun6i_vchan {
@@ -217,6 +216,7 @@ struct sun6i_dma_dev {
struct clk *clk;
struct clk *clk_mbus;
int irq;
+ /* Channel assignment and register access; nests outside vc.lock. */
spinlock_t lock;
struct reset_control *rstc;
struct tasklet_struct task;
@@ -486,22 +486,23 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
if (!pchan)
return -EAGAIN;
- if (!desc) {
- pchan->desc = NULL;
- pchan->done = NULL;
+ if (!desc)
return -EAGAIN;
- }
list_del(&desc->node);
pchan->desc = to_sun6i_desc(&desc->tx);
- pchan->done = NULL;
sun6i_dma_dump_lli(vchan, pchan->desc->v_lli, pchan->desc->p_lli);
irq_reg = pchan->idx / sdev->cfg->num_channels_per_reg;
irq_offset = pchan->idx % sdev->cfg->num_channels_per_reg;
+ /* Do not deliver a previous transfer's interrupt to this descriptor. */
+ sun6i_write_irq_stat(sdev, irq_reg,
+ (DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
+ (irq_offset * DMA_IRQ_CHAN_WIDTH));
+
vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
irq_val = sun6i_read_irq_en(sdev, irq_reg);
@@ -524,15 +525,15 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
struct sun6i_dma_dev *sdev = from_tasklet(sdev, t, task);
struct sun6i_vchan *vchan;
struct sun6i_pchan *pchan;
- unsigned int pchan_alloc = 0;
unsigned int pchan_idx;
+ spin_lock_irq(&sdev->lock);
list_for_each_entry(vchan, &sdev->slave.channels, vc.chan.device_node) {
- spin_lock_irq(&vchan->vc.lock);
+ spin_lock(&vchan->vc.lock);
pchan = vchan->phy;
- if (pchan && pchan->done) {
+ if (pchan && !pchan->desc) {
if (sun6i_dma_start_desc(vchan)) {
/*
* No current txd associated with this channel
@@ -545,10 +546,9 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
pchan->vchan = NULL;
}
}
- spin_unlock_irq(&vchan->vc.lock);
+ spin_unlock(&vchan->vc.lock);
}
- spin_lock_irq(&sdev->lock);
for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
pchan = &sdev->pchans[pchan_idx];
@@ -557,31 +557,20 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
vchan = list_first_entry(&sdev->pending,
struct sun6i_vchan, node);
+ spin_lock(&vchan->vc.lock);
/* Remove from pending channels */
list_del_init(&vchan->node);
- pchan_alloc |= BIT(pchan_idx);
/* Mark this channel allocated */
pchan->vchan = vchan;
vchan->phy = pchan;
dev_dbg(sdev->slave.dev, "pchan %u: alloc vchan %p\n",
pchan->idx, &vchan->vc);
+ sun6i_dma_start_desc(vchan);
+ spin_unlock(&vchan->vc.lock);
}
spin_unlock_irq(&sdev->lock);
-
- for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
- if (!(pchan_alloc & BIT(pchan_idx)))
- continue;
-
- pchan = sdev->pchans + pchan_idx;
- vchan = pchan->vchan;
- if (vchan) {
- spin_lock_irq(&vchan->vc.lock);
- sun6i_dma_start_desc(vchan);
- spin_unlock_irq(&vchan->vc.lock);
- }
- }
}
static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
@@ -590,9 +579,12 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
u32 channels_per_reg = sdev->cfg->num_channels_per_reg;
struct sun6i_vchan *vchan;
struct sun6i_pchan *pchan;
+ struct sun6i_desc *desc;
+ unsigned long flags;
int i, j, ret = IRQ_NONE;
u32 status;
+ spin_lock_irqsave(&sdev->lock, flags);
for (i = 0; i < DIV_ROUND_UP(sdev->num_pchans, channels_per_reg); i++) {
status = sun6i_read_irq_stat(sdev, i);
if (!status)
@@ -607,15 +599,17 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
i * channels_per_reg + j < sdev->num_pchans && status; j++) {
pchan = sdev->pchans + i * channels_per_reg + j;
vchan = pchan->vchan;
- if (vchan && (status & vchan->irq_type)) {
+ desc = pchan->desc;
+ if (vchan && desc && (status & vchan->irq_type)) {
+ spin_lock(&vchan->vc.lock);
if (vchan->cyclic) {
- vchan_cyclic_callback(&pchan->desc->vd);
+ vchan_cyclic_callback(&desc->vd);
} else {
- spin_lock(&vchan->vc.lock);
- vchan_cookie_complete(&pchan->desc->vd);
- pchan->done = pchan->desc;
- spin_unlock(&vchan->vc.lock);
+ /* Completion transfers ownership to virt-dma. */
+ pchan->desc = NULL;
+ vchan_cookie_complete(&desc->vd);
}
+ spin_unlock(&vchan->vc.lock);
}
status = status >> DMA_IRQ_CHAN_WIDTH;
@@ -625,6 +619,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
tasklet_schedule(&sdev->task);
ret = IRQ_HANDLED;
}
+ spin_unlock_irqrestore(&sdev->lock, flags);
return ret;
}
@@ -931,19 +926,20 @@ static int sun6i_dma_pause(struct dma_chan *chan)
{
struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
- struct sun6i_pchan *pchan = vchan->phy;
+ struct sun6i_pchan *pchan;
unsigned long flags;
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: pause\n", &vchan->vc);
+ spin_lock_irqsave(&sdev->lock, flags);
+ pchan = vchan->phy;
if (pchan) {
writel(DMA_CHAN_PAUSE_PAUSE,
pchan->base + DMA_CHAN_PAUSE);
} else {
- spin_lock_irqsave(&sdev->lock, flags);
list_del_init(&vchan->node);
- spin_unlock_irqrestore(&sdev->lock, flags);
}
+ spin_unlock_irqrestore(&sdev->lock, flags);
return 0;
}
@@ -952,23 +948,24 @@ static int sun6i_dma_resume(struct dma_chan *chan)
{
struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
- struct sun6i_pchan *pchan = vchan->phy;
+ struct sun6i_pchan *pchan;
unsigned long flags;
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: resume\n", &vchan->vc);
- spin_lock_irqsave(&vchan->vc.lock, flags);
+ spin_lock_irqsave(&sdev->lock, flags);
+ spin_lock(&vchan->vc.lock);
+ pchan = vchan->phy;
if (pchan) {
writel(DMA_CHAN_PAUSE_RESUME,
pchan->base + DMA_CHAN_PAUSE);
} else if (!list_empty(&vchan->vc.desc_issued)) {
- spin_lock(&sdev->lock);
list_add_tail(&vchan->node, &sdev->pending);
- spin_unlock(&sdev->lock);
}
- spin_unlock_irqrestore(&vchan->vc.lock, flags);
+ spin_unlock(&vchan->vc.lock);
+ spin_unlock_irqrestore(&sdev->lock, flags);
return 0;
}
@@ -977,17 +974,17 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
{
struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
- struct sun6i_pchan *pchan = vchan->phy;
+ struct sun6i_pchan *pchan;
+ u32 irq_reg, irq_shift, irq_mask;
unsigned long flags;
LIST_HEAD(head);
spin_lock_irqsave(&sdev->lock, flags);
+ spin_lock(&vchan->vc.lock);
+ pchan = vchan->phy;
list_del_init(&vchan->node);
- spin_unlock_irqrestore(&sdev->lock, flags);
- spin_lock_irqsave(&vchan->vc.lock, flags);
-
- if (pchan && pchan->desc && pchan->desc != pchan->done) {
+ if (pchan && pchan->desc) {
struct virt_dma_desc *vd = &pchan->desc->vd;
vchan_terminate_vdesc(vd);
@@ -1000,15 +997,25 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
writel(DMA_CHAN_ENABLE_STOP, pchan->base + DMA_CHAN_ENABLE);
writel(DMA_CHAN_PAUSE_RESUME, pchan->base + DMA_CHAN_PAUSE);
+ irq_reg = pchan->idx / sdev->cfg->num_channels_per_reg;
+ irq_shift = (pchan->idx % sdev->cfg->num_channels_per_reg) *
+ DMA_IRQ_CHAN_WIDTH;
+ irq_mask = (DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) << irq_shift;
+ sun6i_write_irq_en(sdev, irq_reg,
+ sun6i_read_irq_en(sdev, irq_reg) & ~irq_mask);
+ sun6i_write_irq_stat(sdev, irq_reg, irq_mask);
+
vchan->phy = NULL;
pchan->vchan = NULL;
pchan->desc = NULL;
- pchan->done = NULL;
}
- spin_unlock_irqrestore(&vchan->vc.lock, flags);
+ spin_unlock(&vchan->vc.lock);
+ spin_unlock_irqrestore(&sdev->lock, flags);
vchan_dma_desc_free_list(&vchan->vc, &head);
+ if (!atomic_read(&sdev->tasklet_shutdown))
+ tasklet_schedule(&sdev->task);
return 0;
}
@@ -1017,8 +1024,9 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
dma_cookie_t cookie,
struct dma_tx_state *state)
{
+ struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
- struct sun6i_pchan *pchan = vchan->phy;
+ struct sun6i_pchan *pchan;
struct sun6i_dma_lli *lli;
struct virt_dma_desc *vd;
struct sun6i_desc *txd;
@@ -1026,11 +1034,12 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
unsigned long flags;
size_t bytes = 0;
+ spin_lock_irqsave(&sdev->lock, flags);
+ spin_lock(&vchan->vc.lock);
ret = dma_cookie_status(chan, cookie, state);
if (ret == DMA_COMPLETE || !state)
- return ret;
-
- spin_lock_irqsave(&vchan->vc.lock, flags);
+ goto out;
+ pchan = vchan->phy;
vd = vchan_find_desc(&vchan->vc, cookie);
txd = to_sun6i_desc(&vd->tx);
@@ -1044,9 +1053,10 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
bytes = sun6i_get_chan_size(pchan);
}
- spin_unlock_irqrestore(&vchan->vc.lock, flags);
-
dma_set_residue(state, bytes);
+out:
+ spin_unlock(&vchan->vc.lock);
+ spin_unlock_irqrestore(&sdev->lock, flags);
return ret;
}
@@ -1057,25 +1067,23 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
unsigned long flags;
- spin_lock_irqsave(&vchan->vc.lock, flags);
+ spin_lock_irqsave(&sdev->lock, flags);
+ spin_lock(&vchan->vc.lock);
if (vchan_issue_pending(&vchan->vc)) {
- spin_lock(&sdev->lock);
-
if (!vchan->phy && list_empty(&vchan->node)) {
list_add_tail(&vchan->node, &sdev->pending);
tasklet_schedule(&sdev->task);
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: issued\n",
&vchan->vc);
}
-
- spin_unlock(&sdev->lock);
} else {
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: nothing to issue\n",
&vchan->vc);
}
- spin_unlock_irqrestore(&vchan->vc.lock, flags);
+ spin_unlock(&vchan->vc.lock);
+ spin_unlock_irqrestore(&sdev->lock, flags);
}
static void sun6i_dma_free_chan_resources(struct dma_chan *chan)
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling
2026-10-23 18:42 ` [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling James Hilliard
@ 2026-10-23 20:32 ` Frank Li
2026-10-23 21:57 ` James Hilliard
0 siblings, 1 reply; 10+ messages in thread
From: Frank Li @ 2026-10-23 20:32 UTC (permalink / raw)
To: James Hilliard
Cc: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine, Hongling Zeng, Chengfeng Ye,
Nguyen Minh Tien, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, Sashiko
On Fri, Oct 23, 2026 at 12:42:37PM -0600, James Hilliard wrote:
> The allocator changes vchan->phy and pchan->vchan under sdev->lock, but
> termination reads the physical channel before taking either lock. The IRQ
> handler also takes an owner snapshot without protecting it. Termination
> can therefore miss a new assignment or detach an owner while an interrupt
> is about to complete its descriptor. An IRQ status snapshot can survive
> reassignment and be applied to the next transfer.
>
> Use sdev->lock for physical-channel ownership, pending channels and the
> shared interrupt registers, nesting vc.lock inside it for descriptor
> lists and cookie updates. Allocate and start each pending channel in one
> critical section. Read and acknowledge IRQ status under the same outer
> lock, and protect cyclic notifications as well as ordinary completions.
>
> Clear the active descriptor pointer before publishing finite completion
> to virt-dma, whose tasklet may free it independently of the allocator.
> An assigned channel without an active descriptor is ready for the
> allocator to start its next transfer or release it. This replaces the
> completed-descriptor pointer and keeps residue queries and termination
> from consulting a descriptor already handed to virt-dma.
>
> Mask and clear a stopped channel's IRQs before releasing its ownership,
> and clear pending status before starting a new descriptor. Schedule the
> allocator after termination so other pending channels can use the released
> physical channel without depending on another device interrupt.
>
> This also keeps process-context acquisitions IRQ-safe. Client callbacks
> remain outside these locks in the virtual-channel completion tasklet.
>
> Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/20260706065650.34B131F00A3A@smtp.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
Is it possible split to more small change? And you almost add sdev->lock,
any place only use vc.lock without sdev->lock?
Frank
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling
2026-10-23 20:32 ` Frank Li
@ 2026-10-23 21:57 ` James Hilliard
0 siblings, 0 replies; 10+ messages in thread
From: James Hilliard @ 2026-10-23 21:57 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine, Hongling Zeng, Chengfeng Ye,
Nguyen Minh Tien, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, Sashiko
On Fri, Oct 23, 2026 at 2:33 PM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Fri, Oct 23, 2026 at 12:42:37PM -0600, James Hilliard wrote:
> > The allocator changes vchan->phy and pchan->vchan under sdev->lock, but
> > termination reads the physical channel before taking either lock. The IRQ
> > handler also takes an owner snapshot without protecting it. Termination
> > can therefore miss a new assignment or detach an owner while an interrupt
> > is about to complete its descriptor. An IRQ status snapshot can survive
> > reassignment and be applied to the next transfer.
> >
> > Use sdev->lock for physical-channel ownership, pending channels and the
> > shared interrupt registers, nesting vc.lock inside it for descriptor
> > lists and cookie updates. Allocate and start each pending channel in one
> > critical section. Read and acknowledge IRQ status under the same outer
> > lock, and protect cyclic notifications as well as ordinary completions.
> >
> > Clear the active descriptor pointer before publishing finite completion
> > to virt-dma, whose tasklet may free it independently of the allocator.
> > An assigned channel without an active descriptor is ready for the
> > allocator to start its next transfer or release it. This replaces the
> > completed-descriptor pointer and keeps residue queries and termination
> > from consulting a descriptor already handed to virt-dma.
> >
> > Mask and clear a stopped channel's IRQs before releasing its ownership,
> > and clear pending status before starting a new descriptor. Schedule the
> > allocator after termination so other pending channels can use the released
> > physical channel without depending on another device interrupt.
> >
> > This also keeps process-context acquisitions IRQ-safe. Client callbacks
> > remain outside these locks in the virtual-channel completion tasklet.
> >
> > Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/r/20260706065650.34B131F00A3A@smtp.kernel.org
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
>
> Is it possible split to more small change? And you almost add sdev->lock,
> any place only use vc.lock without sdev->lock?
Yes. virt-dma takes vc.lock alone in vchan_tx_prep(), vchan_tx_submit()
and vchan_complete() to protect its descriptor lists and cookies.
sdev->lock protects physical-channel ownership and shared registers.
The sun6i paths need both locks when updating that state together with
the virt-dma lists and cookies.
>
> Frank
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 4/6] dmaengine: sun6i: track paused virtual channels
2026-10-23 18:42 [PATCH v2 0/6] dmaengine: sun6i: fix interrupt routing and channel lifetime James Hilliard
` (2 preceding siblings ...)
2026-10-23 18:42 ` [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling James Hilliard
@ 2026-10-23 18:42 ` James Hilliard
2026-10-23 18:42 ` [PATCH v2 5/6] dmaengine: sun6i: synchronize callbacks before releasing channel resources James Hilliard
2026-10-23 18:42 ` [PATCH v2 6/6] dmaengine: sun6i: keep cyclic state with the prepared descriptor James Hilliard
5 siblings, 0 replies; 10+ messages in thread
From: James Hilliard @ 2026-10-23 18:42 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine
Cc: Hongling Zeng, Chengfeng Ye, Nguyen Minh Tien, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, James Hilliard
sun6i_dma_tx_status() only returns the cookie state, so a paused but
incomplete transfer is reported as DMA_IN_PROGRESS. The 8250 RX flush
path pauses the channel and calls its completion helper before terminating
it. That helper ignores an in-progress transfer to avoid consuming a new
RX operation. With sun6i it consequently drops the partial buffer and
leaves rx_running set, preventing another receive from being submitted.
Track pause state under the channel-assignment lock and report DMA_PAUSED
for incomplete cookies, including status queries without a residue output.
Flush the hardware pause request before returning. Completed cookies
remain DMA_COMPLETE.
Keep paused channels out of the pending queue and do not start a queued
descriptor when a paused channel's previous descriptor completes. Release
the completed physical channel so other virtual channels can use it, and
clear its hardware pause request before reassignment. The paused virtual
channel retains its queued descriptors until resume.
Resume requeues an unassigned channel only once and schedules the
allocator, so an idle controller does not require an unrelated completion
to resume it. Clear the pause state when terminating the channel.
Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a21acb9ecc27..157746c9423d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -208,6 +208,7 @@ struct sun6i_vchan {
u8 port;
u8 irq_type;
bool cyclic;
+ bool paused;
};
struct sun6i_dma_dev {
@@ -534,13 +535,17 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
pchan = vchan->phy;
if (pchan && !pchan->desc) {
- if (sun6i_dma_start_desc(vchan)) {
+ if (vchan->paused || sun6i_dma_start_desc(vchan)) {
/*
* No current txd associated with this channel
*/
dev_dbg(sdev->slave.dev, "pchan %u: free\n",
pchan->idx);
+ /* The next owner must not inherit the pause request. */
+ writel(DMA_CHAN_PAUSE_RESUME,
+ pchan->base + DMA_CHAN_PAUSE);
+
/* Mark this channel free */
vchan->phy = NULL;
pchan->vchan = NULL;
@@ -932,10 +937,12 @@ static int sun6i_dma_pause(struct dma_chan *chan)
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: pause\n", &vchan->vc);
spin_lock_irqsave(&sdev->lock, flags);
+ vchan->paused = true;
pchan = vchan->phy;
if (pchan) {
writel(DMA_CHAN_PAUSE_PAUSE,
pchan->base + DMA_CHAN_PAUSE);
+ readl(pchan->base + DMA_CHAN_PAUSE);
} else {
list_del_init(&vchan->node);
}
@@ -957,12 +964,14 @@ static int sun6i_dma_resume(struct dma_chan *chan)
spin_lock(&vchan->vc.lock);
pchan = vchan->phy;
+ vchan->paused = false;
if (pchan) {
writel(DMA_CHAN_PAUSE_RESUME,
pchan->base + DMA_CHAN_PAUSE);
- } else if (!list_empty(&vchan->vc.desc_issued)) {
+ } else if (!list_empty(&vchan->vc.desc_issued) && list_empty(&vchan->node)) {
list_add_tail(&vchan->node, &sdev->pending);
}
+ tasklet_schedule(&sdev->task);
spin_unlock(&vchan->vc.lock);
spin_unlock_irqrestore(&sdev->lock, flags);
@@ -991,6 +1000,7 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
}
vchan->cyclic = false;
+ vchan->paused = false;
vchan_get_all_descriptors(&vchan->vc, &head);
if (pchan) {
@@ -1037,6 +1047,8 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
spin_lock_irqsave(&sdev->lock, flags);
spin_lock(&vchan->vc.lock);
ret = dma_cookie_status(chan, cookie, state);
+ if (ret == DMA_IN_PROGRESS && vchan->paused)
+ ret = DMA_PAUSED;
if (ret == DMA_COMPLETE || !state)
goto out;
pchan = vchan->phy;
@@ -1071,7 +1083,7 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
spin_lock(&vchan->vc.lock);
if (vchan_issue_pending(&vchan->vc)) {
- if (!vchan->phy && list_empty(&vchan->node)) {
+ if (!vchan->paused && !vchan->phy && list_empty(&vchan->node)) {
list_add_tail(&vchan->node, &sdev->pending);
tasklet_schedule(&sdev->task);
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: issued\n",
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 5/6] dmaengine: sun6i: synchronize callbacks before releasing channel resources
2026-10-23 18:42 [PATCH v2 0/6] dmaengine: sun6i: fix interrupt routing and channel lifetime James Hilliard
` (3 preceding siblings ...)
2026-10-23 18:42 ` [PATCH v2 4/6] dmaengine: sun6i: track paused virtual channels James Hilliard
@ 2026-10-23 18:42 ` James Hilliard
2026-10-23 18:42 ` [PATCH v2 6/6] dmaengine: sun6i: keep cyclic state with the prepared descriptor James Hilliard
5 siblings, 0 replies; 10+ messages in thread
From: James Hilliard @ 2026-10-23 18:42 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine
Cc: Hongling Zeng, Chengfeng Ye, Nguyen Minh Tien, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, James Hilliard
The driver has no device_synchronize callback. dmaengine_terminate_sync()
therefore returns after stopping the channel without waiting for the
virtual-channel tasklet. That tasklet may already have removed completed
descriptors from the shared list and be executing their callbacks. For
example, serial8250_release_dma() frees the RX buffer and unmaps the TX
buffer immediately after terminate_sync(), while those callbacks can
still access them.
Use vchan_dma_synchronize() for device_synchronize. Channel termination
excludes the interrupt handler and allocator before detaching the channel,
so they cannot schedule another completion for that transfer. Also stop
the channel and synchronize when freeing its resources.
Keep terminated cyclic descriptors on desc_terminated until synchronization
finishes. vchan_complete() may retain that descriptor outside vc.lock.
Ordinary active descriptors have not published a callback and can still be
freed by terminate_all(). This avoids accumulating descriptors on each
8250 partial-RX flush, which uses the atomic termination API.
Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 157746c9423d..c244ef572492 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -996,12 +996,20 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
if (pchan && pchan->desc) {
struct virt_dma_desc *vd = &pchan->desc->vd;
- vchan_terminate_vdesc(vd);
+ /* A cyclic callback may still hold this descriptor outside vc.lock. */
+ if (vchan->cyclic)
+ vchan_terminate_vdesc(vd);
+ else
+ list_add_tail(&vd->node, &head);
}
vchan->cyclic = false;
vchan->paused = false;
- vchan_get_all_descriptors(&vchan->vc, &head);
+ /* Leave terminated cyclic descriptors for the sleepable synchronize. */
+ list_splice_tail_init(&vchan->vc.desc_allocated, &head);
+ list_splice_tail_init(&vchan->vc.desc_submitted, &head);
+ list_splice_tail_init(&vchan->vc.desc_issued, &head);
+ list_splice_tail_init(&vchan->vc.desc_completed, &head);
if (pchan) {
writel(DMA_CHAN_ENABLE_STOP, pchan->base + DMA_CHAN_ENABLE);
@@ -1100,14 +1108,10 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
static void sun6i_dma_free_chan_resources(struct dma_chan *chan)
{
- struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
- unsigned long flags;
-
- spin_lock_irqsave(&sdev->lock, flags);
- list_del_init(&vchan->node);
- spin_unlock_irqrestore(&sdev->lock, flags);
+ sun6i_dma_terminate_all(chan);
+ vchan_dma_synchronize(chan);
vchan_free_chan_resources(&vchan->vc);
}
@@ -1480,6 +1484,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
sdc->slave.device_pause = sun6i_dma_pause;
sdc->slave.device_resume = sun6i_dma_resume;
sdc->slave.device_terminate_all = sun6i_dma_terminate_all;
+ sdc->slave.device_synchronize = vchan_dma_synchronize;
sdc->slave.src_addr_widths = sdc->cfg->src_addr_widths;
sdc->slave.dst_addr_widths = sdc->cfg->dst_addr_widths;
sdc->slave.directions = BIT(DMA_DEV_TO_MEM) |
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 6/6] dmaengine: sun6i: keep cyclic state with the prepared descriptor
2026-10-23 18:42 [PATCH v2 0/6] dmaengine: sun6i: fix interrupt routing and channel lifetime James Hilliard
` (4 preceding siblings ...)
2026-10-23 18:42 ` [PATCH v2 5/6] dmaengine: sun6i: synchronize callbacks before releasing channel resources James Hilliard
@ 2026-10-23 18:42 ` James Hilliard
5 siblings, 0 replies; 10+ messages in thread
From: James Hilliard @ 2026-10-23 18:42 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Arnd Bergmann, Maxime Ripard, Jens Kuske,
Jean-Francois Moine
Cc: Hongling Zeng, Chengfeng Ye, Nguyen Minh Tien, dmaengine,
linux-arm-kernel, linux-sunxi, linux-kernel, James Hilliard
Preparing a cyclic transfer sets vchan->cyclic before the descriptor is
submitted. A finite transfer on that channel can then select package
interrupts and take the cyclic callback path instead of completing its
cookie. Merely preparing an unrelated future transfer must not change
the active transfer or the meaning of queued descriptors.
Store the flag in sun6i_desc and consult the active descriptor when
selecting interrupts, completing a transfer and terminating it. Finite
descriptors retain their zero-initialized flag. This also makes callback
retirement depend on the descriptor whose lifetime is being ended.
The shared state was identified during the H616 cyclic UART RX review.
Fixes: a90e173f3faf ("dmaengine: sun6i: Add cyclic capability")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c244ef572492..c9a72cfe3f44 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -191,6 +191,7 @@ struct sun6i_desc {
struct virt_dma_desc vd;
dma_addr_t p_lli;
struct sun6i_dma_lli *v_lli;
+ bool cyclic;
};
struct sun6i_pchan {
@@ -207,7 +208,6 @@ struct sun6i_vchan {
struct sun6i_pchan *phy;
u8 port;
u8 irq_type;
- bool cyclic;
bool paused;
};
@@ -504,7 +504,7 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
(DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
(irq_offset * DMA_IRQ_CHAN_WIDTH));
- vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
+ vchan->irq_type = pchan->desc->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
irq_val = sun6i_read_irq_en(sdev, irq_reg);
irq_val &= ~((DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
@@ -607,7 +607,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
desc = pchan->desc;
if (vchan && desc && (status & vchan->irq_type)) {
spin_lock(&vchan->vc.lock);
- if (vchan->cyclic) {
+ if (desc->cyclic) {
vchan_cyclic_callback(&desc->vd);
} else {
/* Completion transfers ownership to virt-dma. */
@@ -908,7 +908,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
prev->p_lli_next = txd->p_lli; /* cyclic list */
- vchan->cyclic = true;
+ txd->cyclic = true;
return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
@@ -997,13 +997,12 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
struct virt_dma_desc *vd = &pchan->desc->vd;
/* A cyclic callback may still hold this descriptor outside vc.lock. */
- if (vchan->cyclic)
+ if (pchan->desc->cyclic)
vchan_terminate_vdesc(vd);
else
list_add_tail(&vd->node, &head);
}
- vchan->cyclic = false;
vchan->paused = false;
/* Leave terminated cyclic descriptors for the sleepable synchronize. */
list_splice_tail_init(&vchan->vc.desc_allocated, &head);
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread