Upstream: sun6i-dma interrupt attribution above channel 7 #40

New issue

Open

opened 2026-08-28 06:15:56 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-08-28 06:15:56 +00:00

Owner

Found while bringing up DMA on the A80, but it is not an A80 bug — which makes it, like
the mc_smp cpu_table fix in #27, reviewable purely on its own merits.

sun6i_dma_interrupt() walks the status registers with i but indexes the channel array with
j alone:

for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
	status = readl(sdev->base + DMA_IRQ_STAT(i));
	for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
		pchan = sdev->pchans + j;          /* i is missing */

Each register covers 8 channels, and sun6i_dma_start_desc() correctly enables the interrupt
in register idx / 8 at offset idx % 8. So a completion reported in register 1 — channels
8 to 15 — is credited to channel j, whose descriptor is then completed twice, and
BUG_ON(tx->cookie < DMA_MIN_COOKIE) fires.

It needs more than 8 channels busy at once, which is why it has survived. The A31 has
sixteen channels too.

Measured with dmatest, 4 threads on each of 53 channels, 400 iterations:


before	`kernel BUG at drivers/dma/dmaengine.h:54` within two minutes, every time
after	208 threads, 0 failures, board up

A second, weaker change in the same loop: the register count rounds down, so a controller
whose channel count is not a multiple of 8 (the H3 has 12) never reads its last status
register. Changed to DIV_ROUND_UP. ⚠️ That half is reasoned from the code only and has not
been seen on an H3 — it may deserve splitting out or dropping.

Blocked on finding the Fixes: tag. The tree is a shallow clone, so git blame stops at
the boundary commit and the introducing change cannot be named from here. That has to be
resolved from full history first.

Tree: dmaengine — list: dmaengine@vger, linux-arm-kernel, linux-sunxi
Patch: patches/linux-dma-sun6i-irq/
Notes: 43-audio-and-dma.md

⚠️ Nothing has been submitted upstream. See 42-upstreaming.md.

Found while bringing up DMA on the A80, but **it is not an A80 bug** — which makes it, like the `mc_smp` `cpu_table` fix in #27, reviewable purely on its own merits. `sun6i_dma_interrupt()` walks the status registers with `i` but indexes the channel array with `j` alone: ```c for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) { status = readl(sdev->base + DMA_IRQ_STAT(i)); for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) { pchan = sdev->pchans + j; /* i is missing */ ``` Each register covers 8 channels, and `sun6i_dma_start_desc()` correctly enables the interrupt in register `idx / 8` at offset `idx % 8`. So a completion reported in register 1 — channels 8 to 15 — is credited to channel `j`, whose descriptor is then completed twice, and `BUG_ON(tx->cookie < DMA_MIN_COOKIE)` fires. It needs more than 8 channels busy at once, which is why it has survived. **The A31 has sixteen channels too.** Measured with dmatest, 4 threads on each of 53 channels, 400 iterations: | | | |---|---| | before | `kernel BUG at drivers/dma/dmaengine.h:54` within two minutes, every time | | after | 208 threads, 0 failures, board up | A second, weaker change in the same loop: the register count rounds *down*, so a controller whose channel count is not a multiple of 8 (the H3 has 12) never reads its last status register. Changed to `DIV_ROUND_UP`. ⚠️ That half is reasoned from the code only and has not been seen on an H3 — it may deserve splitting out or dropping. **Blocked on finding the `Fixes:` tag.** The tree is a shallow clone, so `git blame` stops at the boundary commit and the introducing change cannot be named from here. That has to be resolved from full history first. - Tree: dmaengine — list: dmaengine@vger, linux-arm-kernel, linux-sunxi - Patch: `patches/linux-dma-sun6i-irq/` - Notes: [43-audio-and-dma.md](43-audio-and-dma.md) --- ⚠️ **Nothing has been submitted upstream.** See [42-upstreaming.md](42-upstreaming.md).

tiagoagueda added the

kernel

upstream

P3-normal

labels

2026-08-28 06:15:56 +00:00

tiagoagueda referenced this issue from a commit

2026-08-30 08:58:49 +00:00

Prepare the sun6i-dma interrupt fix for submission (#40)

tiagoagueda commented

2026-08-30 08:59:14 +00:00

Author

Owner

Prepared for submission 2026-08-30

The blocker was stale

This issue says the Fixes: tag cannot be named because the tree is a shallow clone. It is
not - 1.48 M commits reachable, no .git/shallow. The introducing commit had simply never
been looked for:

Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")

the original 2014 driver commit, which is where pchan = sdev->pchans + j came from.

Split into two patches

The issue suggested considering this and it turned out to be necessary: the single commit did
two things, and 42-upstreaming.md's own rule is one logical change per patch. They also have
different origins, which settles it:


`0001`	interrupt attribution fix	`Fixes: 555859308723` (2014, original driver)
`0002`	`DIV_ROUND_UP` register count	no `Fixes:` tag - see below

0002 deliberately carries no tag. The mismatch dates from commit 500fa9e76bbc ("dmaengine:
sun6i: Move number of pchans/vchans/request to device struct", 2017), but the change is
reasoned from the code and has never been observed - the A80 has sixteen channels, an exact
multiple, so it is unaffected. With no observed failure there is nothing to justify a stable
backport. It is last in the series so it can be dropped without disturbing 0001.

Cleaned up for sending

Rebased on a clean 45c13f3f9e3b in a separate worktree, so the exported patches no longer
carry the eight prerequisite-patch-id lines the old export had
The stale "No Fixes: tag ..." paragraph removed from the commit message
Cover letter written
checkpatch.pl --strict: clean on both, one remaining error each, covered below
drivers/dma/sun6i-dma.o compiles

Recipients from get_maintainer.pl:

Vinod Koul <vkoul@kernel.org>                 maintainer, dmaengine
Frank Li <Frank.Li@kernel.org>                reviewer, dmaengine
Chen-Yu Tsai <wens@kernel.org>                maintainer, sunxi
Jernej Skrabec <jernej.skrabec@gmail.com>     maintainer, sunxi
Samuel Holland <samuel@sholland.org>          maintainer, sunxi
dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org

Series is in patches/linux-dma-sun6i-irq/; the worktree is ~/a80/dma-series on the build
host, branch dma-sun6i-irq.

🔴 Two things left, both deliberately not done here

Signed-off-by: is missing, on purpose. It is a Developer Certificate of Origin assertion
and must be the submitter's own - so checkpatch reports Missing Signed-off-by: line(s) on
both patches, and that is now the only thing it reports. Add it with

git rebase --exec 'git commit --amend --no-edit -s' master

in the worktree, or let b4 prep / b4 send do it.

Rebase onto the maintainer tree first. 45c13f3f9e3b is some way behind, and the tree's
only remote is now Forgejo, so it cannot currently fetch upstream. Add
git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git as a fetch remote before
sending. (The handler is unchanged in mainline as of that base, so the patches apply, but the
base should be current.)

And the standing rule from 42-upstreaming.md, which applies here more than anywhere: do not
send a patch you cannot fully explain and defend in review. This one is small and the
reasoning is in the commit messages, but it should be re-derived by hand before it goes.

## Prepared for submission 2026-08-30 ### The blocker was stale This issue says the `Fixes:` tag cannot be named because the tree is a shallow clone. **It is not** - 1.48 M commits reachable, no `.git/shallow`. The introducing commit had simply never been looked for: ``` Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller") ``` the original 2014 driver commit, which is where `pchan = sdev->pchans + j` came from. ### Split into two patches The issue suggested considering this and it turned out to be necessary: the single commit did two things, and `42-upstreaming.md`'s own rule is one logical change per patch. They also have **different origins**, which settles it: | | | | |---|---|---| | `0001` | interrupt attribution fix | `Fixes: 555859308723` (2014, original driver) | | `0002` | `DIV_ROUND_UP` register count | **no `Fixes:` tag** - see below | `0002` deliberately carries no tag. The mismatch dates from commit `500fa9e76bbc` ("dmaengine: sun6i: Move number of pchans/vchans/request to device struct", 2017), but the change is reasoned from the code and has never been observed - the A80 has sixteen channels, an exact multiple, so it is unaffected. With no observed failure there is nothing to justify a stable backport. It is last in the series so it can be dropped without disturbing `0001`. ### Cleaned up for sending - Rebased on a clean `45c13f3f9e3b` in a separate worktree, so the exported patches no longer carry the eight `prerequisite-patch-id` lines the old export had - The stale "No Fixes: tag ..." paragraph removed from the commit message - Cover letter written - `checkpatch.pl --strict`: clean on both, **one** remaining error each, covered below - `drivers/dma/sun6i-dma.o` compiles Recipients from `get_maintainer.pl`: ``` Vinod Koul <vkoul@kernel.org> maintainer, dmaengine Frank Li <Frank.Li@kernel.org> reviewer, dmaengine Chen-Yu Tsai <wens@kernel.org> maintainer, sunxi Jernej Skrabec <jernej.skrabec@gmail.com> maintainer, sunxi Samuel Holland <samuel@sholland.org> maintainer, sunxi dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org ``` Series is in `patches/linux-dma-sun6i-irq/`; the worktree is `~/a80/dma-series` on the build host, branch `dma-sun6i-irq`. ### 🔴 Two things left, both deliberately not done here **`Signed-off-by:` is missing, on purpose.** It is a Developer Certificate of Origin assertion and must be the submitter's own - so `checkpatch` reports `Missing Signed-off-by: line(s)` on both patches, and that is now the *only* thing it reports. Add it with ``` git rebase --exec 'git commit --amend --no-edit -s' master ``` in the worktree, or let `b4 prep` / `b4 send` do it. **Rebase onto the maintainer tree first.** `45c13f3f9e3b` is some way behind, and the tree's only remote is now Forgejo, so it cannot currently fetch upstream. Add `git://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git` as a fetch remote before sending. (The handler is unchanged in mainline as of that base, so the patches apply, but the base should be current.) And the standing rule from `42-upstreaming.md`, which applies here more than anywhere: **do not send a patch you cannot fully explain and defend in review.** This one is small and the reasoning is in the commit messages, but it should be re-derived by hand before it goes.

tiagoagueda referenced this issue

2026-08-30 10:13:49 +00:00

Audit dma_mask on every DMA-capable driver, now that RAM extends above 4 GiB #63