[PATCH AUTOSEL 6.18-5.10] PCI: rockchip: Protect root bus removal with rescan lock

[PATCH AUTOSEL 6.18-5.10] PCI: rockchip: Protect root bus removal with rescan lock

[Sasha Levin]

From: Hans Zhang <18255117159@163.com>

[ Upstream commit 0bd9611587bb494c33566d825fe34b2705e4b167 ]

Hold the pci_rescan_remove_lock lock while stopping and removing a root bus
to avoid racing with concurrent rescan or hotplug operations triggered via
sysfs.  Such races may lead to use-after-free issues or system crashes.

Signed-off-by: Hans Zhang <18255117159@163.com>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
[bhelgaas: commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20260521161822.132996-8-18255117159@163.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Parse the Subject Line
**Record:** `[PCI: rockchip] [Protect] Protect root bus removal with
rescan lock` — subsystem is Rockchip PCIe host controller; action is
synchronization/protection (bug fix).

### Step 1.2: Parse All Commit Message Tags
**Record:**
- **Fixes:** none
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** none
- **Acked-by:** none
- **Link:**
  `https://patch.msgid.link/20260521161822.132996-8-18255117159@163.com`
- **Cc: stable:** none
- **Signed-off-by:** Hans Zhang, Manivannan Sadhasivam, Bjorn Helgaas
  (ignore pipeline-added SOBs per instructions)

Notable: Signed-off-by from PCI maintainer Bjorn Helgaas. No syzbot or
user bug reports.

### Step 1.3: Analyze Commit Body
**Record:**
- **Bug:** `rockchip_pcie_remove()` tears down the root bus without
  holding the global PCI rescan/remove mutex, allowing concurrent sysfs-
  driven rescan or hotplug to operate on the same bus hierarchy.
- **Symptom:** Use-after-free or system crash.
- **Root cause:** Missing `pci_lock_rescan_remove()` /
  `pci_unlock_rescan_remove()` around `pci_stop_root_bus()` +
  `pci_remove_root_bus()`.
- **Version info:** None in commit message.

### Step 1.4: Detect Hidden Bug Fixes
**Record:** Not disguised — explicitly a race-condition / crash fix, not
cleanup or optimization.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory the Changes
**Record:**
- **Files:** `drivers/pci/controller/pcie-rockchip-host.c` (+2 lines)
- **Functions:** `rockchip_pcie_remove()`
- **Scope:** Single-file, surgical fix (2 lines added)

### Step 2.2: Code Flow Change
**Record:**
- **Hunk (remove path):** Before — `pci_stop_root_bus()` and
  `pci_remove_root_bus()` run unlocked. After — same calls wrapped in
  `pci_lock_rescan_remove()` / `pci_unlock_rescan_remove()`. Affects
  driver remove / module-unbind path only.

### Step 2.3: Identify Bug Mechanism
**Record:** **Category:** Synchronization / race condition.
**Mechanism:** Concurrent sysfs PCI rescan (`/sys/bus/pci/rescan`, per-
device `rescan`, `remove`) or hotplug can walk/modify the bus device
list while `rockchip_pcie_remove()` is tearing it down without the
global mutex that sysfs paths already hold.

### Step 2.4: Assess Fix Quality
**Record:** Obviously correct — matches the established pattern in
`pci_host_common_remove()`, `mtk_pcie_remove()`, `mvebu` and `aardvark`
remove paths. Minimal, no API changes. **Regression risk:** Very low;
mutex is the same one used everywhere else for this purpose.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame Changed Lines
**Record:** `pci_stop_root_bus()` / `pci_remove_root_bus()` in
`rockchip_pcie_remove()` introduced by Rob Herring (2020-05-22, commit
`f473182c7524dd`). Remove function itself dates to Shawn Lin
(2018-05-09). Driver added 2016 (`e77f847df54c6`). Bug has been present
since the stop/remove calls were added without locking.

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

### Step 3.3: File History for Related Changes
**Record:** Part of a 9-patch series "[PATCH 0/9] PCI: controller: Add
missing rescan lock around root bus removal" (local mbox). Each patch is
independent per cover letter. `pci_lock_rescan_remove()` infrastructure
added in 2014 (`9d16947b75831`). `pci_host_common_remove()` has used the
lock since 2018 (`01fcb7f777a9f`). Fix is **not** yet merged in this
tree (grep shows no lock in rockchip remove; `git log --grep` for
subject returned empty).

### Step 3.4: Author's Other Commits
**Record:** Hans Zhang is an active PCI contributor (cadence, dwc
capability-search series, etc.). Not the Rockchip driver author; fixing
a cross-driver synchronization gap.

### Step 3.5: Prerequisites
**Record:** No dependencies. `pci_lock_rescan_remove()` /
`pci_unlock_rescan_remove()` exist in this tree (since 2014). Driver
includes `../pci.h` → `<linux/pci.h>`, so no new includes needed.
Standalone, applies cleanly.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Patch Discussion
**Record:** `b4 dig` could not be run on an unmerged commit hash. Used
local mbox `20260522_18255117159_pci_controller_add_missing_rescan_lock_
around_root_bus_removal.mbx`. Cover letter explains race with sysfs
rescan/hotplug → UAF/crash. References sashiko-bot review flagging the
same pattern in cadence code. **No review replies** in the mbox (patches
only). WebFetch of lore URL blocked by bot protection.

### Step 4.2: Reviewers
**Record:** Cover letter only; no Reviewed-by/Acked-by in thread. Commit
has SOB from Manivannan Sadhasivam and Bjorn Helgaas (PCI maintainer).

### Step 4.3: Bug Report
**Record:** No external bug report, syzbot, or KASAN trace. Issue
identified by code review / bot review of the pattern.

### Step 4.4: Related Patches
**Record:** 9-patch series for cadence, dwc, altera, brcmstb, iproc,
mediatek, rockchip, vmd, plda. Each independent. Rockchip is patch 7/9.

### Step 4.5: Stable Mailing List
**Record:** No stable-list discussion found in available sources.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `rockchip_pcie_remove()` — only function modified.

### Step 5.2: Trace Callers
**Record:** Called via `.remove = rockchip_pcie_remove` in
`rockchip_pcie_driver`, registered with `module_platform_driver()`.
Triggers on platform device removal: module unload (`rmmod` if built as
module), driver unbind, or platform teardown.

### Step 5.3: Trace Callees
**Record:** `pci_lock_rescan_remove()`, `pci_stop_root_bus()`,
`pci_remove_root_bus()`, `pci_unlock_rescan_remove()`, then
`irq_domain_remove()`, clock/regulator cleanup.

### Step 5.4: Call Chain / Reachability
**Record:** Race is between `rockchip_pcie_remove()` and sysfs paths in
`pci-sysfs.c` (`rescan_store`, `dev_rescan_store`, `remove_store`,
`bus_rescan_store`) — all hold `pci_lock_rescan_remove()`. An admin
writing to `/sys/bus/pci/rescan` (or per-bus/device rescan/remove) while
the driver is being removed can hit the race. Reachable on any Rockchip
system with `CONFIG_PCIE_ROCKCHIP_HOST`.

### Step 5.5: Similar Patterns
**Record:** Controllers **with** lock: `pci-host-common.c`, `pcie-
mediatek-gen3.c`, `pci-mvebu.c`, `pci-aardvark.c`, `pci-hyperv.c`.
Controllers **without** lock (same bug class): rockchip, cadence, dwc,
altera, brcmstb, iproc, mediatek (non-gen3), vmd, plda, tegra, etc.
Rockchip is a clear oversight relative to the common pattern.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Does Buggy Code Exist?
**Record:** **Yes.** Tree is **v6.18.44** (`git describe HEAD` →
`v6.18.44-1-g2736c32da98b9`, `make kernelversion` → `6.18.44`).
`rockchip_pcie_remove()` at lines 1015–1016 calls `pci_stop_root_bus()`
/ `pci_remove_root_bus()` **without** the lock. Driver present since
v4.8 era; bug since ~2020.

### Step 6.2: Backport Complications
**Record:** **Clean apply** — 2-line addition, no structural changes, no
conflicts expected.

### Step 6.3: Related Fixes Already Present?
**Record:** No equivalent fix in this tree. `git log --grep="Protect
root bus removal"` returned empty. Mediatek-gen3, mvebu, aardvark, pci-
host-common already have the lock; rockchip does not.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem Criticality
**Record:** **drivers/pci/controller** — IMPORTANT. PCI core affects
device enumeration and all downstream PCI devices on Rockchip SoCs
(RK3399, RK3568, etc.).

### Step 7.2: Subsystem Activity
**Record:** Actively maintained; recent rockchip commits in this tree
(link speed, error logging, reset timing).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users of Rockchip SoCs with `CONFIG_PCIE_ROCKCHIP_HOST`
(depends on `ARCH_ROCKCHIP`). Embedded/ARM boards using the legacy
Rockchip AXI PCIe host controller.

### Step 8.2: Trigger Conditions
**Record:** Driver remove/unbind concurrent with PCI sysfs rescan or
remove (typically root). Uncommon in steady state but realistic during
module reload, driver unbind testing, or admin sysfs operations.
Requires privileges for sysfs writes; remove path can be triggered by
module unload or device unbind.

### Step 8.3: Failure Mode Severity
**Record:** UAF / kernel crash — **HIGH** (potential **CRITICAL**
depending on exploitability of the freed PCI structures).

### Step 8.4: Risk-Benefit
**Record:** **Benefit:** HIGH — prevents real crashes on a long-standing
code path. **Risk:** VERY LOW — 2-line addition using existing, well-
tested API, matching multiple peer drivers. **Ratio:** Strongly favors
backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real synchronization bug with documented crash/UAF consequence
- Matches PCI core documentation: rescan/remove must run under
  `pci_rescan_remove_lock` (comment in `probe.c` lines 3536–3538)
- Peer drivers already use this pattern; rockchip is an outlier
- 2-line, obviously correct fix
- Buggy code confirmed present in v6.18.44 tree
- Driver has been in production kernels for years
- Signed-off-by PCI maintainer

**AGAINST backport:**
- No user-reported crash or syzbot reproducer (theoretical/code-review
  finding)
- Part of a 9-patch series (but each patch is independent)

**Unresolved:** No runtime crash report; lore thread review discussion
unavailable (bot-blocked).

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — pattern proven across
   multiple drivers; no Tested-by but change is trivial.
2. Fixes a real bug? **PASS** — missing mutex on a documented-required
   code path.
3. Important issue? **PASS** — UAF/system crash.
4. Small and contained? **PASS** — 2 lines, one function.
5. No new features/APIs? **PASS** — synchronization only.
6. Can apply to local tree? **PASS** — APIs and buggy code both present.

### Step 9.3: Exception Categories
**Record:** None (not device ID, quirk, DT, build, or docs). Standard
bug fix.

### Step 9.4: Decision Rationale

This commit closes a long-standing race in `rockchip_pcie_remove()`
where root bus teardown was not serialized against sysfs PCI
rescan/remove operations. The PCI core has required this mutex since
2014, and the generic host-common remove path has used it since 2018.
Rockchip was simply missed. The fix is minimal, follows established
convention, and prevents crashes on Rockchip platforms during driver
removal concurrent with PCI sysfs operations. For the v6.18.44 tree
being evaluated, the buggy code is present and the fix applies cleanly.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body; no Fixes:/Reported-by/syzbot
- **[Phase 2]** Diff: +2 lines in `rockchip_pcie_remove()` wrapping
  stop/remove with lock
- **[Phase 3]** `git describe HEAD` → v6.18.44; `git blame` →
  stop/remove added f473182c (2020); driver since e77f847df54c6 (2016)
- **[Phase 3]** `git show 9d16947b75831` → lock API added 2014; `git
  blame pci-host-common.c` → lock in remove since 01fcb7f777a9f (2018)
- **[Phase 3]** `git log --grep="Protect root bus removal"` → empty (not
  merged)
- **[Phase 4]** Read local mbox cover + patch 7/9; 9 independent
  patches; sashiko-bot race comment verified
- **[Phase 4]** WebFetch lore URL → blocked; b4 dig on unmerged commit →
  N/A
- **[Phase 5]** `grep pci_lock_rescan_remove drivers/pci/controller/` →
  rockchip absent; mvebu/aardvark/mediatek-gen3/hyperv present
- **[Phase 5]** Read `pci-sysfs.c` rescan/remove stores — all hold lock
- **[Phase 5]** Read `probe.c:3536-3552` — documents lock requirement
- **[Phase 6]** Read `rockchip_pcie_remove()` lines 1009–1016 — buggy
  code confirmed in tree
- **[Phase 6]** `Kconfig` → `PCIE_ROCKCHIP_HOST` tristate,
  `ARCH_ROCKCHIP`
- **[Phase 8]** Race between remove and sysfs rescan → UAF/crash per
  commit message and PCI core design

**YES**The background checks finished and match what the analysis
already assumed:

- **Search for commit in tree:** `b4 dig` on HEAD didn’t match this
  patch; `git log --grep` for the subject also found nothing. The fix
  isn’t in this tree yet.
- **Check if fix already merged:** `git log --grep="rockchip.*rescan
  lock"` and `--grep="Protect root bus removal"` both returned empty —
  confirmed not merged in v6.18.44.

**Verdict stands: YES** — backport the 2-line `pci_lock_rescan_remove()`
wrap in `rockchip_pcie_remove()` to close the sysfs rescan/remove race.

 drivers/pci/controller/pcie-rockchip-host.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db3..d203c4876d304 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -1012,8 +1012,10 @@ static void rockchip_pcie_remove(struct platform_device *pdev)
 	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(rockchip);
 
+	pci_lock_rescan_remove();
 	pci_stop_root_bus(bridge->bus);
 	pci_remove_root_bus(bridge->bus);
+	pci_unlock_rescan_remove();
 	irq_domain_remove(rockchip->irq_domain);
 
 	rockchip_pcie_deinit_phys(rockchip);
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH AUTOSEL 6.18-5.10] iommu/rockchip: disable fetch dte time limit

[Sasha Levin]

From: Simon Xue <xxm@rock-chips.com>

[ Upstream commit 8d4346ecd4950ae08cc76a6de327c264e846758c ]

Disable the Bit 31 of the AUTO_GATING iommu register, as it causes
hangups with the RGA3 (Raster Graphics Acceleration 3) peripheral.
The RGA3 register description of the TRM already states that the bit
must be set to 1. The vendor kernel sets the bit unconditionally to
1 to fix VOP (Video Output Processor) screen black issues. This patch
squashes the 2 vendor kernel commits with the following commit messages:

Master fetch data and cpu update page table may work in parallel, may
have the following procedure:

	master                  cpu
	fetch dte               update page tabl
	        |                       |
	(make dte invalid)  <-  zap iotlb entry
	        |                       |
	fetch dte again
	(make dte invalid)  <-  zap iotlb entry
	        |                       |
	fetch dte again
	(make dte invalid)  <-  zap iotlb entry
	        |                       |
	fetch dte again
	(make iommu block)  <-  zap iotlb entry

New iommu version has the above bug, if fetch dte consecutively four
times, then it will be blocked. Fortunately, we can set bit 31 of
register MMU_AUTO_GATING to 1 to make it work as old version which does
not have this issue.

This issue only appears on RV1126 so far, so make a workaround dedicated
to "rockchip,rv1126" machine type.

iommu/rockchip: fix vop blocked and screen black on RK356X and RK3588

RK3568 and RK3588 has the same issue as RV1126/RV1109 that caused by
dte fetch time limit, So we can set BIT(31) of register 0x24 default
to 1 as a workaround.

Signed-off-by: Simon Xue <xxm@rock-chips.com>
Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: PARSE THE SUBJECT LINE
**Record:** `[iommu/rockchip]` `[disable]` — Disable the fetch-DTE time-
limit hardware behavior in the Rockchip IOMMU AUTO_GATING register
(BIT(31)).

### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
**Record:**
- **Fixes:** — not present (expected for manual review)
- **Reported-by:** — none
- **Tested-by:** — none
- **Reviewed-by:** — none
- **Acked-by:** Heiko Stuebner `<heiko@sntech.de>` (Rockchip maintainer)
- **Link:** — none in commit (submission thread references vendor
  commits and RGA3 driver series)
- **Cc: stable:** — not present (expected)
- **Signed-off-by:** Simon Xue, Sven Püschel, Joerg Roedel (ignore
  pipeline-added SOBs)
- **Notable:** Ack from subsystem maintainer; no syzbot/fuzzer
  involvement

### Step 1.3: ANALYZE THE COMMIT BODY TEXT
**Record:**
- **Bug description:** Newer Rockchip IOMMU hardware has a DTE-fetch
  time limit. When a master re-fetches DTE four times while the CPU
  concurrently zaps IOTLB entries (during page-table updates), the IOMMU
  enters a blocked state.
- **Symptom/failure mode:** IOMMU hang/block → RGA3 peripheral hangups,
  VOP (display) blocked with black screen.
- **Affected hardware:** RV1126/RV1109, RK3568, RK3588 (commit message
  also mentions RK356X broadly).
- **Root cause:** BIT(31) of `RK_MMU_AUTO_GATING` (offset 0x24) defaults
  to 0 on affected silicon; TRM says it must be 1. Vendor kernel sets it
  unconditionally.
- **Version info:** Not tied to a specific kernel version; this is a
  silicon/hardware behavior issue.

### Step 1.4: DETECT HIDDEN BUG FIXES
**Record:** Not disguised — this is an explicit hardware workaround.
Despite "disable" wording in the subject, the fix **sets** BIT(31) to
disable the faulty time-limit feature. This is a classic hardware
quirk/workaround, not a cosmetic cleanup.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: INVENTORY THE CHANGES
**Record:**
- **Files:** `drivers/iommu/rockchip-iommu.c` (+8 lines, 0 removed)
- **Functions modified:** `rk_iommu_enable()` only
- **Scope:** Single-file, surgical fix

### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
**Record:**
- **Hunk 1 (define):** Adds `#define DISABLE_FETCH_DTE_TIME_LIMIT
  BIT(31)`.
- **Hunk 2 (`rk_iommu_enable`):**
  - **Before:** After writing DTE address, ZAP cache, and IRQ mask,
    proceeds directly to enable paging.
  - **After:** Reads `RK_MMU_AUTO_GATING`, ORs in BIT(31), writes it
    back — for each MMU instance.
  - **Affected path:** IOMMU enable during device attach and
    system/runtime resume.

### Step 2.3: IDENTIFY THE BUG MECHANISM
**Record:**
- **Bug category:** Hardware workaround / logic correctness fix
- **Mechanism:** Without BIT(31)=1, concurrent DTE fetch + IOTLB zap can
  trigger a silicon bug after four consecutive DTE fetches, permanently
  blocking the IOMMU. Setting BIT(31) restores legacy (non-buggy)
  behavior.

### Step 2.4: ASSESS THE FIX QUALITY
**Record:**
- **Quality:** Obviously correct — read-modify-write preserves other
  AUTO_GATING bits; matches vendor kernel and TRM guidance.
- **Regression risk:** Very low. Vendor sets unconditionally on all
  affected platforms; bit is documented as should-be-1.
- **Red flags:** Commit message still mentions RV1126-only workaround,
  but code applies unconditionally (intentional per vendor practice and
  RK3568/RK3588 need).

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: BLAME THE CHANGED LINES
**Record:** `rk_iommu_enable()` core logic dates to 2014 (Daniel Kurtz).
`RK_MMU_AUTO_GATING` defined since original driver (2014,
`c68a292152d32`). The **missing workaround** has been present since the
driver's introduction — not a recent regression.

### Step 3.2: FOLLOW THE FIXES: TAG
**Record:** No `Fixes:` tag. Not applicable — this is a hardware silicon
bug, not a commit-introduced regression.

### Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
**Record:**
- Recent related fix already in tree: `62e062a29ad51` — "prevent iommus
  dead loop when two masters share one IOMMU" (different bug, has `Cc:
  stable`).
- `rk3568-iommu` v2 support added in `c55356c534aa6` (2021), present in
  this tree.
- This fix is **standalone** — not part of a multi-patch series
  requiring prerequisites.
- On `master`, this commit (`8d4346ecd4950`) is ahead of
  `stable/linux-6.18.y`.

### Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
**Record:** Simon Xue is an active Rockchip IOMMU contributor (multi-irq
support, dead-loop fix, ISP reset handling). Sven Püschel (Pengutronix)
submitted and tested on RK3588 RGA3.

### Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
**Record:** No dependencies. Patch applies cleanly (`git apply --check`
succeeded). No new structures, APIs, or helper functions required.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
**Record:**
- **Lore URL:** https://patch.msgid.link/20251126-spu-
  iommudtefix-v1-1-f90003dbfcc4@pengutronix.de
- **Series revisions:** v1 submitted 2025-11-26; author pinged
  2026-04-28; Heiko Stuebner suggested resend/v2 due to age; committed
  as-is on mainline 2026-06-02.
- **Reviewer feedback:** Shawn Lin (Rockchip) noted TRM offset
  clarification (RGA3-specific offset vs general IOMMU 0x24) — comment-
  only, no code objection.
- **Stable nominations:** None found in thread.
- **NAKs:** None.

### Step 4.2: CHECK WHO REVIEWED THE PATCH
**Record:** CC'd: Joerg Roedel, Will Deacon, Robin Murphy, Heiko
Stuebner, iommu@, linux-arm-kernel@, linux-rockchip@. Heiko Stuebner
Acked-by in final commit.

### Step 4.3: SEARCH FOR THE BUG REPORT
**Record:** Real-world trigger documented by Pengutronix — sporadic RGA3
hangs on RK3588 during driver development. Vendor kernel commits [2][3]
document VOP black-screen issues. No syzbot/bugzilla report.

### Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
**Record:** Related but independent: RGA3 upstream driver series (v5,
2026-04-28) depends on this IOMMU fix. The IOMMU fix stands alone and is
not a "preparation" commit.

### Step 4.5: CHECK STABLE MAILING LIST HISTORY
**Record:** No stable-list discussion found for this specific fix. (Lore
direct fetch blocked by bot protection; analysis via `b4 dig -m` mbox
download.)

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
**Record:** `rk_iommu_enable()` — only function modified.

### Step 5.2: TRACE CALLERS
**Record:**
- `rk_iommu_attach_device()` → `rk_iommu_enable()` (line 1043) — called
  when a device attaches to an IOMMU domain (e.g., VOP, RGA, NPU).
- `rk_iommu_resume()` → `rk_iommu_enable()` (line 1330) — called on PM
  resume.
- Both are common, user-visible paths on Rockchip boards.

### Step 5.3: TRACE CALLEES
**Record:** Uses existing `rk_iommu_read()` / `rk_iommu_write()`
register accessors, plus existing stall/reset/paging enable sequence. No
new dependencies.

### Step 5.4: FOLLOW THE CALL CHAIN
**Record:** Device probe → `iommu_attach_device` →
`rk_iommu_attach_device` → `rk_iommu_enable`. Triggered during normal
graphics/media driver initialization and suspend/resume. **Reachable
from userspace** indirectly via device usage (display, GPU, RGA
workloads causing IOTLB zaps).

### Step 5.5: SEARCH FOR SIMILAR PATTERNS
**Record:** No similar workaround elsewhere in `rockchip-iommu.c`.
Vendor kernel sets this bit unconditionally — external confirmation of
the pattern.

---

## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE

### Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
**Record:** **YES.** Local tree is **Linux 6.18.44** (`git describe
HEAD` → `v6.18.44-1-gef4bf62bccf3c`). `rk_iommu_enable()` at lines
928–960 lacks the BIT(31) workaround. `RK_MMU_AUTO_GATING` is defined at
line 42. `DISABLE_FETCH_DTE_TIME_LIMIT` is **not** present. Affected DT
platforms exist: `rv1126.dtsi` (v1 `rockchip,iommu`), `rk356x-base.dtsi`
and `rk3588-base.dtsi` (v2 `rockchip,rk3568-iommu`).

### Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
**Record:** **Clean apply** — `git format-patch` + `git apply --check`
succeeded with no conflicts. No refactoring churn in `rk_iommu_enable()`
since 6.18 branch.

### Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
**Record:** The dead-loop fix (`62e062a29ad51`) is present. This DTE
time-limit workaround (`8d4346ecd4950`) is **not** present on
`stable/linux-6.18.y`.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
**Record:** `drivers/iommu/rockchip-iommu.c` — IOMMU driver for Rockchip
SoCs. **IMPORTANT** for ARM/ARM64 embedded (display, media, NPU, ISP).
`CONFIG_ROCKCHIP_IOMMU=y` in `arch/arm64/configs/defconfig`.

### Step 7.2: ASSESS SUBSYSTEM ACTIVITY
**Record:** Actively maintained — recent fixes in 6.17/6.18 merge window
(dead-loop fix, iommu-pages migration). Rockchip platforms (RK3568,
RK3588) are widely deployed.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: DETERMINE WHO IS AFFECTED
**Record:** Users of Rockchip SoCs with IOMMU-enabled peripherals —
**platform-specific** but covering popular boards (RK3568, RK3588,
RV1126). Display (VOP), graphics acceleration (RGA3), and other IOMMU-
backed masters.

### Step 8.2: DETERMINE THE TRIGGER CONDITIONS
**Record:** Concurrent IOMMU master DTE fetch + CPU IOTLB zap during
page-table updates. Realistic during graphics/media workloads and driver
activity. Not every boot, but reproducible under load (Pengutronix
observed sporadic RGA3 hangs).

### Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
**Record:** IOMMU permanent block → **CRITICAL** system hang for
affected peripherals; VOP black screen (display unusable); potential
soft lockup of dependent subsystems.

### Step 8.4: CALCULATE RISK-BENEFIT RATIO
**Record:**
- **Benefit:** HIGH — prevents hardware IOMMU lockup and
  display/peripheral hangs on widely used SoCs.
- **Risk:** VERY LOW — 8-line register write matching vendor kernel and
  TRM; read-modify-write preserves other bits.
- **Ratio:** Strongly favors backport.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: COMPILE THE EVIDENCE

**FOR backporting:**
- Real hardware bug with documented failure mode (IOMMU block, display
  black, RGA3 hang)
- Hardware quirk/workaround — explicitly allowed stable category
- Small (8 lines), surgical, applies cleanly to 6.18.44
- Vendor kernel already does this unconditionally
- TRM documents BIT(31) should be 1
- Acked-by Rockchip maintainer (Heiko Stuebner)
- Affected SoC DT support present in this tree
- Buggy code path exists and has existed since driver introduction
- No prerequisite commits

**AGAINST backporting:**
- No `Cc: stable` or `Fixes:` tag (expected, not disqualifying)
- Applies to all Rockchip IOMMU instances, not SoC-gated (but vendor
  does same; low risk)
- Stale RV1126-only wording in commit message (code is correctly
  unconditional)

**Unresolved:** None that affect the decision.

### Step 9.2: STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — TRM + vendor kernel +
   Pengutronix RK3588 testing
2. Fixes a real bug affecting users? **PASS** — IOMMU hang, black screen
   on production hardware
3. Important issue? **PASS** — CRITICAL (hang, display failure)
4. Small and contained? **PASS** — 8 lines, one function
5. No new features or APIs? **PASS** — register workaround only
6. Can apply to local tree? **PASS** — clean apply verified

### Step 9.3: EXCEPTION CATEGORIES
**Record:** **Hardware quirk/workaround** — qualifies under stable
exception rules.

### Step 9.4: DECISION RATIONALE

For **Linux 6.18.44**, Rockchip IOMMU users on RK3568, RK3588, and
RV1126 platforms are exposed to a silicon bug where the IOMMU can enter
a blocked state during normal page-table maintenance, causing peripheral
hangs and display black screens. The fix is a minimal, vendor-validated
register write that matches TRM documentation. It applies cleanly, has
maintainer ack, and fixes a serious user-visible failure on hardware
well-supported in this tree.

---

## Verification

- [Phase 1] Parsed commit `8d4346ecd4950`: subject, body, tags (Acked-by
  Heiko Stuebner; no Fixes/stable/syzbot)
- [Phase 2] Diff analysis: +8 lines in `rk_iommu_enable()`, adds BIT(31)
  write to `RK_MMU_AUTO_GATING`
- [Phase 3] `git blame`: `rk_iommu_enable()` from 2014;
  `RK_MMU_AUTO_GATING` from original 2014 driver
- [Phase 3] No Fixes: tag to follow
- [Phase 3] Related commit `62e062a29ad51` (different bug) already in
  tree; this fix is not
- [Phase 3] `git format-patch` + `git apply --check`: applies cleanly
- [Phase 3] Commit on `master` but not on current `stable/linux-6.18.y`
  HEAD
- [Phase 4] `b4 dig -c 8d4346ecd4950`: lore thread found
- [Phase 4] `b4 dig -a`: v1 submission 2025-11-26; committed 2026-06-02
- [Phase 4] `b4 dig -w`: Joerg Roedel, Heiko Stuebner, Robin Murphy CC'd
- [Phase 4] Mbox: Pengutronix confirmed RK3588 RGA3 hangs; Shawn Lin
  comment-only TRM note; Heiko suggested resend; no NAKs, no stable
  nomination
- [Phase 5] Callers: `rk_iommu_attach_device()` (line 1043),
  `rk_iommu_resume()` (line 1330)
- [Phase 5] IOTLB zap path: `rk_iommu_zap_iova()` used from map/unmap —
  confirms concurrent zap mechanism
- [Phase 6] `git describe HEAD`: v6.18.44; `make kernelversion`: 6.18.44
- [Phase 6] Buggy code confirmed present: no
  `DISABLE_FETCH_DTE_TIME_LIMIT` in tree
- [Phase 6] DT: `rv1126.dtsi`, `rk356x-base.dtsi`, `rk3588-base.dtsi`
  have IOMMU nodes
- [Phase 6] `rk3568-iommu` support since 2021 (`c55356c534aa6`), in tree
- [Phase 7] `CONFIG_ROCKCHIP_IOMMU=y` in arm64 defconfig
- [Phase 8] Failure mode: IOMMU block → hang/black screen, severity
  CRITICAL

**YES**The background search on `master` found the commit:
**`8d4346ecd4950`** — `iommu/rockchip: disable fetch dte time limit`.

**Summary:** This is an 8-line hardware workaround for Rockchip IOMMU
silicon (RK3568, RK3588, RV1126). Without setting BIT(31) of
`RK_MMU_AUTO_GATING`, the IOMMU can block during concurrent DTE fetch
and IOTLB zaps, causing peripheral hangs and VOP black screens. The
patch applies cleanly to the local **6.18.44** tree, where the buggy
code is still present.

**YES**

 drivers/iommu/rockchip-iommu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0861dd469bd86..2d0dabb0d101a 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -76,6 +76,8 @@
 #define SPAGE_ORDER 12
 #define SPAGE_SIZE (1 << SPAGE_ORDER)
 
+#define DISABLE_FETCH_DTE_TIME_LIMIT BIT(31)
+
  /*
   * Support mapping any size that fits in one page table:
   *   4 KiB to 4 MiB
@@ -930,6 +932,7 @@ static int rk_iommu_enable(struct rk_iommu *iommu)
 	struct iommu_domain *domain = iommu->domain;
 	struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
 	int ret, i;
+	u32 auto_gate;
 
 	ret = clk_bulk_enable(iommu->num_clocks, iommu->clocks);
 	if (ret)
@@ -948,6 +951,11 @@ static int rk_iommu_enable(struct rk_iommu *iommu)
 			       rk_ops->mk_dtentries(rk_domain->dt_dma));
 		rk_iommu_base_command(iommu->bases[i], RK_MMU_CMD_ZAP_CACHE);
 		rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, RK_MMU_IRQ_MASK);
+
+		/* Workaround for iommu blocked, BIT(31) default to 1 */
+		auto_gate = rk_iommu_read(iommu->bases[i], RK_MMU_AUTO_GATING);
+		auto_gate |= DISABLE_FETCH_DTE_TIME_LIMIT;
+		rk_iommu_write(iommu->bases[i], RK_MMU_AUTO_GATING, auto_gate);
 	}
 
 	ret = rk_iommu_enable_paging(iommu);
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH AUTOSEL 6.18-5.10] ASoC: rockchip: rockchip_pdm: Handle runtime PM resume failures in set_fmt

[Sasha Levin]

From: bui duc phuc <phucduc.bui@gmail.com>

[ Upstream commit ee7b5f7b39332febf917f9ebf212842cc9379815 ]

rockchip_pdm_set_fmt() calls pm_runtime_get_sync() before accessing
hardware registers, but ignores its return value.
If the runtime resume fails, the function continues to perform register
accesses while the device state is undefined.
Replace pm_runtime_get_sync() with pm_runtime_resume_and_get() and
return early on failure to avoid unpowered register accesses.

Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260522110302.349421F000E9@smtp.kernel.org/
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
Link: https://patch.msgid.link/20260602101608.45137-6-phucduc.bui@gmail.com
Signed-off-by: Mark Brown <broonie@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: Parse Subject Line
**Record:** `[ASoC: rockchip: rockchip_pdm]` `[Handle]` — Handle runtime
PM resume failures in the `set_fmt` DAI callback.

### Step 1.2: Parse All Commit Message Tags
**Record:**
| Tag | Value |
|-----|-------|
| Reported-by | Sashiko AI Review \<sashiko-bot@kernel.org\> |
| Closes |
https://lore.kernel.org/all/20260522110302.349421F000E9@smtp.kernel.org/
|
| Link |
https://patch.msgid.link/20260602101608.45137-6-phucduc.bui@gmail.com |
| Signed-off-by | bui duc phuc \<phucduc.bui@gmail.com\> |
| Signed-off-by | Mark Brown \<broonie@kernel.org\> (committer/ASoC
maintainer) |

Notable patterns: Static-analysis report (Sashiko AI), not syzbot or a
user crash report. No `Fixes:` tag (expected). No `Cc:
stable@vger.kernel.org`. Mark Brown merged it.

### Step 1.3: Analyze Commit Body
**Record:**
- **Bug:** `rockchip_pdm_set_fmt()` calls `pm_runtime_get_sync()` but
  ignores its return value. If runtime resume fails, register writes
  proceed while the device is not powered/resumed.
- **Symptom:** Undefined device state; unpowered register accesses
  (historically documented as system hang in this driver).
- **Root cause:** Incomplete error handling when runtime PM resume fails
  (clock enable failure in `rockchip_pdm_runtime_resume()`).
- **Fix:** Replace `pm_runtime_get_sync()` with
  `pm_runtime_resume_and_get()` and return the error early.

### Step 1.4: Detect Hidden Bug Fixes
**Record:** Not disguised as cleanup — explicitly a bug fix. It
completes error handling that was left incomplete when runtime PM was
added to `set_fmt` in 2019 (commit `c85064435fe7a2`).

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory Changes
**Record:**
- **File:** `sound/soc/rockchip/rockchip_pdm.c` (+5 / −1)
- **Function:** `rockchip_pdm_set_fmt()`
- **Scope:** Single-file, surgical fix (5 lines)

### Step 2.2: Code Flow Change
**Record:**
- **Before:** `pm_runtime_get_sync()` → always `regmap_update_bits()` →
  `pm_runtime_put()` → return 0, regardless of resume outcome.
- **After:** `pm_runtime_resume_and_get()` → on failure, return error
  immediately (no register access, no `pm_runtime_put()`) → on success,
  same register access path as before.
- **Path affected:** DAI format configuration during ASoC card setup
  (`set_fmt` callback).

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Error-path / logic correctness fix (ignored return value
  → unsafe hardware access).
- **Mechanism:** `rockchip_pdm_runtime_resume()` can fail on
  `clk_prepare_enable()` for `pdm->clk` or `pdm->hclk`. With the old
  code, `pm_runtime_get_sync()` returns negative but execution continues
  to `regmap_update_bits()` on an unpowered controller. The 2019 commit
  that introduced `pm_runtime_get_sync()` here explicitly stated that
  regmap ops with power domain off "will lead system hang."

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct. Matches the pattern already used in
  `rockchip_pdm_resume()` in the same file (since commit
  `76a6f4537650e`, 2022).
- **Regression risk:** Very low. On failure, propagates error to caller
  instead of proceeding unsafely.
- **Red flags:** None. No API changes, no refactoring.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame Changed Lines
**Record:**
- `rockchip_pdm_set_fmt()` body: original commit `fc05a5b2225306`
  (2017).
- `pm_runtime_get_sync()`/`pm_runtime_put()`: commit `c85064435fe7a2`
  (2019-04-03) — "fix regmap_ops hang issue."
- Buggy ignored-return-value pattern present since 2019.

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

### Step 3.3: File History for Related Changes
**Record:**
- `76a6f4537650e` (2022): Same `pm_runtime_resume_and_get()` + error
  check applied to `rockchip_pdm_resume()`.
- `ef0a098efb366`: Missing `clk_disable_unprepare()` fix in runtime
  resume.
- Part of series "[PATCH v2 0/5] ASoC: rockchip: Reorder clock enable
  sequence" (patch 5/5), but this hunk is **standalone** — it does not
  depend on the clock-reorder patches (patches 3–4).

### Step 3.4: Author's Other Commits
**Record:** Author phucduc.bui@gmail.com; no prior rockchip ASoC commits
in this tree. Mark Brown (committer) is ASoC maintainer.

### Step 3.5: Prerequisites
**Record:** No prerequisites. `pm_runtime_resume_and_get()` already
exists and is used in this file at line 685. Patch applies cleanly (`git
apply --check` passed).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Patch Discussion
**Record:**
- **b4 dig URL:**
  https://patch.msgid.link/20260602101608.45137-6-phucduc.bui@gmail.com
- **Series:** v2, patch 5/5 of "ASoC: rockchip: Reorder clock enable
  sequence"
- **Sashiko review:** Flagged the ignored `pm_runtime_get_sync()` return
  value; also noted a separate pre-existing clock underflow issue in
  `rockchip_pdm_remove()` (unrelated to this patch).
- **Stable nominations:** None found in thread.
- **NAKs:** None found.

### Step 4.2: Reviewers
**Record:** CC'd Mark Brown, Heiko Stuebner, Liam Girdwood, Takashi
Iwai, linux-sound@, linux-rockchip@. Rob Herring Acked-by on an earlier
patch in the series (DT bindings), not specifically this one. Mark Brown
merged.

### Step 4.3: Bug Report
**Record:** Sashiko AI static analysis (not a runtime crash report).
Original Closes link points to the Sashiko review bot email. Patch
submission notes: **"compile-tested only."**

### Step 4.4: Related Patches / Series
**Record:** Patches 1–4 cover clock reorder and regcache sync in runtime
resume for PDM/SPDIF. This patch (5/5) is independent — only touches
`set_fmt` error handling.

### Step 4.5: Stable Mailing List
**Record:** Not searched separately; no stable nomination found in the
patch thread.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `rockchip_pdm_set_fmt()` (modified); callers via
`rockchip_pdm_dai_ops.set_fmt`.

### Step 5.2: Trace Callers
**Record:**
- `rockchip_pdm_dai_ops.set_fmt` → registered in `rockchip_pdm_dai`
- Called via `snd_soc_dai_set_fmt()` in `sound/soc/soc-dai.c`
- Invoked from `soc-core.c` during machine/DAI link format setup
- **Context:** Normal audio card initialization/configuration path on
  Rockchip boards using PDM microphones.

### Step 5.3: Trace Callees
**Record:** `pm_runtime_resume_and_get()` → may call
`rockchip_pdm_runtime_resume()` → `clk_prepare_enable()`. On success:
`regmap_update_bits()`, `pm_runtime_put()`.

### Step 5.4: Call Chain / Reachability
**Record:** Reachable during audio subsystem setup when a machine driver
configures the PDM DAI format. Requires `CONFIG_SND_SOC_ROCKCHIP_PDM`
(or built-in rockchip audio). Trigger requires runtime resume failure
(e.g., clock failure), which is an error path but realistic.

### Step 5.5: Similar Patterns
**Record:** Same file already uses `pm_runtime_resume_and_get()` with
error check in `rockchip_pdm_resume()` (lines 685–687). Kernel docs in
`include/linux/pm_runtime.h` explicitly recommend
`pm_runtime_resume_and_get()` over `pm_runtime_get_sync()` when the
return value is checked.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Does Buggy Code Exist?
**Record:** **Yes.** Local tree is **v6.18.44** (`git describe HEAD`:
`v6.18.44-1-g2736c32da98b9`). At lines 337–339, `rockchip_pdm_set_fmt()`
still has unchecked `pm_runtime_get_sync()`. Fix commit `ee7b5f7b39332`
is on master but **not** in this tree.

### Step 6.2: Backport Complications
**Record:** Clean apply confirmed. No conflicting changes in the hunk
area. Low difficulty.

### Step 6.3: Related Fixes Already Present?
**Record:** `76a6f4537650e` (pm_runtime_resume_and_get in
`rockchip_pdm_resume`) is present. The `set_fmt` path was missed and
remains unfixed.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem Criticality
**Record:** **ASoC / Rockchip PDM driver** — IMPORTANT for embedded
Rockchip platforms (rk3229, px30, rk3308, rk3568, rv1126), PERIPHERAL
globally.

### Step 7.2: Subsystem Activity
**Record:** Active — recent commits in `sound/soc/rockchip/` include
SAI, i2s-tdm, and runtime PM cleanups.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users of Rockchip SoCs with PDM (digital microphone
capture). Config/driver-specific, not universal.

### Step 8.2: Trigger Conditions
**Record:** `set_fmt` called while device is runtime-suspended AND
`rockchip_pdm_runtime_resume()` fails (clock enable failure).
Unprivileged users cannot directly trigger `set_fmt`, but audio
subsystem setup during boot or `modprobe`/card registration can. Failure
path is uncommon but valid.

### Step 8.3: Failure Mode Severity
**Record:** **System hang** — explicitly documented in the 2019 commit
that introduced runtime PM here: "regmap_ops will lead system hang" when
power domain is off. **Severity: CRITICAL** for affected hardware when
triggered; **LOW** probability.

### Step 8.4: Risk-Benefit Ratio
**Record:**
- **Benefit:** Prevents potential system hang on Rockchip PDM hardware
  during audio setup error paths; completes incomplete error handling
  from 2019.
- **Risk:** Very low — 5-line change, established API, same pattern
  already in the file.
- **Ratio:** Favorable for backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real bug: ignored runtime PM resume failure return value
- Documented hang risk from register access without power (2019 commit
  message)
- Small (5 lines), surgical, applies cleanly
- Follows pattern already in same file since 2022
- Merged by ASoC maintainer Mark Brown
- `pm_runtime_resume_and_get()` API present in this tree

**AGAINST backport:**
- Compile-tested only (no hardware Tested-by)
- Found by AI static analysis, not a user crash report
- Driver-specific (Rockchip PDM only)
- Trigger requires runtime resume failure (uncommon)
- Part of a 5-patch series (though this patch is standalone)

**Unresolved:** No hardware testing confirmation; no explicit stable
nomination from reviewers.

### Step 9.2: Stable Rules Checklist
| Criterion | Result |
|-----------|--------|
| 1. Obviously correct and tested? | **PASS** — logically correct;
compile-tested only |
| 2. Fixes a real bug? | **PASS** — ignored error return on runtime PM
resume |
| 3. Important issue? | **PASS** — potential system hang on affected
hardware |
| 4. Small and contained? | **PASS** — 5 lines, 1 file |
| 5. No new features/APIs? | **PASS** — error handling only |
| 6. Can apply to local tree? | **PASS** — clean apply, buggy code
present |

### Step 9.3: Exception Categories
**Record:** None (not device ID, quirk, DT, build, or docs). Standard
driver bug fix.

### Step 9.4: Problem and Decision Rationale

This commit closes a gap in runtime PM error handling in
`rockchip_pdm_set_fmt()`. When runtime resume fails (e.g., clock enable
error in `rockchip_pdm_runtime_resume()`), the driver previously
proceeded to write hardware registers anyway. The original 2019 fix that
added `pm_runtime_get_sync()` explicitly documented that regmap access
without power causes a **system hang** — this patch ensures that failure
path is handled correctly by returning early, matching the pattern
already applied to `rockchip_pdm_resume()` in the same file.

For the **v6.18.y** tree checked out here, the buggy code is present,
the fix applies cleanly, and the change is minimal with negligible
regression risk. While the trigger is an error path and the patch is
compile-tested only, preventing a documented system hang on real
Rockchip hardware meets stable kernel criteria.

---

## Verification

- **[Phase 1]** Parsed commit `ee7b5f7b39332`: tags, body, Sashiko
  Reported-by
- **[Phase 2]** Diff: +5/−1 in `rockchip_pdm_set_fmt()`; error-path fix
  confirmed
- **[Phase 3]** `git blame -L 319,342`: pm_runtime lines from
  `c85064435fe7a2` (2019)
- **[Phase 3]** `git show c85064435fe7a2`: original commit documents
  system hang without power
- **[Phase 3]** `git show 76a6f4537650e`: same pattern already in
  `rockchip_pdm_resume()`
- **[Phase 3]** `git apply --check`: patch applies cleanly to local tree
- **[Phase 4]** `b4 dig -c ee7b5f7b39332`: lore URL found; patch 5/5 of
  v2 series
- **[Phase 4]** `b4 dig -a`: series context; standalone nature of this
  hunk verified
- **[Phase 4]** `b4 dig -w`: maintainers CC'd; Mark Brown merged
- **[Phase 4]** Mbox: Sashiko review + "compile-tested only" note; no
  stable nomination
- **[Phase 5]** `grep set_fmt sound/soc/soc-dai.c soc-core.c`: call
  chain via ASoC core confirmed
- **[Phase 5]** Read `rockchip_pdm_runtime_resume()`: clock enable
  failure returns negative
- **[Phase 5]** Read `include/linux/pm_runtime.h`: kernel recommends
  `pm_runtime_resume_and_get()`
- **[Phase 6]** `git describe HEAD`: v6.18.44; buggy code at line 337
  confirmed present
- **[Phase 6]** Fix not in tree; `pm_runtime_resume_and_get` already
  used at line 685
- **[Phase 8]** Failure mode: system hang per 2019 commit message;
  severity CRITICAL when triggered

**YES****Verdict: YES** — backport to this tree (v6.18.44).

`rockchip_pdm_set_fmt()` still ignores a failed runtime PM resume and
can touch registers while the PDM block is off; the 2019 hang fix left
that gap. The patch is small (5 lines), applies cleanly, and matches the
pattern already used in `rockchip_pdm_resume()` in the same file. Main
caveat: compile-tested only, found by static analysis rather than a user
crash report.

 sound/soc/rockchip/rockchip_pdm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/rockchip/rockchip_pdm.c b/sound/soc/rockchip/rockchip_pdm.c
index c69cdd6f24994..01292b63bf9d6 100644
--- a/sound/soc/rockchip/rockchip_pdm.c
+++ b/sound/soc/rockchip/rockchip_pdm.c
@@ -321,6 +321,7 @@ static int rockchip_pdm_set_fmt(struct snd_soc_dai *cpu_dai,
 {
 	struct rk_pdm_dev *pdm = to_info(cpu_dai);
 	unsigned int mask = 0, val = 0;
+	int ret;
 
 	mask = PDM_CKP_MSK;
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
@@ -334,7 +335,10 @@ static int rockchip_pdm_set_fmt(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 
-	pm_runtime_get_sync(cpu_dai->dev);
+	ret = pm_runtime_resume_and_get(cpu_dai->dev);
+	if (ret)
+		return ret;
+
 	regmap_update_bits(pdm->regmap, PDM_CLK_CTRL, mask, val);
 	pm_runtime_put(cpu_dai->dev);
 
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH AUTOSEL 6.18-5.10] ASoC: rockchip: spdif: Restore regcache cache-only mode on sync failure

[Sasha Levin]

From: bui duc phuc <phucduc.bui@gmail.com>

[ Upstream commit 3546e9aa691ac981e4734fedd1646d0180784893 ]

If regcache_sync() fails during runtime resume, the driver disables the
clocks and returns an error. However, the regmap cache-only mode is left
disabled.
Restore cache-only mode in the error path so subsequent register accesses
continue to use the cache while the device is inactive.

Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260522103713.6C09D1F000E9@smtp.kernel.org/
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
Link: https://patch.msgid.link/20260602101608.45137-5-phucduc.bui@gmail.com
Signed-off-by: Mark Brown <broonie@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:** `[ASoC: rockchip: spdif] [Restore] Restore regcache cache-
only mode on sync failure during runtime resume error path`

### Step 1.2: Commit Tags
**Record:**
- **Reported-by:** Sashiko AI Review `<sashiko-bot@kernel.org>` —
  automated review bot, not a user crash report
- **Closes:** https://lore.kernel.org/all/20260522103713.6C09D1F000E9@sm
  tp.kernel.org/ — Sashiko review thread
- **Signed-off-by:** bui duc phuc `<phucduc.bui@gmail.com>` — author
- **Link:**
  https://patch.msgid.link/20260602101608.45137-5-phucduc.bui@gmail.com
  — patch 5 of a series (message-id suffix)
- **Signed-off-by:** Mark Brown `<broonie@kernel.org>` — ASoC subsystem
  maintainer
- **No Fixes:** tag (expected for manual review pipeline)
- **No Cc: stable@vger.kernel.org** (expected; not a negative signal)
- **No Tested-by / Reviewed-by / Acked-by**

Notable: maintainer sign-off from Mark Brown; same author
(`phucduc.bui`) recently landed an analogous `regcache_cache_only`
error-path fix for `gpio-pca953x` with `Cc: stable@vger.kernel.org`.

### Step 1.3: Commit Body Analysis
**Record:**
- **Bug:** On `regcache_sync()` failure in `rk_spdif_runtime_resume()`,
  clocks are disabled and an error is returned, but
  `regcache_cache_only(false)` is never reverted.
- **Symptom:** After a failed resume, regmap leaves cache-only mode
  while the device is inactive; subsequent register accesses attempt
  hardware I/O instead of using the cache.
- **Root cause:** Incomplete error-path state restoration — suspend sets
  `cache_only(true)`, resume sets `cache_only(false)` before sync, but
  the sync-failure path omits restoring `cache_only(true)`.
- **Version info:** None stated in the commit message.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Not disguised — this is an explicit error-path state-machine
bug fix, though the subject uses "Restore" rather than "fix".

---

## Phase 2: Diff Analysis

### Step 2.1: Change Inventory
**Record:**
- **Files:** `sound/soc/rockchip/rockchip_spdif.c` — 1 line added (+1
  net in the shown hunk)
- **Function modified:** `rk_spdif_runtime_resume()`
- **Scope:** Single-file, surgical fix

Note: upstream diff shows `hclk` enabled before `mclk`; this tree
enables `mclk` then `hclk`. The added line placement (inside the
`regcache_sync()` failure block, before clock disable) is identical in
intent.

### Step 2.2: Code Flow Change
**Record:**
- **Hunk (regcache_sync error path):**
  - **Before:** On sync failure → disable clocks → return error, leaving
    `cache_only == false`
  - **After:** On sync failure → `regcache_cache_only(map, true)` →
    disable clocks → return error
- **Affected path:** Runtime PM resume error path only (not the success
  path)

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Error-path / state consistency bug (regmap cache-mode
  invariant violation)
- **Mechanism:** `rk_spdif_runtime_suspend()` sets cache-only; resume
  clears it before sync; failed sync leaves the map in "live hardware"
  mode while clocks are off and the device is inactive. The fix restores
  the suspended-state invariant.

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct — mirrors the established pattern in
  `sgtl5000.c` and the recently backported `pca953x` fix by the same
  author.
- **Regression risk:** Very low — one line on an already-rare error
  path.
- **Red flags:** None.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:**
- Buggy `regcache_sync()` error path introduced by **3628c6987fb45**
  (2016-09-07): "ASoC: rockchip: spdif: restore register during
  runtime_suspend/resume cycle"
- Related prior fix: **6d94d0090527b** (2022-12-08) added missing
  `clk_disable_unprepare()` on hclk failure — same function, same class
  of incomplete error handling
- PM runtime integration: **f50d67f9eff62** (2020-07-13)

### Step 3.2: Fixes: Tag
**Record:** Not applicable — no `Fixes:` tag in the commit message.

### Step 3.3: Related File History
**Record:**
- Recent changes to this file are cleanups (`RUNTIME_PM_OPS`, remove
  callback, DAI merge) — no overlapping fix for this bug.
- Fix commit message not found in this tree — **fix is not yet applied
  locally**.
- Patch appears standalone (single line, one file); message-id `-5`
  suggests a series, but no series dependency is evident from the diff.

### Step 3.4: Author Context
**Record:**
- Author `phucduc.bui` has no other commits under `sound/soc/rockchip/`
  in this tree.
- Same author authored **2e4bc8422cdee** (`gpio: pca953x: fix cache_only
  ... on restore_context() failure`), which was backported to this
  stable tree with `Cc: stable@vger.kernel.org`.

### Step 3.5: Dependencies
**Record:** No prerequisites — self-contained one-line addition. Applies
cleanly to this tree (clock order differs cosmetically, hunk location
unchanged).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Patch Discussion
**Record:** `b4 dig -m "Restore regcache cache-only mode on sync
failure"` returned no match. `b4 dig -m
"20260602101608.45137-5-phucduc.bui@gmail.com"` returned no match.
Lore/patch.msgid.link URLs blocked by Anubis bot protection — **could
not read review thread content**.

### Step 4.2: Reviewers
**Record:** `b4 dig -w` not usable (no thread match). Mark Brown
(maintainer) Signed-off-by confirms maintainer acceptance.

### Step 4.3: Bug Report
**Record:** Reported by Sashiko AI Review (automated static analysis),
not syzbot or a user crash report. Underlying issue is code-review-
identified state inconsistency, not a filed oops trace.

### Step 4.4: Related Patches
**Record:** Same author/class of fix in `gpio-pca953x` (already in this
tree at `2e4bc8422cdee`). `sgtl5000.c` already implements the correct
pattern at lines 1135–1139.

### Step 4.5: Stable List History
**Record:** Could not search lore stable list (Anubis blocking). The
analogous pca953x fix from this author explicitly carried `Cc:
stable@vger.kernel.org` and was merged here by Greg K-H.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `rk_spdif_runtime_resume()` modified; related:
`rk_spdif_runtime_suspend()`, `rk_spdif_hw_params()`,
`rk_spdif_trigger()`

### Step 5.2: Callers
**Record:**
- `rk_spdif_runtime_resume()` registered via `RUNTIME_PM_OPS()` at line
  377 — invoked by PM core on runtime resume
- Direct call from `rk_spdif_probe()` when PM runtime is disabled (lines
  338–341)
- Regmap users: `rk_spdif_hw_params()`, `rk_spdif_trigger()` — ASoC
  PCM/DAI paths during active audio

### Step 5.3: Callees
**Record:** `clk_prepare_enable()`, `regcache_cache_only()`,
`regcache_mark_dirty()`, `regcache_sync()`, `clk_disable_unprepare()`

### Step 5.4: Reachability
**Record:**
- Resume path reachable on every runtime PM resume (suspend/resume
  cycles, audio start on Rockchip boards)
- Bug triggers only when `regcache_sync()` returns error (uncommon but
  real — bus/clock/hardware failure during sync)
- After bug triggers, any regmap access while device is inactive hits
  hardware path instead of cache — reachable from subsequent resume
  retries or regmap ops if PM state is inconsistent

### Step 5.5: Similar Patterns
**Record:**
- **Correct pattern:** `sound/soc/codecs/sgtl5000.c:1135-1139` restores
  `cache_only(true)` on sync failure
- **Same bug class, same author:** `drivers/gpio/gpio-pca953x.c`
  `pca953x_restore_context()` err path
- **Same bug present:** `sound/soc/rockchip/rockchip_sai.c:251-277` —
  also lacks cache-only restore on sync failure (out of scope for this
  commit)

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **YES.** Local tree is **v6.18.44** (`6.18.44`). Buggy code
at:

```98:102:sound/soc/rockchip/rockchip_spdif.c
        ret = regcache_sync(spdif->regmap);
        if (ret) {
                clk_disable_unprepare(spdif->mclk);
                clk_disable_unprepare(spdif->hclk);
        }
```

Missing `regcache_cache_only(spdif->regmap, true)`. Bug present since
3628c6987fb45 (2016).

### Step 6.2: Backport Complications
**Record:** Clean apply expected — add one line inside existing `if
(ret)` block. Clock enable order differs from upstream diff but hunk
location is unchanged.

### Step 6.3: Related Fixes Already Present?
**Record:** No equivalent fix in this tree. Prior related fix
6d94d0090527b (missing clk disable) is present. Fix commit not found via
grep or git log.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem and Criticality
**Record:** **ASoC / Rockchip SPDIF driver** — **PERIPHERAL** (Rockchip
embedded SoC audio output). Affects boards using the in-SoC SPDIF
controller (RK3288, RK3399, RK3568, etc.).

### Step 7.2: Subsystem Activity
**Record:** Moderate recent activity (SAI driver additions, cleanups);
SPDIF driver itself is mature with infrequent changes.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users of Rockchip platforms with
`CONFIG_SND_SOC_ROCKCHIP_SPDIF` and the built-in SPDIF DAI —
embedded/ARM boards, not universal x86 users.

### Step 8.2: Trigger Conditions
**Record:**
- **Trigger:** `regcache_sync()` failure during runtime resume
- **Likelihood:** Uncommon (requires hardware/bus/clock issue during
  sync)
- **Unprivileged trigger:** No — requires device access and a resume
  failure condition

### Step 8.3: Failure Mode Severity
**Record:**
- **Failure mode:** Regmap attempts live MMIO
  (`devm_regmap_init_mmio_clk` uses `hclk`) while driver considers
  device suspended; register state may be inconsistent; subsequent
  resume/audio operations may fail, hang, or produce silent corruption
- **Severity:** **MEDIUM** — real functional bug on an error path, not a
  common crash, but can leave driver in an unrecoverable inconsistent
  state without the fix

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Restores correct PM/regmap state invariant; prevents
  post-failure regmap/hardware mismatch on Rockchip SPDIF; aligns with
  established kernel patterns
- **Risk:** Very low — one line, error path only
- **Ratio:** Favorable for backport

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real bug — incomplete error-path state restoration since 2016
- Bug confirmed present in v6.18.44 tree
- One-line, obviously correct fix matching `sgtl5000` and backported
  `pca953x` precedent
- ASoC maintainer (Mark Brown) Signed-off-by
- Same author successfully got analogous fix into this stable tree
- Low regression risk

**AGAINST backport:**
- Rare trigger (`regcache_sync()` failure)
- Driver-specific, embedded Rockchip only
- No user crash report or syzbot — flagged by AI review bot
- Medium not critical severity

**UNRESOLVED:**
- Full lore review thread content (Anubis blocked)
- Whether reviewers explicitly nominated for stable
- Whether patch 5/N series has other dependencies (diff appears
  standalone)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — pattern proven elsewhere;
   maintainer SOB; no Tested-by
2. Fixes a real bug affecting users? **PASS** — state-machine bug on
   resume error path for Rockchip SPDIF users
3. Important issue? **PASS (MEDIUM)** — can cause driver
   malfunction/hang on error path, not security/data-corruption in
   common case
4. Small and contained? **PASS** — 1 line, 1 file
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — buggy code present, clean apply

### Step 9.3: Exception Categories
**Record:** Not applicable — standard bug fix, not device
ID/quirk/build/doc exception.

### Step 9.4: Decision Rationale
This is a well-understood error-path invariant fix: when runtime resume
fails after disabling cache-only mode, the driver must restore cache-
only mode before turning clocks off — exactly what suspend established.
The bug has existed since the regcache resume logic was added in 2016
and is present in this 6.18.y tree. While the trigger is uncommon and
impact is driver-specific, the fix is trivial, low-risk, maintainer-
approved, and follows a pattern already accepted for stable (pca953x,
sgtl5000). Conservative stable policy favors such surgical error-path
fixes that prevent leaving hardware drivers in inconsistent states.

---

## Verification

- **[Phase 1]** Parsed commit message and all tags from user-provided
  commit text
- **[Phase 2]** Diff analysis: 1 line added in
  `rk_spdif_runtime_resume()` error path
- **[Phase 3]** `git describe HEAD`: v6.18.44-1-g2736c32da98b9 / kernel
  6.18.44
- **[Phase 3]** `git blame -L 77,105`: regcache error path from
  3628c6987fb45 (2016)
- **[Phase 3]** `git show 3628c6987fb45`: confirmed introduction of
  regcache_sync without cache-only restore on error
- **[Phase 3]** `git show 6d94d0090527b`: related prior error-path fix
  in same function
- **[Phase 3]** `git log --grep="Restore regcache"`: no fix commit in
  tree
- **[Phase 3]** `git show 2e4bc8422cdee`: same author's analogous
  pca953x fix backported here with Cc: stable
- **[Phase 4]** `b4 dig -m "Restore regcache..."`: no match
- **[Phase 4]** WebFetch lore/patch.msgid.link URLs: blocked by Anubis —
  **UNVERIFIED** review thread content
- **[Phase 5]** Read `rockchip_spdif.c`: confirmed regmap users and PM
  ops registration
- **[Phase 5]** Read `sgtl5000.c:1135-1139`: confirmed correct reference
  pattern
- **[Phase 5]** Read `rockchip_sai.c:251-277`: same missing pattern
  (separate issue)
- **[Phase 5]** Read `drivers/base/regmap/regcache.c:551-570`: confirmed
  cache_only semantics
- **[Phase 6]** Read current `rockchip_spdif.c:66-105`: confirmed buggy
  code without fix
- **[Phase 6]** `git log --oneline -20 --
  sound/soc/rockchip/rockchip_spdif.c`: no conflicting fix present

**YES****Verdict: YES** — backport to this tree (v6.18.44).

The Rockchip SPDIF driver leaves `regcache_cache_only` disabled when
`regcache_sync()` fails during runtime resume, so later register
accesses can hit hardware while the device is still inactive. The fix is
a one-line restore of cache-only mode on that error path — same pattern
as `sgtl5000` and the author’s backported `pca953x` fix. The bug has
been present since 2016 and is still in this checkout; it’s a small,
low-risk, maintainer-signed error-path fix.

 sound/soc/rockchip/rockchip_spdif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/rockchip/rockchip_spdif.c b/sound/soc/rockchip/rockchip_spdif.c
index d365168934dc6..7a15f680de2fe 100644
--- a/sound/soc/rockchip/rockchip_spdif.c
+++ b/sound/soc/rockchip/rockchip_spdif.c
@@ -97,6 +97,7 @@ static int rk_spdif_runtime_resume(struct device *dev)
 
 	ret = regcache_sync(spdif->regmap);
 	if (ret) {
+		regcache_cache_only(spdif->regmap, true);
 		clk_disable_unprepare(spdif->mclk);
 		clk_disable_unprepare(spdif->hclk);
 	}
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH AUTOSEL 6.18-5.10] ASoC: rockchip: rockchip_pdm: Reorder clock enable sequence

[Sasha Levin]

From: bui duc phuc <phucduc.bui@gmail.com>

[ Upstream commit 3168721d6ec3b610edf6a3c22ad190722a27d276 ]

Enable the 'hclk' bus clock before the 'clk' controller clock during
runtime resume.
The bus clock provides the register access interface, so enable it before
the controller clock. This also makes the resume sequence the reverse of
the suspend sequence, which keeps the clock ordering consistent.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
Link: https://patch.msgid.link/20260602101608.45137-4-phucduc.bui@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Connection lost, reconnecting to https://agentn.us.api5.cursor.sh
(attempt 1)...
Retry attempt 1...
Connection lost, reconnecting to https://agentn.us.api5.cursor.sh
(attempt 2)...
Retry attempt 2...
Connection lost, reconnecting to https://agentn.us.api5.cursor.sh
(attempt 3)...
Retry attempt 3...
## Phase 1: Commit Message Forensics

### Step 1.1: Parse the Subject Line
**Record:** `[ASoC: rockchip: rockchip_pdm]` `[Reorder]` — Reorder the
runtime-resume clock enable sequence for the Rockchip PDM driver.

### Step 1.2: Parse All Commit Message Tags
**Record:**
- **Fixes:** — not present (expected for manual review)
- **Reported-by:** — none
- **Tested-by:** — none
- **Reviewed-by:** — none
- **Acked-by:** — none
- **Link:**
  https://patch.msgid.link/20260602101608.45137-4-phucduc.bui@gmail.com
- **Cc: stable@vger.kernel.org:** — not present (expected)
- **Signed-off-by:** bui duc phuc, Mark Brown (ASoC maintainer)

Notable: no user report, syzbot, or explicit stable nomination. Mark
Brown accepted the patch.

### Step 1.3: Analyze the Commit Body
**Record:**
- **Bug:** `rockchip_pdm_runtime_resume()` enables `pdm_clk` (controller
  clock) before `pdm_hclk` (bus clock).
- **Symptom/failure mode:** Not explicitly described (no crash, hang, or
  user report). The commit argues that register access requires the bus
  clock, so resume ordering is wrong and does not mirror suspend.
- **Version info:** none in the message.
- **Root cause:** Bus clock (`hclk`) provides the register interface; it
  must be enabled before the controller clock (`clk`). Suspend disables
  `clk` then `hclk`; resume should reverse that.

### Step 1.4: Detect Hidden Bug Fixes
**Record:** Yes — this is a PM correctness bug disguised as ordering
cleanup. Resume currently mirrors suspend instead of reversing it, which
is incorrect for clock domains where the bus clock gates register
access.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory the Changes
**Record:**
- **Files:** `sound/soc/rockchip/rockchip_pdm.c` (+/- ~6 logical lines
  in one hunk)
- **Functions modified:** `rockchip_pdm_runtime_resume()`
- **Scope:** Single-file, surgical PM fix

### Step 2.2: Code Flow Change
**Record:**
- **Hunk (runtime resume):**
  - **Before:** enable `pdm->clk`, then `pdm->hclk`; on second failure,
    disable `pdm->clk`
  - **After:** enable `pdm->hclk`, then `pdm->clk`; on second failure,
    disable `pdm->hclk`
- **Path affected:** Runtime PM resume and anything that calls it
  (system sleep resume via `pm_runtime_resume_and_get()`)

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Logic / PM correctness fix (clock enable ordering)
- **Mechanism:** Suspend disables controller clock first, then bus
  clock. Resume must enable bus clock first, then controller clock.
  Current code enables both in the same order as suspend, violating
  standard clock-domain ordering and the driver’s own probe path (probe
  enables `hclk` first).

### Step 2.4: Fix Quality
**Record:**
- Fix is obviously correct and minimal.
- Matches the pattern used in `rockchip_sai.c` and `rockchip_i2s_tdm.c`
  (hclk before functional clock on resume).
- Regression risk is very low: only reorders two existing
  `clk_prepare_enable()` calls and corresponding error-path cleanup.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame the Changed Lines
**Record:**
- Buggy ordering introduced in **fc05a5b222530** (“ASoC: rockchip: add
  support for pdm controller”, June 2017).
- Error-path cleanup added later in **ef0a098efb366** (Dec 2022).
- Bug has existed since driver introduction; present in this tree.

### Step 3.2: Follow the Fixes: Tag
**Record:** No `Fixes:` tag — not applicable.

### Step 3.3: File History for Related Changes
**Record:**
- Related prior fix: **ef0a098efb366** — missing
  `clk_disable_unprepare()` on error path in the same function (already
  in this 6.18.y tree).
- No evidence this is part of a multi-patch dependency series.
- Standalone fix.

### Step 3.4: Author's Other Commits
**Record:** Author (bui duc phuc) has other ASoC cleanup/guard patches;
this is a targeted Rockchip PDM PM fix accepted by maintainer Mark
Brown.

### Step 3.5: Dependent/Prerequisite Commits
**Record:** No dependencies. Code structures (`pdm->clk`, `pdm->hclk`,
runtime PM callbacks) all exist in this tree. Applies standalone.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Patch Discussion
**Record:**
- `b4 dig -c 8f78f7bc1806c` failed — commit not in this checkout.
- Link fetch blocked (403 / bot protection).
- Could not retrieve lore thread content.

### Step 4.2: Reviewers
**Record:** UNVERIFIED — `b4 dig -w` failed for the same reason. Mark
Brown’s Signed-off-by confirms maintainer acceptance.

### Step 4.3: Bug Report Search
**Record:** No bug report, syzbot link, or crash description in the
commit message or accessible lore thread.

### Step 4.4: Related Patches / Series
**Record:** Message-ID suffix `45137-4` suggests patch 4 of a series,
but no related mbox files for this patch were found in the workspace.
Fix itself is self-contained.

### Step 4.5: Stable Mailing List History
**Record:** UNVERIFIED — could not search lore due to access
restrictions. No `Cc: stable@vger.kernel.org` in the commit message.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `rockchip_pdm_runtime_resume()` (modified), with callers:
- `rockchip_pdm_probe()` (when runtime PM disabled)
- `rockchip_pdm_pm_ops` runtime resume callback
- `rockchip_pdm_resume()` via `pm_runtime_resume_and_get()`

### Step 5.2: Callers
**Record:**
- **Runtime PM idle/resume cycle:** common audio power-management path
- **System sleep resume:** `rockchip_pdm_resume()` →
  `pm_runtime_resume_and_get()` → `regcache_sync()`
- **Probe fallback:** only when `CONFIG_PM` disabled

### Step 5.3: Callees
**Record:** `clk_prepare_enable()`, `clk_disable_unprepare()`,
`dev_err()`

### Step 5.4: Call Chain / Reachability
**Record:**
- Resume path is reachable on Rockchip boards using PDM microphones
  (RK3328, RK3568, RV1126).
- Trigger: runtime PM resume after idle, or system suspend/resume.
- Not directly userspace-triggerable as a security primitive, but
  reachable during normal audio use and system PM.

### Step 5.5: Similar Patterns
**Record:**
- **Correct pattern:** `rockchip_sai.c` and `rockchip_i2s_tdm.c` enable
  `hclk` before functional clock on resume.
- **Same bug pattern:** `rockchip_spdif.c` also enables mclk before hclk
  on resume (not fixed by this commit).
- **PDM probe:** enables `hclk` first at line 614.

---

## Phase 6: Cross-Referencing Against the Local Tree

### Step 6.1: Does the Buggy Code Exist?
**Record:** **Yes.** Local tree is **v6.18.44** (`6.18.44`). Current
code at lines 425–435 enables `pdm->clk` before `pdm->hclk`. Bug present
since v4.13 era (2017 driver addition).

### Step 6.2: Backport Complications
**Record:** Expected **clean apply** — single hunk, no structural
changes needed. No significant recent churn in this function beyond
unrelated cleanups.

### Step 6.3: Related Fixes Already Present?
**Record:** **ef0a098efb366** (error-path cleanup in the same function)
is already in this tree. The clock-ordering fix is **not** present.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem and Criticality
**Record:** **ASoC / Rockchip PDM audio driver** — **IMPORTANT** for
embedded Rockchip platforms using PDM digital microphones; not core-
kernel, but relevant to production ARM64 boards.

### Step 7.2: Subsystem Activity
**Record:** Driver is mature but still receives maintenance (runtime PM
conversion, warning fixes, RK3568/RV1126 support). Active enough that PM
paths matter.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users of Rockchip SoCs with PDM enabled in device tree (e.g.
RK3568, RK3328, RV1126). Config/platform-specific, not universal.

### Step 8.2: Trigger Conditions
**Record:**
- Runtime PM resume after autosuspend
- System sleep resume (`rockchip_pdm_resume()`)
- Common during audio use on battery-powered/embedded devices
- Not unprivileged attack surface; normal device PM operation

### Step 8.3: Failure Mode Severity
**Record:**
- **Potential failure:** clock enable/resume problems, PDM capture
  failure after suspend/resume, possible hardware misbehavior if
  controller clock is enabled without bus clock
- **Observed/reported severity:** **UNVERIFIED** — no crash report in
  commit message; bug latent since 2017
- **Classification:** **MEDIUM** — functional PM/resume correctness on
  real hardware, not demonstrated crash/security/corruption

### Step 8.4: Risk-Benefit Ratio
**Record:**
- **Benefit:** Correct PM behavior on resume; aligns with sibling
  Rockchip drivers and probe ordering; may fix intermittent post-resume
  audio failures
- **Risk:** Very low — 6-line reorder, no API changes
- **Ratio:** Moderate benefit, very low risk; importance is somewhat
  reduced by lack of demonstrated user impact

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Compile

**FOR backport:**
- Real PM bug: resume does not reverse suspend clock order
- Bus clock must precede controller clock for register access
- Probe already enables `hclk` first; runtime resume is internally
  inconsistent
- Matches correct pattern in `rockchip_sai` and `rockchip_i2s_tdm`
- Used on system sleep resume before `regcache_sync()`
- Small, surgical, maintainer-reviewed fix
- Buggy code exists in this 6.18.y tree since 2017
- Prior fix in same function already accepted into stable tree

**AGAINST backport:**
- No user report, syzbot, or documented crash/hang
- Commit does not describe concrete user-visible symptoms
- Bug latent ~9 years without reported failures
- Stable “important issue” bar usually wants demonstrated impact (crash,
  corruption, security, deadlock)
- Similar wrong ordering exists in `rockchip_spdif.c` and has also
  persisted

**UNRESOLVED:**
- Lore review thread content
- Whether reviewers explicitly nominated for stable
- Concrete failure reports on any Rockchip board

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logically correct;
   maintainer signed off; no Tested-by
2. Fixes a real bug affecting users? **PASS** — real PM ordering bug,
   though user impact unverified
3. Important issue? **BORDERLINE/PASS** — can break audio after resume
   on embedded hardware; not proven crash/corruption
4. Small and contained? **PASS** — one function, ~6 lines
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — code exists, clean apply expected

### Step 9.3: Exception Categories
**Record:** Not a device-ID/quirk/DT/build/docs exception. Standard
driver PM bug fix.

### Step 9.4: Decision Rationale

This is a genuine runtime-PM bug in a driver that exists and is used in
the local **6.18.y** tree. The fix is minimal, obviously correct, and
aligns the PDM driver with the established Rockchip pattern (`hclk`
before functional clock on resume, reverse of suspend). The resume path
is exercised during normal audio PM and system sleep resume before
register restore.

The main weakness is the absence of any reported crash or user failure
despite the bug being present since 2017. That lowers urgency, but the
fix is still appropriate for stable because:
- it corrects objectively wrong PM ordering on a real resume path,
- it is zero-risk and self-contained,
- incorrect clock ordering on resume can cause device malfunction after
  suspend on Rockchip PDM hardware,
- a related fix in the same function is already in this stable tree.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body; no Reported-by/syzbot/Fixes
  tags found
- **[Phase 2]** Diff analysis: swap of two `clk_prepare_enable()` calls
  and corresponding error cleanup in `rockchip_pdm_runtime_resume()`
- **[Phase 3]** `git blame`: buggy order from fc05a5b222530 (2017);
  ef0a098efb366 added error-path cleanup (2022)
- **[Phase 3]** `git log --oneline -20 --
  sound/soc/rockchip/rockchip_pdm.c`: confirmed related prior fix in
  tree
- **[Phase 3]** `git merge-base --is-ancestor fc05a5b222530 HEAD`:
  original driver commit is in this tree
- **[Phase 4]** `b4 dig -c 8f78f7bc1806c`: failed — commit not in
  checkout
- **[Phase 4]** WebFetch/curl lore: blocked (403/bot protection) —
  thread content UNVERIFIED
- **[Phase 5]** Read callers: probe, PM ops, `rockchip_pdm_resume()` via
  `pm_runtime_resume_and_get()`
- **[Phase 5]** Compared patterns in `rockchip_sai.c`,
  `rockchip_i2s_tdm.c`, `rockchip_spdif.c`
- **[Phase 6]** `git describe HEAD` + Makefile: tree is v6.18.44
- **[Phase 6]** Read current `rockchip_pdm.c`: buggy ordering confirmed
  at lines 425–435
- **[Phase 6]** DT check: PDM present for rk3568/rk3328 in
  `arch/arm64/boot/dts/rockchip/`
- **[Phase 8]** Failure mode assessed as functional PM/resume issue; no
  verified crash report

**YES**

 sound/soc/rockchip/rockchip_pdm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_pdm.c b/sound/soc/rockchip/rockchip_pdm.c
index 01292b63bf9d6..115e90d3bbfe0 100644
--- a/sound/soc/rockchip/rockchip_pdm.c
+++ b/sound/soc/rockchip/rockchip_pdm.c
@@ -426,16 +426,16 @@ static int rockchip_pdm_runtime_resume(struct device *dev)
 	struct rk_pdm_dev *pdm = dev_get_drvdata(dev);
 	int ret;
 
-	ret = clk_prepare_enable(pdm->clk);
+	ret = clk_prepare_enable(pdm->hclk);
 	if (ret) {
-		dev_err(pdm->dev, "clock enable failed %d\n", ret);
+		dev_err(pdm->dev, "hclock enable failed %d\n", ret);
 		return ret;
 	}
 
-	ret = clk_prepare_enable(pdm->hclk);
+	ret = clk_prepare_enable(pdm->clk);
 	if (ret) {
-		clk_disable_unprepare(pdm->clk);
-		dev_err(pdm->dev, "hclock enable failed %d\n", ret);
+		clk_disable_unprepare(pdm->hclk);
+		dev_err(pdm->dev, "clock enable failed %d\n", ret);
 		return ret;
 	}
 
-- 
2.53.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip