[PATCH AUTOSEL 6.18-5.15] dmaengine: dw-axi-dmac: fix PM for system sleep and channel alloc

[PATCH AUTOSEL 6.18-5.15] dmaengine: dw-axi-dmac: fix PM for system sleep and channel alloc

[Sasha Levin]

From: Tze Yee Ng <tze.yee.ng@altera.com>

[ Upstream commit df0c2dc68770cf43f15df40b184df030b850ea05 ]

The driver only had runtime PM callbacks. If a channel stayed allocated
across system suspend/resume, the runtime usage count could remain
non-zero while hardware state (DMAC_CFG, clocks) was lost, and
axi_dma_runtime_resume() would not run to restore it.

Add system-sleep PM ops that use pm_runtime_force_suspend() and
pm_runtime_force_resume() so suspend/resume reuses the existing
axi_dma_suspend() and axi_dma_resume() paths.

Replace pm_runtime_get() with pm_runtime_resume_and_get() in
dma_chan_alloc_chan_resources() so clocks are enabled before a client
can immediately submit a transfer and touch MMIO.

Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
Link: https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508d8ac6371896.1779688569.git.tze.yee.ng@altera.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `dmaengine: dw-axi-dmac: fix PM for system
sleep and channel alloc`

**Local tree:** `v6.18.44` (`stable/linux-6.18.y`, detached HEAD)
**Mainline commit:** `df0c2dc68770c` (merged by Vinod Koul, 2026-06-11)
**Status in this tree:** Buggy code is present; fix is **not** yet
applied (`NOT_IN_TREE`)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Parse the subject line
**Record:** `[dmaengine: dw-axi-dmac]` `[fix]` — Correct power-
management handling for system sleep and DMA channel allocation.

### Step 1.2: Parse all commit message tags
**Record:**
- **Fixes:** — absent (expected for manual review)
- **Reported-by:** — absent
- **Tested-by:** — absent
- **Reviewed-by / Acked-by:** — absent in commit message
- **Link:** https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508d8ac63
  71896.1779688569.git.tze.yee.ng@altera.com
- **Cc: stable:** — absent (not a negative signal)
- **Signed-off-by:** Tze Yee Ng (author), Vinod Koul (subsystem
  maintainer, committer)
- **Notable:** Merged by dmaengine maintainer; patch 2/2 in a reviewed
  series

### Step 1.3: Analyze commit body
**Record:**
- **Bug:** Driver registered only runtime PM callbacks. With a channel
  allocated across system suspend/resume, runtime usage count can stay
  non-zero while hardware state (DMAC_CFG, clocks) is lost;
  `axi_dma_runtime_resume()` is then skipped.
- **Symptom:** DMA controller left with clocks off and/or DMAC_CFG not
  restored after resume; subsequent DMA/MMIO can fail or hang.
- **Second bug:** `pm_runtime_get()` in
  `dma_chan_alloc_chan_resources()` bumps the usage counter without
  resuming; a client can submit a transfer immediately and touch MMIO
  before clocks are enabled.
- **Root cause:** Missing system-sleep PM ops; incorrect runtime PM API
  usage on channel allocation.
- **Version info:** None stated; driver has had this pattern since 2018.

### Step 1.4: Detect hidden bug fixes
**Record:** Not disguised — explicitly described as a PM bug fix. The
`pm_runtime_resume_and_get()` change also adds missing
`pm_runtime_put()` on error paths (refcount balance), which is proper
error-path cleanup tied to the fix.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory the changes
**Record:**
- **File:** `drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c` (+9 / -2)
- **Functions modified:** `dma_chan_alloc_chan_resources()`,
  `dw_axi_dma_pm_ops`
- **Scope:** Single-file, surgical fix

### Step 2.2: Code flow change per hunk

**Hunk 1 — `dma_chan_alloc_chan_resources()`:**
- **Before:** Check idle → allocate descriptor pool → `pm_runtime_get()`
  (counter only, no resume) → return 0. Error paths did not balance
  runtime PM.
- **After:** `pm_runtime_resume_and_get()` first (resume + increment);
  on `-EBUSY` / `-ENOMEM`, `pm_runtime_put()` before return.
- **Path affected:** Normal DMA client channel allocation (common
  client-driver path).

**Hunk 2 — `dw_axi_dma_pm_ops`:**
- **Before:** Only `SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend,
  axi_dma_runtime_resume, NULL)`.
- **After:** Adds `SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
  pm_runtime_force_resume)`.
- **Path affected:** System suspend/resume (S3/hibernate on affected
  SoCs).

### Step 2.3: Bug mechanism
**Record:**
- **Category (a):** Error-path refcount fix — `pm_runtime_put()` on
  allocation failure after `resume_and_get`.
- **Category (b):** PM / suspend-resume correctness — system sleep now
  forces runtime suspend/resume regardless of usage count.
- **Category (c):** Reference-counting / PM API misuse —
  `pm_runtime_get()` does not resume; `pm_runtime_resume_and_get()`
  does.
- **Specific mechanism:** After system sleep, hardware is reset but
  software refcount says device is "active," so runtime resume is
  skipped and `axi_dma_resume()` (clocks + DMAC enable) never runs.

### Step 2.4: Fix quality
**Record:**
- **Quality:** High. Uses the standard kernel pattern documented in
  `DEFINE_RUNTIME_DEV_PM_OPS()` / `pm_runtime.h` comments.
- **Minimal:** 9 lines, no API changes.
- **Regression risk:** Very low. `pm_runtime_force_suspend/resume` are
  well-tested core PM helpers; error-path `pm_runtime_put()` is correct
  pairing.
- **Red flags:** None.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame changed lines
**Record:**
- `dma_chan_alloc_chan_resources()` and `pm_runtime_get()`: introduced
  in `1fe20f1b84548` (2018-03-06, "Introduce DW AXI DMAC driver").
- `dw_axi_dma_pm_ops` with runtime-only ops: same commit, 2018.
- Bug has been present since driver introduction in this tree.

### Step 3.2: Follow Fixes: tag
**Record:** No `Fixes:` tag. N/A.

### Step 3.3: File history for related changes
**Record:**
- Recent stable-tree changes: StarFive JH8100/JH7110 support, per-
  channel IRQ, array overrun fix.
- Patch 1 of series (`dc6d681e1571c` — "drop redundant DMAC enable in
  block start") is **not** in this tree (`PATCH1_NOT_IN_TREE`).
- This commit (patch 2) is **standalone**; it does not depend on patch
  1. Patch 1 without patch 2 would expose the PM gap more; patch 2 alone
  is sufficient and correct for 6.18.y.

### Step 3.4: Author's other commits
**Record:** Tze Yee Ng — Altera/Intel contributor; author of
stratix10-svc fixes. Vinod Koul committed and is dmaengine maintainer.

### Step 3.5: Prerequisites
**Record:** No prerequisites. `pm_runtime_force_suspend`,
`pm_runtime_force_resume`, and `pm_runtime_resume_and_get` all exist in
this tree's `include/linux/pm_runtime.h`. Patch applies cleanly (`git
apply --check` exit 0).

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original patch discussion
**Record:**
- **b4 dig URL:** https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508
  d8ac6371896.1779688569.git.tze.yee.ng@altera.com
- **Series:** v2 0/2 "clean up DMAC enable and PM" (2026-05-25)
- **Revisions:** v2 only found by b4 dig `-a`
- **Key feedback:** Patch 2 added per review feedback from Sashiko
  Watanabe (AI review bot flagged issues; patch 2 addresses PM gap
  identified in review)
- **Maintainer:** Vinod Koul replied "Applied, thanks!" applying both
  patches
- **Stable nomination in thread:** None found
- **NAKs:** None found

### Step 4.2: Reviewers from b4 dig -w
**Record:** CC'd: Eugeniy Paltsev (Synopsys, original driver author),
Vinod Koul, Frank Li, dmaengine@vger.kernel.org, linux-
kernel@vger.kernel.org.

### Step 4.3: Bug report
**Record:** No external bug report or syzbot link. Bug identified
through code review / PM analysis during series review.

### Step 4.4: Related patches / series
**Record:** 2-patch series. Only patch 2 is needed for this backport
decision. Patch 1 is optional cleanup not present in 6.18.y.

### Step 4.5: Stable mailing list
**Record:** Not searched separately; no stable discussion found in
downloaded thread.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `dma_chan_alloc_chan_resources()`,
`dma_chan_free_chan_resources()` (unchanged, has matching
`pm_runtime_put`), `axi_dma_suspend()`, `axi_dma_resume()`,
`axi_dma_runtime_suspend/resume()`, `dw_axi_dma_pm_ops`.

### Step 5.2: Callers
**Record:** `dma_chan_alloc_chan_resources` is registered as
`device_alloc_chan_resources` in the dmaengine device ops (line 1565).
Called by any DMA client requesting a channel — SDHCI, SPI, audio, etc.
on affected SoCs.

### Step 5.3: Callees
**Record:** `pm_runtime_resume_and_get()` → `pm_runtime_get_active()` →
`__pm_runtime_resume()`; system sleep uses
`pm_runtime_force_suspend/resume` → existing `axi_dma_suspend/resume`
(clock disable/enable, `axi_dma_disable/enable`).

### Step 5.4: Call chain / reachability
**Record:**
1. **Suspend/resume:** Platform system sleep → driver
   `.suspend`/`.resume` → force runtime suspend/resume → restore clocks
   and DMAC.
2. **Channel alloc:** Userspace/driver → `dma_request_channel()` →
   `alloc_chan_resources()` → must have clocks before any transfer.
- **Userspace reachable:** Yes, indirectly via drivers using DMA on
  StarFive, Intel KMB, Altera/Intel FPGA platforms.

### Step 5.5: Similar patterns
**Record:** Other DMA drivers in this tree already use
`pm_runtime_resume_and_get()` in alloc paths (e.g. `zynqmp_dma.c`,
`tegra20-apb-dma.c`, `stm32-dma.c`) and/or
`SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)` (e.g. `dw_mmc-pltfm.c`, `idma64.c`). This fix
aligns dw-axi-dmac with established practice.

---

## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE

### Step 6.1: Does buggy code exist?
**Record:** **Yes.** Current tree at `v6.18.44` has:
- `pm_runtime_get()` at line 538 in `dma_chan_alloc_chan_resources()`
- Runtime-only `dw_axi_dma_pm_ops` at lines 1654–1656
- Affected platforms in OF table: `snps,axi-dma-1.01a`, `intel,kmb-axi-
  dma`, `starfive,jh7110-axi-dma`, `starfive,jh8100-axi-dma`

### Step 6.2: Backport complications
**Record:** Clean apply expected. `git apply --check` on mainline patch
succeeded. No structural divergence in the changed regions.

### Step 6.3: Related fixes already present?
**Record:** No equivalent fix found. `git merge-base --is-ancestor
df0c2dc68770c HEAD` → `NOT_IN_TREE`.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem criticality
**Record:** `drivers/dma/dw-axi-dmac` — **IMPORTANT** (DMA engine for
multiple embedded SoC platforms; suspend/resume and DMA are core to I/O
on those systems).

### Step 7.2: Subsystem activity
**Record:** Actively maintained in 6.18.y (StarFive JH8100, per-channel
IRQ, overrun fix in recent history).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** Users of dw-axi-dmac on Intel KMB, StarFive JH7110/JH8100,
and Synopsys/Altera AXI DMA platforms — embedded boards, FPGA SoCs.
Config: `CONFIG_DW_AXI_DMAC` (or built-in on those platforms).

### Step 8.2: Trigger conditions
**Record:**
1. DMA channel allocated, system enters suspend (S3/hibernate), then
   resumes — **common** on laptops/embedded devices.
2. Device runtime-suspended, client allocates channel and immediately
   submits transfer — **plausible** under autosuspend.
- **Unprivileged trigger:** Indirectly yes (e.g., triggering suspend or
  I/O that uses DMA).

### Step 8.3: Failure mode severity
**Record:**
- DMA failures after resume (broken I/O: storage, network, audio)
- MMIO with clocks disabled → bus hang, timeout, or oops
- **Severity: HIGH** (system-level I/O breakage; potential hang)

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** HIGH — fixes real suspend/resume breakage on shipping
  hardware
- **Risk:** LOW — 9-line, standard PM pattern, maintainer-merged
- **Ratio:** Strongly favors backport

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Fixes real PM bug present since driver introduction (2018)
- Affects multiple platforms in this stable tree
- Can break DMA and cause hangs after suspend/resume
- Small, obviously correct, maintainer-merged fix
- Applies cleanly to v6.18.44
- Uses established kernel PM APIs/patterns
- Standalone — does not require patch 1 of the series

**AGAINST backport:**
- No syzbot/user crash report (review-found bug)
- Driver-specific, not core kernel (but suspend/resume is critical for
  affected users)

**Unresolved:** No explicit `Tested-by` on hardware; no `Cc: stable`
nomination in thread.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — standard PM pattern; merged
   by maintainer after review
2. Fixes a real bug? **PASS** — suspend/resume state desync and alloc-
   without-resume
3. Important issue? **PASS** — HIGH: post-resume DMA failure / potential
   hang
4. Small and contained? **PASS** — 9 lines, one file
5. No new features/APIs? **PASS** — only PM ops wiring and correct API
   usage
6. Can apply to local tree? **PASS** — verified clean apply

### Step 9.3: Exception categories
**Record:** None (not a quirk/ID/DT/docs fix) — qualifies on straight
bug-fix merits.

### Step 9.4: Decision rationale

For **linux-6.18.y (v6.18.44)**, the buggy code is present and the fix
is absent. The commit addresses a longstanding power-management defect:
without system-sleep PM ops, suspend/resume can leave the DMAC with
clocks off and hardware unconfigured while the runtime PM counter
indicates the device is still active. The `pm_runtime_resume_and_get()`
change fixes a second, independently valid bug where channel allocation
does not ensure the device is resumed before clients can use it. The
change is minimal, follows patterns already used elsewhere in
`drivers/dma/`, and applies cleanly. This is appropriate stable
material.

---

## Verification

- **[Phase 1]** `git describe HEAD` → `v6.18.44`; parsed commit message
  from user query and `git show df0c2dc68770c`
- **[Phase 2]** Read current `dw-axi-dmac-platform.c` lines 516–564,
  1315–1356, 1654–1656; confirmed diff matches missing fix
- **[Phase 3]** `git blame` lines 516–540, 1654–1656 → `1fe20f1b84548`
  (2018); `git merge-base --is-ancestor df0c2dc68770c HEAD` →
  `NOT_IN_TREE`; patch 1 also `NOT_IN_TREE`
- **[Phase 3]** `git apply --check` on `df0c2dc68770c` patch → exit 0
  (clean apply)
- **[Phase 4]** `b4 dig -c df0c2dc68770c` → lore URL; `b4 dig -a` → v2
  series; `b4 dig -w` → maintainers CC'd; mbox → Vinod "Applied,
  thanks!"
- **[Phase 4]** WebFetch lkml.iu.edu cover letter → patch 2 addresses
  Sashiko Watanabe review feedback
- **[Phase 5]** `grep pm_runtime_resume_and_get drivers/dma/` → pattern
  used in peer drivers; read `axi_dma_enable/suspend/resume` code
- **[Phase 6]** Confirmed buggy `pm_runtime_get` and runtime-only PM ops
  in tree; OF compatibles present
- **[Phase 7]** `git log --oneline -20 -- drivers/dma/dw-axi-dmac/` →
  active maintenance
- **[Phase 8]** Failure mode derived from verified code paths
  (`axi_dma_suspend` disables clocks; `pm_runtime_get` does not resume
  per `pm_runtime.h` docs)

**YES**

 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index b0e689f48bb67..ea4e47ee0d710 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -516,11 +516,17 @@ static void dw_axi_dma_synchronize(struct dma_chan *dchan)
 static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
 {
 	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(chan->chip->dev);
+	if (ret < 0)
+		return ret;
 
 	/* ASSERT: channel is idle */
 	if (axi_chan_is_hw_enable(chan)) {
 		dev_err(chan2dev(chan), "%s is non-idle!\n",
 			axi_chan_name(chan));
+		pm_runtime_put(chan->chip->dev);
 		return -EBUSY;
 	}
 
@@ -531,12 +537,11 @@ static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
 					  64, 0);
 	if (!chan->desc_pool) {
 		dev_err(chan2dev(chan), "No memory for descriptors\n");
+		pm_runtime_put(chan->chip->dev);
 		return -ENOMEM;
 	}
 	dev_vdbg(dchan2dev(dchan), "%s: allocating\n", axi_chan_name(chan));
 
-	pm_runtime_get(chan->chip->dev);
-
 	return 0;
 }
 
@@ -1652,6 +1657,8 @@ static void dw_remove(struct platform_device *pdev)
 }
 
 static const struct dev_pm_ops dw_axi_dma_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
 };
 
-- 
2.53.0

[PATCH AUTOSEL 6.18-6.12] dmaengine: altera-msgdma: Use memcpy_toio for descriptor FIFO writes

[Sasha Levin]

From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

[ Upstream commit 94963138cce29f85605d76c94fa1d43a0335ead9 ]

The descriptor FIFO requires that all words of a descriptor are written
in order, with the control word written last to flush it into the DMA
engine. Using memcpy() with __force to __iomem is not the correct API
and does not guarantee appropriate MMIO access on all architectures.

Replace the descriptor body copy with memcpy_toio(), using
offsetof(struct msgdma_extended_desc, control) to exclude the control
word. This matches the previous sizeof(desc->hw_desc) - sizeof(u32)
length only when control is the last struct member; add a static_assert
to enforce that layout so a future field after control cannot silently
break FIFO ordering.

Keep writing the control word separately with write barriers, so it
remains the final word pushed into the FIFO.

Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
Link: https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab9bd76.1779697226.git.tze.yee.ng@altera.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

**Step 1.1 – Subject line**

Record: `[dmaengine: altera-msgdma] [Use] Use memcpy_toio for descriptor
FIFO writes` — subsystem is Altera mSGDMA DMAengine driver; action is
replacing incorrect MMIO copy API.

**Step 1.2 – Tags**

Record:
- **Fixes:** none
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by / Acked-by:** none
- **Link:** https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab
  9bd76.1779697226.git.tze.yee.ng@altera.com
- **Signed-off-by:** Adrian Ng Ho Yin, Tze Yee Ng, Vinod Koul
  (maintainer)
- **Cc: stable@vger.kernel.org:** absent (expected for manual review)
- Notable: no syzbot, no user bug report, no crash trace

**Step 1.3 – Body analysis**

Record:
- **Bug:** Descriptor FIFO hardware requires ordered 32-bit word writes;
  control word must be written last to flush the descriptor. Using
  `memcpy()` with `__force` cast to `__iomem` is not the correct MMIO
  API and does not guarantee proper access on all architectures.
- **Symptom:** Malformed or partially written descriptors can cause DMA
  to wrong addresses, transfer failures, or silent data corruption.
- **Root cause:** Body of descriptor copied via plain `memcpy()` to MMIO
  instead of `memcpy_toio()`.
- **Version info:** none in message

**Step 1.4 – Hidden bug fix?**

Record: **Yes.** Although framed as an API correction, it fixes a real
hardware-interaction bug. The pre-existing comment in the driver already
acknowledged uncertainty about `memcpy()` write ordering; this commit
completes that fix by using the proper MMIO helper.

---

## Phase 2: Diff Analysis

**Step 2.1 – Inventory**

Record:
- **File:** `drivers/dma/altera-msgdma.c` (+15 / −9, 24 lines touched)
- **Function modified:** `msgdma_copy_one()` only
- **Scope:** single-file, surgical fix

**Step 2.2 – Code flow change**

Record:
- **Hunk 1 (before FIFO wait):** adds duplicate `static_assert`
  verifying `control` is the last struct member.
- **Hunk 2 (before body copy):** adds second identical `static_assert`
  (copy-paste error acknowledged by author on list).
- **Hunk 3 (body copy):** `memcpy((void __force *)hw_desc, ...)` →
  `memcpy_toio(hw_desc, ..., offsetof(..., control))`; control word
  still written separately via `iowrite32()` + `wmb()`.
- **Path affected:** every descriptor submission to hardware FIFO
  (normal DMA operation path).

**Step 2.3 – Bug mechanism**

Record: **Memory safety / MMIO correctness fix.** Category: incorrect
MMIO access pattern. Plain `memcpy()` to `__iomem` may use CPU memory
copy optimizations (reordering, non-MMIO stores) that do not satisfy
hardware FIFO ordering requirements. `memcpy_toio()` uses architecture-
appropriate MMIO store sequences.

**Step 2.4 – Fix quality**

Record: Fix is logically correct and minimal. Regression risk is very
low — it replaces one copy primitive with the documented equivalent.
Minor quality blemish: duplicate `static_assert` blocks (author agreed
to fix in v3, but v2 was merged as-is). Pre-existing concerns (BE
endianness mix of `memcpy_toio` + `iowrite32`, `mdelay()` while holding
spinlock) are unchanged by this patch.

---

## Phase 3: Git History Investigation

**Step 3.1 – Blame**

Record: Buggy `memcpy()` path introduced in `6084fc2ec478d` (Aug 2017,
Stefan Roese, "Use macros instead of structs..."). Original driver added
in `a85c6f1b2921c` (Jul 2017). Bug has been present ~9 years.

**Step 3.2 – Fixes: tag**

Record: N/A — no Fixes: tag present.

**Step 3.3 – Related file history**

Record: Recent stable-tree changes to this file include descriptor
free/cleanup fixes (`54e4ada1a4206`, `d3ddfab0969b1`), spinlock IRQ
variant fix (`261d3a85d9598`). No related fix for MMIO copy already
present. Standalone patch (v2 of 1-patch series).

**Step 3.4 – Author context**

Record: Authors are Altera/Intel engineers (hardware vendor). Vinod Koul
(dmaengine maintainer) applied the patch. Authors are not regular
altera-msgdma maintainers but submitted from hardware expertise.

**Step 3.5 – Dependencies**

Record: No prerequisites. Uses `memcpy_toio()` and `static_assert`, both
available in Linux 6.18. Applies cleanly (`git apply --check` passed).

---

## Phase 4: Mailing List and External Research

**Step 4.1 – Original discussion**

Record:
- **URL:** https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab9
  bd76.1779697226.git.tze.yee.ng@altera.com
- **Series:** v2 only (v1 not in thread); committed version matches v2
- **Maintainer response:** Vinod Koul — "Applied, thanks!"
- **No stable nomination** from reviewers
- **No NAKs** from human reviewers

**Step 4.2 – Reviewers**

Record: CC'd: Olivier Dautricourt, Stefan Roese (original driver
author), Vinod Koul, Frank Li, dmaengine@, linux-kernel@. Appropriate
maintainers included.

**Step 4.3 – Bug report**

Record: No external bug report. Sashiko AI review flagged duplicate
static_assert (Low) and pre-existing MMIO/endianness/spinlock+mdelay
issues (High, pre-existing). Author Tze Yee Ng agreed duplicate assert
was copy-paste error; offered v3 with single assert and optional
`iowrite32()` loop if Frank Li preferred. Frank Li asked author to
review Sashiko comments; no further human NAK before merge.

**Step 4.4 – Related patches**

Record: Standalone. Author indicated FIFO polling and stricter MMIO
access could be separate follow-ups.

**Step 4.5 – Stable list history**

Record: Not searched separately; no stable nomination found in patch
thread.

---

## Phase 5: Code Semantic Analysis

**Step 5.1 – Key functions**

Record: `msgdma_copy_one()` modified; callers unchanged.

**Step 5.2 – Callers**

Record:
- `msgdma_copy_desc_to_fifo()` → called from `msgdma_start_transfer()`
- `msgdma_start_transfer()` called from:
  - `msgdma_issue_pending()` (under `spin_lock_irqsave`)
  - `msgdma_irq_handler()` (under `spin_lock`)
- Reachable on every DMA transfer submission and from IRQ when
  controller becomes idle.

**Step 5.3 – Callees**

Record: `ioread32()` (FIFO full check), `mdelay(1)` (wait loop),
`memcpy_toio()` (new), `wmb()`, `iowrite32()` (control word flush).

**Step 5.4 – Reachability**

Record: Triggered whenever userspace/kernel submits DMA operations
through the dmaengine API on Altera mSGDMA hardware
(`CONFIG_ALTERA_MSGDMA`). Common operational path, not init-only or
error-only.

**Step 5.5 – Similar patterns**

Record: Other dma drivers use `memcpy_toio()` for MMIO (e.g., edma). The
forced `memcpy()` to `__iomem` pattern is explicitly discouraged in
kernel MMIO documentation.

---

## Phase 6: Cross-Reference Against Local Tree

**Step 6.1 – Buggy code in this tree?**

Record: **Yes.** Local tree is **Linux 6.18.44** (`git describe HEAD` →
v6.18.44). Buggy `memcpy((void __force *)hw_desc, ...)` present at lines
518–519 of `drivers/dma/altera-msgdma.c`. Bug present since driver
introduction (2017).

**Step 6.2 – Backport complications**

Record: **Clean apply** confirmed via `git format-patch -1 94963138cce29
| git apply --check`. No conflicting recent changes to this function in
6.18.y.

**Step 6.3 – Related fixes already present?**

Record: Commit `94963138cce29` is **not** in `stable/linux-6.18.y` (`git
merge-base --is-ancestor` returned exit 1). No equivalent fix found via
grep for `memcpy_toio` in this file.

---

## Phase 7: Subsystem and Maintainer Context

**Step 7.1 – Subsystem criticality**

Record: **dmaengine / Altera mSGDMA driver** — **PERIPHERAL** (niche
FPGA/embedded hardware, `CONFIG_ALTERA_MSGDMA`). However, DMA descriptor
corruption affects memory integrity for users of that hardware.

**Step 7.2 – Subsystem activity**

Record: Driver receives occasional maintenance (descriptor lifecycle,
spinlock fixes in 2024–2025). Mature, low-churn code.

---

## Phase 8: Impact and Risk Assessment

**Step 8.1 – Who is affected**

Record: Users with `CONFIG_ALTERA_MSGDMA` on Altera/Intel FPGA platforms
(PCIe-connected Cyclone and similar, ARM and x86 per original driver
testing). Config-specific, platform-specific.

**Step 8.2 – Trigger conditions**

Record: Every DMA descriptor submission. Not timing-dependent race;
architecture-dependent MMIO behavior. Unprivileged users can trigger via
dmaengine consumers using this controller.

**Step 8.3 – Failure mode severity**

Record: Corrupted descriptor fields → DMA to/from wrong physical
addresses → **data corruption** (CRITICAL for affected users). Could
also cause transfer failure or hardware stall. No crash/oops required
for impact.

**Step 8.4 – Risk-benefit**

Record:
- **Benefit:** Prevents potential silent memory corruption on non-x86 or
  strict MMIO architectures; uses correct kernel API; hardware vendor
  fix.
- **Risk:** Very low — 1-line functional change plus compile-time
  assert; applies cleanly.
- **Ratio:** Favorable for affected hardware users; limited broader
  impact due to niche driver.

---

## Phase 9: Final Synthesis

**Step 9.1 – Evidence summary**

**FOR backport:**
- Real MMIO correctness bug present since 2017 in this tree
- Potential DMA data corruption (critical for affected users)
- Small, obviously correct, clean-apply fix
- Hardware vendor submitted with maintainer acceptance
- Original driver comment acknowledged memcpy ordering uncertainty
- Commit not yet in 6.18.y

**AGAINST backport:**
- No user bug report, crash, or syzbot finding
- Very niche driver (FPGA embedded)
- Driver has functioned with workaround since 2017 without reported
  failures
- Duplicate `static_assert` in merged code (cosmetic)
- Does not address all MMIO concerns raised in AI review (pre-existing)
- No explicit stable nomination

**Unresolved:** No verified report of the bug manifesting in production;
impact inferred from API semantics and hardware requirements.

**Step 9.2 – Stable rules checklist**

1. Obviously correct and tested? **PASS** — API fix is standard; no
   runtime Tested-by, but logically sound.
2. Fixes a real bug affecting users? **PASS** — incorrect MMIO access on
   descriptor submission path.
3. Important issue? **PASS** — potential DMA data corruption.
4. Small and contained? **PASS** — 1 file, ~24 lines.
5. No new features/APIs? **PASS** — behavior correction only.
6. Can apply to local tree? **PASS** — clean apply verified.

**Step 9.3 – Exception categories**

Record: Not a device ID, quirk, DT, build, or docs fix. Standard driver
correctness bug fix.

**Step 9.4 – Decision rationale**

For Linux 6.18.y, the buggy code exists, the fix applies cleanly, and
the failure mode (DMA writing to wrong memory) is serious for the Altera
mSGDMA users this driver serves. While the driver is niche and lacks a
field bug report, the original code explicitly doubted `memcpy()`
ordering, the hardware vendor corrected the MMIO access pattern, and the
patch is minimal with negligible regression risk. Stable trees routinely
accept such hardware-interaction fixes when data corruption is the
consequence.

---

## Verification

- [Phase 1] Parsed commit `94963138cce29` on master; tags: Link, SOBs
  only; no Reported-by/syzbot
- [Phase 2] Diff: 1 file, `msgdma_copy_one()` only; `memcpy` →
  `memcpy_toio`; duplicate static_assert confirmed in `git show
  94963138cce29`
- [Phase 3] `git blame`: buggy memcpy from `6084fc2ec478d` (2017);
  driver from `a85c6f1b2921c` (2017)
- [Phase 3] `git log -20 -- drivers/dma/altera-msgdma.c`: no prior MMIO
  fix
- [Phase 3] `git format-patch -1 94963138cce29 | git apply --check`:
  clean apply
- [Phase 4] `b4 dig -c 94963138cce29`: lore URL found; v2 series
- [Phase 4] `b4 dig -c 94963138cce29 -w`: Vinod Koul, Frank Li, Stefan
  Roese CC'd
- [Phase 4] `b4 dig -m /tmp/altera-msgdma.mbx`: Vinod applied; author
  acknowledged duplicate assert; no stable nomination
- [Phase 5] Call chain verified in source:
  `msgdma_issue_pending`/`msgdma_irq_handler` → `msgdma_start_transfer`
  → `msgdma_copy_desc_to_fifo` → `msgdma_copy_one`
- [Phase 6] `git describe HEAD`: v6.18.44; `make kernelversion`: 6.18.44
- [Phase 6] Buggy `memcpy((void __force *)hw_desc` confirmed at line 518
  in local tree
- [Phase 6] `git merge-base --is-ancestor 94963138cce29
  stable/linux-6.18.y`: NOT ancestor (exit 1)
- [Phase 6] `grep memcpy_toio drivers/dma/altera-msgdma.c`: no match in
  local tree (fix absent)
- [Phase 7] `grep CONFIG_ALTERA_MSGDMA drivers/dma/Kconfig`: tristate,
  depends on HAS_IOMEM
- [Phase 8] Struct layout verified: `control` is last field at offset 28
  in 32-byte `msgdma_extended_desc`

**YES**

 drivers/dma/altera-msgdma.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c
index a203fdd84950e..e1811a9be0bbe 100644
--- a/drivers/dma/altera-msgdma.c
+++ b/drivers/dma/altera-msgdma.c
@@ -498,6 +498,11 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
 {
 	void __iomem *hw_desc = mdev->desc;
 
+	/* Ensure control is the last field — required for correct FIFO flush ordering */
+	static_assert(offsetof(struct msgdma_extended_desc, control) ==
+		      sizeof(struct msgdma_extended_desc) - sizeof(u32),
+		      "control must be the last field in msgdma_extended_desc");
+
 	/*
 	 * Check if the DESC FIFO it not full. If its full, we need to wait
 	 * for at least one entry to become free again
@@ -506,17 +511,18 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
 	       MSGDMA_CSR_STAT_DESC_BUF_FULL)
 		mdelay(1);
 
+	/* Ensure control is the last field — required for correct FIFO flush ordering */
+	static_assert(offsetof(struct msgdma_extended_desc, control) ==
+			sizeof(struct msgdma_extended_desc) - sizeof(u32),
+			"control must be the last field in msgdma_extended_desc");
+
 	/*
-	 * The descriptor needs to get copied into the descriptor FIFO
-	 * of the DMA controller. The descriptor will get flushed to the
-	 * FIFO, once the last word (control word) is written. Since we
-	 * are not 100% sure that memcpy() writes all word in the "correct"
-	 * order (address from low to high) on all architectures, we make
-	 * sure this control word is written last by single coding it and
-	 * adding some write-barriers here.
+	 * Copy the descriptor into the descriptor FIFO of the DMA controller,
+	 * excluding the control word. The FIFO is flushed and the descriptor
+	 * becomes valid once the control word is written last.
 	 */
-	memcpy((void __force *)hw_desc, &desc->hw_desc,
-	       sizeof(desc->hw_desc) - sizeof(u32));
+	memcpy_toio(hw_desc, &desc->hw_desc,
+		    offsetof(struct msgdma_extended_desc, control));
 
 	/* Write control word last to flush this descriptor into the FIFO */
 	mdev->idle = false;
-- 
2.53.0

Re: [PATCH AUTOSEL 6.18-5.15] dmaengine: dw-axi-dmac: fix PM for system sleep and channel alloc

[sashiko-bot]

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

Pre-existing issues:
- [High] The shared interrupt handler accesses hardware registers without checking if the device is active, and the suspend callback fails to synchronize IRQs.
--

commit 11e86e3a3829f6e35823522311eef18dacb1afef
Author: Tze Yee Ng <tze.yee.ng@altera.com>

dmaengine: dw-axi-dmac: fix PM for system sleep and channel alloc
    
The driver only had runtime PM callbacks. If a channel stayed allocated
across system suspend/resume, the runtime usage count could remain
non-zero while hardware state (DMAC_CFG, clocks) was lost, and
axi_dma_runtime_resume() would not run to restore it.

Add system-sleep PM ops that use pm_runtime_force_suspend() and
pm_runtime_force_resume() so suspend/resume reuses the existing
axi_dma_suspend() and axi_dma_resume() paths.

Link: https://patch.msgid.link/18bf778a3a1cc2f377ef8eb0d1508d8ac6371896.1779688569.git.tze.yee.ng@altera.com

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index b23536645ff7c..4fd77f5b67c47 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c

[ ... ]

> @@ -1654,6 +1659,8 @@ static void dw_remove(struct platform_device *pdev)
>  }
>  
>  static const struct dev_pm_ops dw_axi_dma_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
>  };

[Severity: High]
This is a pre-existing issue, but does the shared interrupt handler have a
potential race condition with device suspend?

The driver requests a shared interrupt:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_req_irqs() {
    ...
		ret = devm_request_irq(chip->dev, chip->irq[i], dw_axi_dma_interrupt,
				IRQF_SHARED, KBUILD_MODNAME, chip);
    ...
}

However, the interrupt handler accesses hardware registers unconditionally:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dw_axi_dma_interrupt() {
    ...
	axi_dma_irq_disable(chip);
    ...
}

If another device on the same shared IRQ line triggers an interrupt while this
DMA controller is runtime suspended (with clocks disabled), could this cause a
bus hang or system crash when accessing unclocked registers?

Additionally, the suspend routine cuts clocks without synchronizing IRQs:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_dma_suspend() {
    ...
	clk_disable_unprepare(chip->core_clk);
	clk_disable_unprepare(chip->cfgr_clk);
    ...
}

Could an in-flight IRQ handler crash when the clocks are cut mid-execution?

Should the interrupt handler use pm_runtime_get_if_active() to check if the
device is active before proceeding, and should the suspend callback call
synchronize_irq() to ensure no handler is executing before disabling the clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-209-sashal@kernel.org?part=1

Re: [PATCH AUTOSEL 6.18-6.12] dmaengine: altera-msgdma: Use memcpy_toio for descriptor FIFO writes

[sashiko-bot]

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

New issues:
- [Low] Duplicate `static_assert` declaration.

Pre-existing issues:
- [Critical] Inconsistent MMIO accessor variants used for the same FIFO, causing data corruption on big-endian architectures.
- [Critical] Handling of zero-length transfers allocates a descriptor without accounting for it, causing out-of-bounds memory writes if the free list is empty.
- [High] Unbounded `mdelay(1)` loop while holding a spinlock with interrupts disabled.
- [High] Interrupt handler is registered before the spinlock and lists it relies on are initialized.
--

commit 53559846cb40149ec4edf05249c1dcd2c2956df1
Author: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

dmaengine: altera-msgdma: Use memcpy_toio for descriptor FIFO writes

This commit fixes an issue where the descriptor body copy was using memcpy()
with a cast to __iomem, which does not guarantee proper MMIO access across
all architectures. It replaces the body copy with memcpy_toio(), excluding
the final control word which is still written separately.

Link: https://patch.msgid.link/f6f3b4a2e2eb0eb1a51976de3f5d1ef5bab9bd76.1779697226.git.tze.yee.ng@altera.com

> diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c
> index a203fdd84950e..e1811a9be0bbe 100644
> --- a/drivers/dma/altera-msgdma.c
> +++ b/drivers/dma/altera-msgdma.c
> @@ -498,6 +498,11 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
>  {
>  	void __iomem *hw_desc = mdev->desc;
>  
> +	/* Ensure control is the last field — required for correct FIFO flush ordering */
> +	static_assert(offsetof(struct msgdma_extended_desc, control) ==
> +		      sizeof(struct msgdma_extended_desc) - sizeof(u32),
> +		      "control must be the last field in msgdma_extended_desc");
> +
>  	/*
>  	 * Check if the DESC FIFO it not full. If its full, we need to wait
>  	 * for at least one entry to become free again
>  	 */
>  	while (ioread32(mdev->csr + MSGDMA_CSR_STATUS) &
>  	       MSGDMA_CSR_STAT_DESC_BUF_FULL)
>  		mdelay(1);

[Severity: High]
This is a pre-existing issue, but could this unbounded mdelay(1) loop cause a
hard lockup?

This function is called by msgdma_issue_pending() and msgdma_irq_handler()
which both hold mdev->lock and disable local interrupts. Busy-waiting for
milliseconds inside an IRQ-disabled atomic context will stall the CPU and
cause a hard lockup if the hardware stalls.

>  
> +	/* Ensure control is the last field — required for correct FIFO flush ordering */
> +	static_assert(offsetof(struct msgdma_extended_desc, control) ==
> +			sizeof(struct msgdma_extended_desc) - sizeof(u32),
> +			"control must be the last field in msgdma_extended_desc");
> +

[Severity: Low]
this isn't a bug, but you've duplicated this static_assert statement from
a few lines above.

>  	/*
> -	 * The descriptor needs to get copied into the descriptor FIFO
> -	 * of the DMA controller. The descriptor will get flushed to the
> -	 * FIFO, once the last word (control word) is written. Since we
> -	 * are not 100% sure that memcpy() writes all word in the "correct"
> -	 * order (address from low to high) on all architectures, we make
> -	 * sure this control word is written last by single coding it and
> -	 * adding some write-barriers here.
> +	 * Copy the descriptor into the descriptor FIFO of the DMA controller,
> +	 * excluding the control word. The FIFO is flushed and the descriptor
> +	 * becomes valid once the control word is written last.
>  	 */
> -	memcpy((void __force *)hw_desc, &desc->hw_desc,
> -	       sizeof(desc->hw_desc) - sizeof(u32));
> +	memcpy_toio(hw_desc, &desc->hw_desc,
> +		    offsetof(struct msgdma_extended_desc, control));
>  
>  	/* Write control word last to flush this descriptor into the FIFO */
>  	mdev->idle = false;
>  	wmb();
>  	iowrite32(desc->hw_desc.control, hw_desc +
>  		  offsetof(struct msgdma_extended_desc, control));

[Severity: Critical]
This is a pre-existing issue, but does mixing memcpy_toio() and iowrite32() on
the same hardware FIFO corrupt descriptor data on big-endian platforms?

The patch uses memcpy_toio() which preserves CPU byte order for the first 28
bytes of the descriptor, and iowrite32() which performs CPU-to-little-endian
byteswapping on big-endian architectures for the final 4-byte control word.

Writing to the same hardware FIFO with mixed accessor variants violates the
subsystem guidelines and ensures the descriptor will be malformed on
big-endian platforms.

[Severity: High]
This is a pre-existing issue, but is the interrupt handler registered before
the spinlock and lists it relies on are initialized?

drivers/dma/altera-msgdma.c:msgdma_probe() {
[ ... ]
	ret = devm_request_irq(&pdev->dev, mdev->irq, msgdma_irq_handler,
			       0, dev_name(&pdev->dev), mdev);
	if (ret)
		return ret;

	tasklet_setup(&mdev->irq_tasklet, msgdma_tasklet);

	dma_cookie_init(&mdev->dmachan);

	spin_lock_init(&mdev->lock);
[ ... ]
}

If a shared or spurious interrupt fires immediately during device probe,
msgdma_irq_handler() will attempt to lock the uninitialized mdev->lock and
access uninitialized list heads.

[Severity: Critical]
This is a pre-existing issue, but does a zero-length transfer bypass the free
list capacity check and cause out-of-bounds memory writes?

drivers/dma/altera-msgdma.c:msgdma_prep_memcpy() {
[ ... ]
	desc_cnt = DIV_ROUND_UP(len, MSGDMA_MAX_TRANS_LEN);

	spin_lock_irqsave(&mdev->lock, irqflags);
	if (desc_cnt > mdev->desc_free_cnt) {
		spin_unlock_irqrestore(&mdev->lock, irqflags);
		dev_dbg(mdev->dev, "mdev %p descs are not available\n", mdev);
		return NULL;
	}
	mdev->desc_free_cnt -= desc_cnt;
	spin_unlock_irqrestore(&mdev->lock, irqflags);

	do {
		/* Allocate and populate the descriptor */
		new = msgdma_get_descriptor(mdev);
[ ... ]
}

If len == 0, desc_cnt evaluates to 0. The bounds check passes even if
mdev->desc_free_cnt is 0. The function then unconditionally enters the do-while
loop and allocates a descriptor without accounting for it.

When msgdma_get_descriptor() calls list_first_entry() on the empty free_list,
it will return an invalid pointer aliasing the device struct. Subsequent
list_del() and hardware descriptor initialization will corrupt adjacent memory
inside mdev.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-290-sashal@kernel.org?part=1