Linux MultiMedia Card development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-5.10] mmc: cqhci: fix to missed endian conversions
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:44 ` Sasha Levin
  2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] mmc: dw_mmc: stop and complete DMA also in STATE_DATA_BUSY state Sasha Levin
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:44 UTC (permalink / raw)
  To: patches, stable
  Cc: Ben Dooks, Adrian Hunter, Ulf Hansson, Sasha Levin, quic_asutoshd,
	linux-mmc, linux-kernel

From: Ben Dooks <ben.dooks@codethink.co.uk>

[ Upstream commit 0aae2c73175821a71c0e20607f8e333be5b71940 ]

Fix two places where the wrong type or conversion of little-endian
types to fix the following sparse warnings:

drivers/mmc/host/cqhci-core.c:487:15: warning: incorrect type in assignment (different base types)
drivers/mmc/host/cqhci-core.c:487:15:    expected restricted __le32 [usertype]
drivers/mmc/host/cqhci-core.c:487:15:    got int
drivers/mmc/host/cqhci-core.c:566:19: warning: incorrect type in assignment (different base types)
drivers/mmc/host/cqhci-core.c:566:19:    expected unsigned long long [usertype] *task_desc
drivers/mmc/host/cqhci-core.c:566:19:    got restricted __le64 [usertype] *

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

This is a real missed little-endian conversion in CQHCI DMA descriptors,
not a sparse-only cleanup. The local tree is mainline 7.3-rc4; the bug
has been in this file since v4.16.

===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================

**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[mmc / cqhci] [fix] missed little-endian conversions on CQHCI
descriptors**

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review pipeline)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** none
- **Acked-by:** Adrian Hunter \<adrian.hunter@intel.com\> (CQHCI/SDHCI
  expert)
- **Link:** none in the commit; mailing-list thread recovered via `b4
  dig`
- **Cc: stable:** none (not a negative signal here)
- **Signed-off-by:** Ben Dooks \<ben.dooks@codethink.co.uk\> (author);
  Ulf Hansson \<ulfh@kernel.org\> (MMC maintainer). Ignore any Sasha
  Levin SOB.

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug described:** two assignments use the wrong type / omit
  `cpu_to_le*` when writing `__le32` / `__le64` CQHCI descriptors.
- **Symptom in the message:** sparse type warnings only. No stack trace,
  no reproducer, no user report.
- **Failure mode implied by the code (not stated in the message):** on a
  big-endian CPU, transfer-descriptor attributes (including
  `CQHCI_DAT_LENGTH`) and DCMD task descriptors are stored in CPU byte
  order instead of the little-endian layout the CQHCI controller
  expects.
- **Version info:** none in the message.
- **Root cause as given:** missed endian conversion / wrong pointer
  type.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Yes, this is a hidden functional endian fix.** The subject
says “fix” but the body only cites sparse. The same file already
converts sibling descriptor fields with `cpu_to_le32`/`cpu_to_le64`.
Omitting that on `__le32 *attr` and `__le64 *task_desc` is a hardware-
descriptor bug on BE, not a warning-only cleanup.

===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================

**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/mmc/host/cqhci-core.c` — 7 insertions, 7 deletions
  (14 lines touched).
- **Functions:** `cqhci_set_tran_desc()`, `cqhci_prep_dcmd_desc()`.
- **Scope:** single-file surgical fix.

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Hunk 1 (`cqhci_set_tran_desc`):** before, `*(__le32 *)attr` was
  assigned a native `int` bitfield (`VALID|END|INT|ACT|DAT_LENGTH`).
  Address words already used `cpu_to_le64`/`cpu_to_le32`. After, the
  attribute word is wrapped in `cpu_to_le32()`. Path: every CQE data
  transfer descriptor (normal I/O path).
- **Hunk 2 (`cqhci_prep_dcmd_desc`):** before, `task_desc` was `u64 *`
  and `*task_desc |= data` mixed a CPU-endian `u64` into memory obtained
  as `__le64`. After, `task_desc` is `__le64 *` and the OR uses
  `cpu_to_le64(data)`. Path: CQE direct commands (DCMD), e.g. cache
  flush / status when `MMC_CAP2_CQE_DCMD` is set.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Type / endianness]** CQHCI descriptors in memory are little-
endian. `CQHCI_DAT_LENGTH` lives in bits 16–31; DCMD
`CMD_INDEX`/`CMD_TIMING`/`RESP_TYPE` live in bits 16–24. Those fields do
not fit in byte 0, so a native store is wrong on BE. The controller
would see a swapped length and scrambled DCMD opcode/timing. The fix is
the same conversion already used in `cqhci_prep_task_desc()`
(`task_desc[0] = cpu_to_le64(desc0)`).

**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct and minimal. On little-endian,
`cpu_to_le32`/`cpu_to_le64` are no-ops, so the common CQHCI population
does not change. Regression risk is very low. No API, locking, or
structure-layout change. `update_dcmd_desc` still mutates the CPU-endian
`u64 data` before conversion (Tegra’s callback is consistent with that).

===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================

**Step 3.1: BLAME THE CHANGED LINES**
Record: Buggy assignments come from **a4080225f51dce** (“mmc: cqhci:
support for command queue enabled host”, Venkat Gopalakrishnan,
2017-11-29), first in **v4.16-rc1**. Address-field conversions were
already present then; the attribute/DCMD stores were the miss. Code has
been in-tree for many years.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The introducing commit is a4080225f51dce
(v4.16). Intermediate related work: **ee49d0321f025** (Eric Biggers,
2021) converted `cqhci_prep_task_desc()` to `__le64 *` + `cpu_to_le64`
and did not fix DCMD / transfer-attr stores.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent `cqhci-core.c` work is halt/recovery/callback related,
not endian. Not a numbered series. Standalone.

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Ben Dooks is a regular MMC endian/sparse contributor (`dw_mmc`
LE descriptor fixes, endian-agnostic IO). Not the MMC maintainer. Ulf
Hansson applied it; Adrian Hunter acked it.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: No functional prerequisite. `cqhci_set_tran_desc` was exported
in **52bf134fca61f** (v6.11) for DWCMSHC; the body bug is independent of
that. File rename `cqhci.c` → `cqhci-core.c` (**0653300224a69**) is a
path tweak only for older trees. Can apply standalone.

===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- Lore URL from `b4 dig -c 0aae2c73175821`: https://patch.msgid.link/202
  60622145036.513129-1-ben.dooks@codethink.co.uk
- **Series:** v1 only (`b4 dig -a`). Applied version is that v1.
- **Review:** Adrian Hunter Acked-by. Ulf Hansson: “Applied for next,
  thanks!”
- **NAKs / stable nominations:** none in the thread.
- WebFetch of lore.kernel.org returned 403; thread was read from `b4 dig
  -m` mbox instead.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included Adrian Hunter, Asutosh Das, Ritesh
Harjani, Ulf Hansson, linux-mmc, linux-kernel. Right maintainers were on
the original mail.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No Reported-by, bugzilla, or syzbot link. Trigger was sparse,
not a user crash report.

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone single patch. Not 2/N.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** (lore.kernel.org/stable WebFetch 403). Original
thread did not mention stable.

===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================

**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `cqhci_set_tran_desc`, `cqhci_prep_dcmd_desc`. Callers also
involved: `cqhci_prep_tran_desc`, `cqhci_request`,
`dwcmshc_set_tran_desc`.

**Step 5.2: TRACE CALLERS**
Record:
- `cqhci_set_tran_desc`: `cqhci_prep_tran_desc` (default),
  `dwcmshc_set_tran_desc` in `sdhci-of-dwcmshc.c` (Rockchip/DWC MSHC).
- `cqhci_prep_dcmd_desc`: `cqhci_request()` when `!mrq->data`.
- `cqhci_request` is `.cqe_request` — MMC command-queue I/O, not init-
  only.

**Step 5.3: TRACE CALLEES**
Record: Bitfield macros, `get_desc()`, optional `update_dcmd_desc`
(Tegra only), `cpu_to_le32`/`cpu_to_le64`. No alloc/lock changes.

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace block I/O on CQE eMMC → MMC core → `cqe_request` →
descriptor prep → doorbell. Reachable on any CQHCI host (Qualcomm, i.MX,
Tegra, MTK, Broadcom STB, Arasan, Intel GLK, TI AM654, Rockchip DWCMSHC,
etc.). BE vs LE is a kernel endian config, not a separate code path.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `cqhci_prep_task_desc` already converts correctly.
`setup_trans_desc()` still writes `VALID|ACT|END` through a `u8 *`;
those bits fit in byte 0, so a LE controller still sees the correct LSB
regardless of CPU endianness. The two sites this patch touches are the
ones that put multi-byte fields into `__le32`/`__le64` words.

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

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: This checkout is **v7.3-rc4** (`7.3.0-rc4`, HEAD
`f0100363d8c37`, origin/master). Commit **0aae2c73175821 is already an
ancestor of HEAD** (merged in `mmc-v7.3`). Pre-fix code is in
`0aae2c73^`. The unconverted stores existed from v4.16 until this
commit. The bug is in this tree’s history and in any stable series that
still has the old assignments.

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply on this tree (already applied). Older stables may
still name the file `cqhci.c` and may still have a static
`cqhci_set_tran_desc`; that is a trivial context/path adjustment, not a
semantic rework.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No earlier equivalent DCMD/attr conversion. `ee49d0321f025` only
fixed `cqhci_prep_task_desc`.

===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **MMC host / CQHCI** (`drivers/mmc/host/`). **IMPORTANT** — eMMC
command queue on widely used SoCs and some PCI hosts. Not core mm/VFS,
not staging.

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (halt, recovery, crypto, DWCMSHC callbacks).
CQHCI itself is mature (2017).

===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: CQHCI users on **big-endian** kernels. In-tree `supports-cqe` DT
is ARM64 (Qualcomm, Rockchip, Tegra, …). ARM64 has
`CONFIG_CPU_BIG_ENDIAN`. No MIPS `supports-cqe` DT. Little-endian CQHCI
(the usual phone/board case) is unaffected.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Any CQE data transfer (`cqhci_set_tran_desc`) or DCMD
(`cqhci_prep_dcmd_desc`) on BE. Common path if CQE is enabled. Not a
race. Unprivileged I/O can exercise it if the device is
mounted/accessible.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Wrong LE descriptor in DMA memory → controller misreads length /
DCMD fields → **I/O failure and possible data corruption**. **Severity:
HIGH** on BE CQHCI; **none** on LE.

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: **Benefit:** high for BE CQHCI (makes CQE descriptors spec-
correct); none for LE. **Risk:** very low (7 lines, LE no-op, no
locking/API). **Ratio:** favorable. This is the usual justification for
descriptor endian fixes in stable.

===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================

**Step 9.1: COMPILE THE EVIDENCE**

**FOR backporting:**
- Real missed `cpu_to_le*` on hardware DMA/task descriptors, present
  since v4.16.
- Sibling `cqhci_prep_task_desc` and the address fields in the same
  function already convert; these two stores were the hole.
- On BE, `DAT_LENGTH` and DCMD opcode/timing/response type would be
  byte-swapped → CQE malfunction / corruption risk.
- 7-line, obviously correct, LE no-op.
- Acked by Adrian Hunter; applied by Ulf Hansson.
- Matches the endianness/type-bug class used in stable.

**AGAINST backporting:**
- Commit text only mentions sparse; no user report, no `Fixes:`, no `Cc:
  stable`.
- Typical CQHCI hardware is ARM64/x86 LE, so most stable users see no
  runtime change.
- Author did not describe a user-visible failure.

**UNRESOLVED:**
- Whether this was discussed on stable@ (lore 403).
- Whether any production BE CQHCI board is in the wild (in-tree CQE DT
  is ARM64; ARM64 BE exists but is uncommon).

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — mechanical `cpu_to_le*` to
   match existing descriptor stores; maintainer ack.
2. Fixes a real bug that affects users? **PASS** — wrong CQHCI
   descriptor endianness on BE is a real protocol/hardware bug, not
   whitespace.
3. Important issue? **PASS** — wrong transfer length / DCMD fields can
   break CQE I/O or corrupt data. Sparse is how it was found, not the
   user-visible impact.
4. Small and contained? **PASS** — 7 lines, one file, two functions.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — already present on this
   7.3-rc4 tree; the pre-fix pattern is the long-standing one.

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/docs exception. Closest adjacent class
is **endianness correctness** (and incidentally a sparse warning). Not a
reason to reject.

**Step 9.4: MAKE YOUR DECISION**
The local tree is mainline **7.3.0-rc4**, which already contains the
fix. The evaluation is whether that fix should also be selected for
stable: the buggy stores have been in CQHCI since **v4.16**, the change
is self-contained, and it corrects hardware-visible descriptors. Sparse
was the detector; the bug is endian-incorrect DMA descriptors. Risk on
the LE majority is zero. That is appropriate for stable.

===============================================================
What the commit solves and why it matters
===============================================================
CQHCI transfer and DCMD descriptors are little-endian in memory. Two
writes used native CPU values: transfer attributes
(`VALID`/`END`/`ACT`/`DAT_LENGTH`) and the DCMD task word. On big-endian
that swaps the length and command fields the controller DMA-reads, so
command-queue I/O can fail or transfer the wrong size. Little-endian
kernels are unchanged. The patch is small, already reviewed by the CQHCI
expert, and needs no extra commits.

Risk vs benefit: very low risk, real correctness fix for BE CQHCI, no
behavior change for typical LE devices.

===============================================================
Verification
===============================================================
- [Phase 1] Parsed commit 0aae2c73175821: Acked-by Adrian Hunter; SOB
  Ben Dooks, Ulf Hansson; no Fixes/Reported-by/Cc: stable.
- [Phase 2] Diff: 7+/-7 in `cqhci_set_tran_desc` and
  `cqhci_prep_dcmd_desc` only.
- [Phase 2] Confirmed `CQHCI_DAT_LENGTH` is bits 16–31 and DCMD fields
  are bits 16+ in `cqhci.h`.
- [Phase 2] Pre-fix parent still assigned native ints/`u64` into `__le*`
  memory; addresses already used `cpu_to_le*`.
- [Phase 3] `git blame`: buggy code from a4080225f51dce (v4.16-rc1).
- [Phase 3] `cqhci_prep_task_desc` conversion added in ee49d0321f025
  (2021); DCMD/attr left unconverted.
- [Phase 3] Author history on `drivers/mmc/`: several real dw_mmc endian
  fixes.
- [Phase 3] `git merge-base --is-ancestor 0aae2c73175821 HEAD` → commit
  is in this tree.
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1-only thread; Hunter ack; Ulf
  “Applied for next”; lists linux-mmc + Hunter/Hansson.
- [Phase 4] lore WebFetch 403; used local mbox. Stable@ search
  UNVERIFIED (403).
- [Phase 5] Callers: `cqhci_request` → prep_tran_desc/prep_dcmd_desc;
  `dwcmshc_set_tran_desc` calls `cqhci_set_tran_desc`.
- [Phase 5] No semcode MCP; used git grep.
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
  kernelversion` = 7.3.0-rc4.
- [Phase 6] Fix merged via f96718fc1f614 (`mmc-v7.3`).
- [Phase 6] In-tree `supports-cqe` is ARM64-only; no MIPS matches.
- [Phase 7] CQHCI selected by MSM, i.MX, Tegra, MTK, brcmstb, Arasan,
  DWCMSHC, AM654, SDHCI PCI.
- [Phase 8] Failure mode: BE descriptor corruption → CQE I/O failure /
  possible data corruption; LE no-op.
- UNVERIFIED: production BE CQHCI user reports; stable@ discussion.

The patch is small, obviously correct, and fixes a real hardware-
descriptor endian bug that has been in this tree’s CQHCI code since
v4.16.

**YES**

 drivers/mmc/host/cqhci-core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index 178277d90c31c..00614b500b2bc 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -484,11 +484,11 @@ void cqhci_set_tran_desc(u8 *desc, dma_addr_t addr, int len, bool end,
 {
 	__le32 *attr = (__le32 __force *)desc;
 
-	*attr = (CQHCI_VALID(1) |
-		 CQHCI_END(end ? 1 : 0) |
-		 CQHCI_INT(0) |
-		 CQHCI_ACT(0x4) |
-		 CQHCI_DAT_LENGTH(len));
+	*attr = cpu_to_le32((CQHCI_VALID(1) |
+			     CQHCI_END(end ? 1 : 0) |
+			     CQHCI_INT(0) |
+			     CQHCI_ACT(0x4) |
+			     CQHCI_DAT_LENGTH(len)));
 
 	if (dma64) {
 		__le64 *dataddr = (__le64 __force *)(desc + 4);
@@ -542,7 +542,7 @@ static int cqhci_prep_tran_desc(struct mmc_request *mrq,
 static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
 				   struct mmc_request *mrq)
 {
-	u64 *task_desc = NULL;
+	__le64 *task_desc = NULL;
 	u64 data = 0;
 	u8 resp_type;
 	u8 *desc;
@@ -574,7 +574,7 @@ static void cqhci_prep_dcmd_desc(struct mmc_host *mmc,
 		 CQHCI_CMD_TIMING(timing) | CQHCI_RESP_TYPE(resp_type));
 	if (cq_host->ops->update_dcmd_desc)
 		cq_host->ops->update_dcmd_desc(mmc, mrq, &data);
-	*task_desc |= data;
+	*task_desc |= cpu_to_le64(data);
 	desc = (u8 *)task_desc;
 	pr_debug("%s: cqhci: dcmd: cmd: %d timing: %d resp: %d\n",
 		 mmc_hostname(mmc), mrq->cmd->opcode, timing, resp_type);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.18-5.15] mmc: dw_mmc: stop and complete DMA also in STATE_DATA_BUSY state
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
  2026-10-19 13:44 ` [PATCH AUTOSEL 6.18-5.10] mmc: cqhci: fix to missed endian conversions Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
  2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.12] mmc: moxart: report DMA completion timeout Sasha Levin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
  To: patches, stable
  Cc: Marek Szyprowski, Shawn Lin, Ulf Hansson, Sasha Levin, jh80.chung,
	linux-mmc, linux-kernel

From: Marek Szyprowski <m.szyprowski@samsung.com>

[ Upstream commit cb3fcb8d118b874178251d1f02613f60d25e4ddc ]

When transfer was stopped in STATE_DATA_BUSY state, there was no call to
dma_ops->cleanup function, so the DMA mapped buffer was never properly
unmapped. Fix this by calling dw_mci_stop_dma() function also in that
state to ensure proper cleanup call when DMA transfer was used.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The local checkout is mainline **v7.3-rc4** (`origin/master`). The
candidate is `cb3fcb8d118b8` (`mmc: dw_mmc: stop and complete DMA also
in STATE_DATA_BUSY state`): a one-line DMA-cleanup hole in a host used
on Exynos, Rockchip, and similar SoCs.

## Phase 1: Commit message forensics

**Step 1.1 — Subject**
Record: **[mmc/dw_mmc] [stop and complete] Call DMA stop/cleanup in
`STATE_DATA_BUSY` so mapped buffers are unmapped.**

**Step 1.2 — Tags**
Record:
- Signed-off-by: Marek Szyprowski (author; DMA-mapping maintainer)
- Reviewed-by: Shawn Lin (dw_mmc co-maintainer)
- Signed-off-by: Ulf Hansson (MMC maintainer)
- No Fixes:, Reported-by, Tested-by, Link:, or Cc: stable (absence is
  expected here)

**Step 1.3 — Body**
Record: **Bug:** finishing a transfer in `STATE_DATA_BUSY` never called
`dma_ops->cleanup`, so a DMA-mapped SG list was left mapped.
**Symptom:** DMA mapping leak (DMA-API/IOMMU leftover mappings). **Root
cause:** `dw_mci_stop_dma()` already runs in the sending-data / error
paths, but not in `STATE_DATA_BUSY` before `host->data = NULL`.
**Versions:** none stated.

**Step 1.4 — Hidden bug fix?**
Record: **No disguise.** It is an explicit resource-cleanup fix.

## Phase 2: Diff analysis

**Step 2.1 — Inventory**
Record: `drivers/mmc/host/dw_mmc.c` **+1 / −0**. Function:
`dw_mci_work_func()`. **Single-file, one-line surgical fix.**

**Step 2.2 — Flow**
Record: In `case STATE_DATA_BUSY:`, after `EVENT_DATA_COMPLETE` and
`dw_mci_stop_fault_timer()`, **before** `host->data = NULL`, the code
now calls `dw_mci_stop_dma(host)`. That function stops the DMA engine if
`host->using_dma` and then runs `dma_ops->cleanup()`. Previously this
state only nulled `host->data` and completed the request.

**Step 2.3 — Bug mechanism**
Record: **Resource leak / DMA mapping leak.** `dw_mci_dma_cleanup()`
unmaps only when `host->data` is set and `host_cookie == COOKIE_MAPPED`.
Clearing `host->data` first made later cleanup a no-op.

Verified paths:
- Success: IDMAC/EDMAC `dw_mci_dmac_complete_dma()` already calls
  cleanup, then `EVENT_XFER_COMPLETE` moves the machine into
  `STATE_DATA_BUSY`. Second cleanup is a no-op (`COOKIE_UNMAPPED`).
- Error from `STATE_SENDING_DATA`: already calls `dw_mci_stop_dma()`.
- **DMA start failure then PIO fallback:** `dw_mci_submit_data_dma()`
  sets `using_dma = 1` after a successful map, then on
  `dma_ops->start()` failure calls `stop()` but **not** cleanup, and
  **does not** clear `using_dma`. PIO then completes via
  `STATE_DATA_BUSY`. Without this line the mapping is left. Verified:
  `using_dma` is assigned only at lines 998 and 1010; PIO fallback
  (lines 1070–1103) never clears it.
- `COOKIE_PRE_MAPPED` block I/O still unmaps in `dw_mci_post_req()`;
  this change does not double-unmap that path.

**Step 2.4 — Fix quality**
Record: **Obviously correct** and matches the other data-path states.
Cleanup is idempotent. `dw_mci_start_request()` zeros `pending_events`,
so the extra `EVENT_XFER_COMPLETE` bit from `dw_mci_stop_dma()` does not
leak into the next request. **Regression risk: low.**
`dmaengine_terminate_async()` / IDMAC reset after a completed transfer
is the same stop used on errors; IDMAC start already resets the engine.

## Phase 3: Git history

**Step 3.1 — Blame**
Record: `host->data = NULL` in `STATE_DATA_BUSY` is from
**f95f3850f7a9e1** (Will Newton, 2011-01-02), first in **v2.6.38**. The
hole is old.

**Step 3.2 — Fixes: tag**
Record: **None.** Related older fix **faecf41106123** (“fix unmap sg
twice when finding data err”, 2016) made cleanup skip IDMAC complete
when `EVENT_DATA_ERROR` is set, so `stop_dma()` is the unmap path on
errors. That makes missing `stop_dma()` in `STATE_DATA_BUSY` more
important, not less.

**Step 3.3 — File history**
Record: Recent dw_mmc work is refactor (board struct, PM macros). This
commit is standalone, not `N/M`. No follow-up in `cb3fcb8d118b8..HEAD`
except an unrelated `dw_mci_pmops` move.

**Step 3.4 — Author**
Record: Marek Szyprowski is the DMA-mapping maintainer; first dw_mmc
commit in this file. Shawn Lin (dw_mmc maintainer) reviewed it. Ulf
Hansson applied it.

**Step 3.5 — Dependencies**
Record: **None.** It only calls existing `dw_mci_stop_dma()`. Applies
standalone.

## Phase 4: Mailing list

**Step 4.1**
Record: https://patch.msgid.link/20260603083935.3201411-1-
m.szyprowski@samsung.com
**v1 only** (no v2/v3). Thread (3 messages via `b4 dig -m`):
1. Patch (3 Jun 2026)
2. Shawn Lin: Reviewed-by
3. Ulf Hansson: “Applied for next, thanks! Please, let me know if you
   think there is reason to tag this for stable kernels too.” **No
   reply** in the thread.

**Step 4.2**
Record: CCs were linux-mmc, Jaehoon Chung, Shawn Lin, Ulf Hansson — the
right maintainers.

**Step 4.3**
Record: No syzbot/bugzilla. Author describes a mapping leak, not a crash
log.

**Step 4.4**
Record: Standalone, not part of a series.

**Step 4.5**
Record: **UNVERIFIED** — lore.kernel.org/stable is behind bot protection
(403/Anubis). linux-mmc thread has Ulf’s open stable question and no
nomination.

## Phase 5: Code semantics

**Step 5.1**
Record: Modified: `dw_mci_work_func`. Callee: `dw_mci_stop_dma` →
`dma_ops->stop` / `dma_ops->cleanup` (`dw_mci_idmac_stop_dma` or
`dw_mci_edmac_stop_dma`, then `dw_mci_dma_cleanup`).

**Step 5.2**
Record: `dw_mci_work_func` is the host BH work (`host->bh_work`), queued
from IRQ/timeout/DMA complete. Reachable on every data transfer for this
host.

**Step 5.3**
Record: Completes MMC requests (`dw_mci_data_complete`,
`dw_mci_request_end` → `mmc_request_done`).

**Step 5.4**
Record: Userspace (block I/O, ioctl, card init) triggers it via the MMC
core. **Yes, reachable from userspace** on DW MMC hardware.

**Step 5.5**
Record: Same `dw_mci_stop_dma()` already used in `STATE_SENDING_CMD`
(cmd+data error) and twice in `STATE_SENDING_DATA`. This fills the
missing sibling.

## Phase 6: This tree

**Step 6.1**
Record: Tree is **v7.3-rc4** (`git describe`:
`v7.3-rc4-27-gf0100363d8c37`, `VERSION=7 PATCHLEVEL=3`). Candidate **is
an ancestor of HEAD**. Buggy pattern existed here before that commit;
introduced in v2.6.38.

**Step 6.2**
Record: **Clean apply** on this tree (already applied). Context
(`dw_mci_stop_fault_timer` then `host->data = NULL`) is present.

**Step 6.3**
Record: No other fix for this hole. faecf41106123 is a related double-
unmap fix, not a substitute.

## Phase 7: Subsystem

**Step 7.1**
Record: **MMC host (Synopsys DesignWare).** **IMPORTANT** — eMMC/SD on
Exynos, Rockchip, Hisilicon, StarFive, BlueField, K3 (`MMC_DW_*` in
Kconfig). Not core mm/VFS, not staging.

**Step 7.2**
Record: File is actively maintained (refactors in 2026 plus this fix).
The state machine itself is old and stable.

## Phase 8: Impact and risk

**Step 8.1**
Record: **Driver-specific / config-specific** (`CONFIG_MMC_DW`). Large
ARM/embedded population.

**Step 8.2**
Record: Hits whenever `STATE_DATA_BUSY` finishes a DMA-using request
that did not already go through `stop_dma()`. Common path still unmaps
via the DMA complete callback or `post_req`. The leak is real on **DMA-
submit failure → PIO**, and on any DATA_BUSY completion that never ran
complete/stop. Unprivileged block I/O can reach the host.

**Step 8.3**
Record: **DMA mapping leak.** Leftover mappings: DMA-API warnings, IOMMU
table growth, coherency issues if pages are reused while still mapped.
**Severity: HIGH** on the leak path, **MEDIUM** overall because the hot
success path already unmaps.

**Step 8.4**
Record: **Benefit: high** for DW MMC users (correct DMA lifetime, stop
IDMAC after data-complete). **Risk: very low** (1 line, idempotent
cleanup). **Ratio: favorable.**

## Phase 9: Synthesis

**For backporting**
- Real DMA-mapping leak; author is the DMA-mapping maintainer.
- One line, same helper already used in sibling states.
- dw_mmc co-maintainer Reviewed-by; MMC maintainer applied it.
- Hardware is common; bug dates to v2.6.38.
- Cleanup is idempotent; next request clears pending events.
- No series dependency, no follow-up revert.

**Against**
- No Fixes:/Cc: stable (expected in this pipeline).
- Ulf explicitly asked whether it belonged in stable; author never
  answered.
- No crash/syzbot report; success-path unmap often already happened.
- Leak is most clearly proven on the DMA-start-failure → PIO path, not
  every transfer.

**Unresolved**
- No stable@ discussion found (lore blocked).
- Exact reproducer Marek hit is not in the mail (only the mechanism).

**Stable rules**
1. Obviously correct and tested? **PASS** — 1-line, reviewed, in
   mainline since 2026-07-06.
2. Real user-affecting bug? **PASS** — leftover DMA mappings on DW MMC.
3. Important (crash/security/corruption/deadlock)? **PASS** — mapping
   leak can WARN, exhaust IOMMU, and cause coherency bugs; not a panic,
   but it is a real DMA-lifetime bug, not style.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS**.

**Exception category:** none (not a device ID / quirk / DT / build / doc
patch). It is a normal contained bugfix.

Ulf’s “tag for stable?” question is caution, not a NAK. The instructions
say missing stable tags must not drive a NO. On technical merit this
belongs in stable: tiny, correct, and it closes a DMA unmap hole in a
widely deployed MMC host.

Verification:
- [Phase 1] Parsed tags: Marek SOB, Shawn Lin R-b, Ulf SOB; no
  Fixes/Reported-by/Cc: stable
- [Phase 2] Diff: +1 line `dw_mci_stop_dma(host)` in `STATE_DATA_BUSY`
  before `host->data = NULL`
- [Phase 2] `dw_mci_stop_dma`/`dw_mci_dma_cleanup` read; cleanup gated
  on `COOKIE_MAPPED`
- [Phase 2] IDMAC/EDMAC complete path already calls cleanup; `using_dma`
  not cleared on DMA-start failure (lines 998, 1010, 1039–1045, 1070)
- [Phase 3] blame: f95f3850f7a9e1 (v2.6.38); candidate cb3fcb8d118b8;
  related faecf41106123 double-unmap
- [Phase 3] `git merge-base --is-ancestor cb3fcb8d118b8 HEAD` — true; no
  follow-up fix on dw_mmc.c
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 only; Shawn R-b; Ulf applied and
  asked about stable; no author reply
- [Phase 4] lore WebFetch 403; mbox used instead
- [Phase 5] callers: BH work from IRQ/DTO/DMA complete; userspace via
  MMC core
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
  kernelversion` = 7.3.0-rc4
- [Phase 7] MAINTAINERS: Jaehoon Chung, Shawn Lin; Kconfig MMC_DW_*
  platforms
- [Phase 8] Failure mode: DMA mapping leak, severity HIGH on leak path
- UNVERIFIED: stable@ archive search (Anubis); exact runtime stack Marek
  saw

**YES**

 drivers/mmc/host/dw_mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index c5db92bbb0946..90cc41901edaa 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2213,6 +2213,7 @@ static void dw_mci_work_func(struct work_struct *t)
 			}
 
 			dw_mci_stop_fault_timer(host);
+			dw_mci_stop_dma(host);
 			host->data = NULL;
 			set_bit(EVENT_DATA_COMPLETE, &host->completed_events);
 			err = dw_mci_data_complete(host, data);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.18-6.12] mmc: moxart: report DMA completion timeout
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
  2026-10-19 13:44 ` [PATCH AUTOSEL 6.18-5.10] mmc: cqhci: fix to missed endian conversions Sasha Levin
  2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] mmc: dw_mmc: stop and complete DMA also in STATE_DATA_BUSY state Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
  2026-10-19 13:48 ` [PATCH AUTOSEL 6.18] mmc: rtsx_usb_sdmmc: suppress false CD after init timeout Sasha Levin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
  To: patches, stable
  Cc: Pengpeng Hou, Ulf Hansson, Sasha Levin, linux-mmc, linux-kernel

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

[ Upstream commit e29e650077e01dddf69bc8fac31e0cd60043aa7c ]

moxart_transfer_dma() waits for the DMA completion but ignores
wait_for_completion_interruptible_timeout(). It then unconditionally
reports the full transfer length in data->bytes_xfered.

Terminate the DMA channel and set data->error when the wait is
interrupted or times out. Only report host->data_len as transferred
after the completion is observed.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The local tree is mainline **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`).
The candidate is `e29e650077e01` (“mmc: moxart: report DMA completion
timeout”). Analysis below is of that commit against this checkout.

===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================

**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[mmc/moxart] [report] [DMA wait timeout was ignored; now
surface it as an I/O error]**

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Signed-off-by: Pengpeng Hou \<pengpeng@iscas.ac.cn\> (author)
- Signed-off-by: Ulf Hansson \<ulfh@kernel.org\> (MMC host maintainer;
  applied)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, Link:,
  or Cc: stable (absence is expected for this review)
- Notable: maintainer SOB only; no user/syzbot report

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `moxart_transfer_dma()` calls
  `wait_for_completion_interruptible_timeout()` but discards the return
  value, then always sets `data->bytes_xfered = host->data_len`.
- **Symptom:** A timed-out or interrupted DMA transfer is reported as a
  full successful transfer.
- **Fix:** On `timeout <= 0`, `dmaengine_terminate_sync()`, set
  `data->error`, skip `bytes_xfered` update, still unmap.
- **Version info:** none in the message. Driver dates to v3.16
  (`1b66e94e6b999`).
- **Root cause (author):** ignored wait status plus unconditional
  success accounting.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not disguised. Explicit timeout/error-path fix. Also stops DMA
before `dma_unmap_sg()`, which is a memory-safety fix even though the
subject only says “report”.

===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/mmc/host/moxart-mmc.c` — **+9 / −2** (11-line net in
  one function)
- Function: `moxart_transfer_dma()`
- Scope: **single-file surgical fix**

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record (verified against `e29e650077e01^`):
- **Before:** map SG → optionally submit DMA → wait (return ignored) →
  always `bytes_xfered = host->data_len` → unmap. Timeout, signal, or
  failed `dmaengine_prep_slave_sg()` still looked like a full transfer,
  and unmap ran while DMA could still be active.
- **After:** same wait; if `timeout <= 0`, terminate the channel, set
  `data->error` to `-ERESTARTSYS` or `-ETIMEDOUT`, skip accounting, then
  unmap. Success path unchanged.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** logic/correctness (ignored wait) **plus** memory safety
  (unmap while DMA may still run) **plus** false success to the MMC
  block layer.
- **Mechanism:** `wait_for_completion_interruptible_timeout()` returns 0
  on timeout, `-ERESTARTSYS` if interrupted, remaining jiffies on
  success (`kernel/sched/completion.c`). `timeout ?: -ETIMEDOUT` maps
  those correctly.
- MMC block completion uses `bytes_xfered` as success size
  (`mmc_blk_mq_complete_rq()`). A full `bytes_xfered` with no error
  completes the request as OK — silent data corruption on write,
  stale/wrong data on read.
- `sh_mmcif.c` already uses the same terminate + `data->error` pattern.

**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct and minimal. Matches documented wait return values
  and existing MMC DMA drivers.
- `moxart_transfer_dma()` is called with the host spinlock dropped
  (`moxart_request()`), so `dmaengine_terminate_sync()` is in process
  context as required.
- Residual issues (pre-existing, not introduced): still waits even if
  `desc` is NULL; PIO wait at line 436 still ignores its return. After
  DMA error, `moxart_request()` still waits for `MASK_DATA`. Harmless
  extra delay; `data->error` is already set.
- Regression risk: **low**.

===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================

**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Function and ignored wait: `1b66e94e6b999` (“mmc: moxart: Add MOXA ART
  SD/MMC driver”), **v3.16-rc1**.
- Unconditional `bytes_xfered = host->data_len` after wait:
  `54fd8cd617996` (2024, “Fix accounting in DMA transfer”) — moved the
  assignment after the wait but still did not check the wait result.
- Bug class (ignored wait + unmap-while-active) has been in the driver
  since introduction.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. Introduced-by is
`1b66e94e6b999`; `git merge-base --is-ancestor 1b66e94e6b999 HEAD`
succeeds.

**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Recent: platform helpers, `devm_mmc_alloc_host`, sg_miter revert, DMA
  accounting, `moxart_use_dma()`.
- Not part of a numbered series. Standalone.
- Prerequisite for a clean textual apply of this exact hunk:
  `54fd8cd617996` (already in this tree). The timeout check itself does
  not depend on that accounting change.

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Pengpeng Hou is not the MMC maintainer; many nearby commits are small
  correctness/ID-table patches.
- Ulf Hansson applied it (“Applied for next, thanks!”).
- Only this moxart change from the author in `drivers/mmc/host/`.

**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Uses `dmaengine_terminate_sync()` from `include/linux/dmaengine.h`
  (added `b36f09c3c441a`, 2015). Present here; used by other MMC hosts.
- Standalone. No new helpers.

===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c e29e650077e01` →
  https://patch.msgid.link/20260623061117.35280-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: **v1 only**, applied as submitted.
- Thread (saved via `b4 dig -m`): Ulf Hansson, 2026-07-06: “Applied for
  next, thanks!” No NAK, no requested changes, no Cc: stable from
  reviewers.
- lore WebFetch returned HTTP 403; discussion recovered from the mbox.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc Pengpeng Hou, Ulf Hansson, Binbin Zhou,
Huacai Chen, linux-mmc, linux-kernel. Maintainer was the recipient and
applied it. Zhou/Chen look like list/CC noise (Loongson MMC), not
reviewers of this driver.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No Reported-by, no bugzilla/syzbot link. Code-inspection fix,
not a user report.

**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone v1. Same author also has `mmc: sdhci-of-ma35d1: add
missing MODULE_DEVICE_TABLE()` — unrelated.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable WebFetch returned 403. `git log
--grep='DMA completion timeout' -- drivers/mmc/host/` only this commit.
**UNVERIFIED:** no confirmation of a prior stable-list NAK.

===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================

**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `moxart_transfer_dma()` only. Uses `moxart_dma_complete()` as
DMA callback (`complete(&host->dma_complete)`).

**Step 5.2: TRACE CALLERS**
Record:
- Sole caller: `moxart_request()` (`mmc_host_ops.request`).
- Path: MMC core request → host `.request` → DMA if `data_len >
  fifo_width && have_dma`.
- Process context, lock dropped. Userspace block I/O to the SD card
  reaches this.

**Step 5.3: TRACE CALLEES**
Record: `dma_map_sg` / `dmaengine_prep_slave_sg` / `dmaengine_submit` /
`dma_async_issue_pending` / wait / (new) `dmaengine_terminate_sync` /
`dma_unmap_sg`.

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable from normal MMC/SD reads and writes on MOXA ART
(UC-7112-LX and related). Timeout is 1000 ms (`host->timeout =
msecs_to_jiffies(1000)`). Unprivileged I/O on that hardware can hit it.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record:
- PIO path in the same file already sets `data->error = -ETIMEDOUT` on
  FIFO wait failure — DMA was inconsistent.
- `moxart_request()` still ignores the PIO completion wait return (line
  436). Same class of bug, not this patch.
- `sh_mmcif.c` checks timeout/interrupt, zeros `bytes_xfered` on error,
  calls `dmaengine_terminate_sync()`.

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

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes. File and function have been here since v3.16. Pre-image
confirmed at `e29e650077e01^`. `git merge-base --is-ancestor
e29e650077e01 HEAD` succeeds (fix already on this mainline snapshot).
Introduced ~v3.16.

**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Hunk is local to `moxart_transfer_dma()`. APIs used exist here.
**Clean apply** on this tree (already applied). No conflicting later
logic change except an unrelated platform-helper commit.

**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `54fd8cd617996` only moved accounting after the wait; it did not
check the wait. No other timeout fix for this DMA path.

===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================

**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record:
- Subsystem: MMC host (`drivers/mmc/host/moxart-mmc.c`),
  `CONFIG_MMC_MOXART`, `depends on ARCH_MOXART || COMPILE_TEST`.
- Hardware: MOXA ART (Faraday FA526 ARMv4), UC-7101 / UC-7112-LX /
  UC-7110 / IA240 / IA241 / IA3341.
- Criticality: **PERIPHERAL** (legacy industrial SoC), but the bug is in
  the host’s main DMA I/O path.

**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Driver is mature (2014) with occasional fixes. Not an active
rewrite. Long-standing bug.

===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================

**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific / platform-specific** — MOXA ART boards with
this MMC host and DMA enabled. Not universal.

**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: DMA completion misses the 1s wait, or the waiter is
  interrupted.
- Also: `dma_map_sg`/prep failure (completion never signaled) previously
  still reported a full transfer.
- Commonness: timeout is not every I/O; when it happens the old code
  treated it as success.
- Unprivileged userspace I/O on that hardware can reach it.

**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- False `bytes_xfered` + no `data->error` → block layer completes I/O as
  OK (`mmc_blk_mq_complete_rq` uses `bytes_xfered`). **Data
  corruption.**
- `dma_unmap_sg()` while the engine may still be running → DMA to
  unmapped memory, crash or memory corruption.
- Later `moxart_wait_for_status(MASK_DATA)` can still timeout and set
  `data->error`, but unmap already happened inside
  `moxart_transfer_dma()`, and if the controller finished while the DMA
  engine did not, MASK_DATA can succeed and the false-success path
  remains.
- Severity: **HIGH** (corruption / use of unmapped DMA buffers). Not a
  panic on every timeout, but the failure mode is serious.

**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** HIGH for MOXA ART users (correct I/O error, DMA stopped
  before unmap). LOW globally (few platforms).
- **Risk:** VERY LOW (9 lines, error path, maintainer-applied, same
  pattern as other hosts).
- **Ratio:** benefit outweighs risk for a stable backport of this driver
  fix.

===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================

**Step 9.1: COMPILE THE EVIDENCE**

FOR backporting:
- Real bug: ignored wait, fake full transfer, unmap while DMA may still
  run.
- Failure mode is data corruption and DMA-to-unmapped-memory, not
  cosmetics.
- Small (+9/−2), obviously correct, process-context terminate.
- PIO path already reports timeout; DMA did not.
- Bug present since v3.16; DMA I/O path of an in-tree driver.
- MMC maintainer applied v1 with no objections.
- `dmaengine_terminate_sync` and the surrounding function exist in this
  tree.

AGAINST:
- Niche ARMv4 industrial SoC; no Reported-by / syzbot.
- Author looks like an audit-style contributor; not a hardware user
  report.
- PIO completion wait still ignores its return (incomplete sibling).
- No Fixes:/Cc: stable (not a negative per the review rules).

UNRESOLVED:
- Stable-list discussion could not be fetched (HTTP 403).
- No public confirmation of how often DMA actually times out on
  UC-7112-LX hardware.

**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — return-value handling
   matches the API; maintainer applied it; same pattern as `sh_mmcif`.
2. Fixes a real bug that affects users? **PASS** — false success and
   unmap-while-active on this host’s DMA path.
3. Important issue? **PASS** — data corruption and DMA memory safety;
   HIGH, not a panic-on-boot.
4. Small and contained? **PASS** — one function, 9 insertions.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — pre-image and APIs are here;
   commit is already an ancestor of HEAD.

**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not a device ID, quirk, DT, build, or doc fix). Qualifies
as a normal bugfix.

**Step 9.4: MAKE YOUR DECISION**
This tree has the buggy DMA path. The patch is a small, maintainer-acked
correction of ignored DMA completion that otherwise reports a full
transfer and unmaps buffers while the engine may still run. That is
stable-worthy even on a niche host: the risk is negligible and the
failure mode is corruption.

Verification:
- [Phase 1] Parsed tags: author + Ulf Hansson SOB only; no
  Fixes/Reported-by/Cc: stable
- [Phase 1] Body: ignored wait + unconditional `bytes_xfered`
- [Phase 2] Diff: +9/−2 in `moxart_transfer_dma()`; timeout<=0 →
  terminate_sync, set error, skip accounting
- [Phase 2] Pre-image at `e29e650077e01^` confirmed ignored wait then
  `bytes_xfered = host->data_len`
- [Phase 2] `wait_for_completion_interruptible_timeout` returns
  -ERESTARTSYS / 0 / remaining jiffies (`kernel/sched/completion.c`)
- [Phase 2] `mmc_blk_mq_complete_rq()` completes OK when `bytes_xfered
  != 0`
- [Phase 3] Blame: wait from `1b66e94e6b999` (v3.16); accounting move
  `54fd8cd617996` still ignored wait
- [Phase 3] `git merge-base --is-ancestor 1b66e94e6b999 HEAD` and
  `e29e650077e01 HEAD` both true
- [Phase 3] `dmaengine_terminate_sync` added in `b36f09c3c441a` (2015);
  present in this tree
- [Phase 4] `b4 dig`: lore
  https://patch.msgid.link/20260623061117.35280-1-pengpeng@iscas.ac.cn ;
  v1 only
- [Phase 4] mbox: Ulf “Applied for next, thanks!”; no NAK; no Cc: stable
- [Phase 4] WebFetch lore/stable: HTTP 403 (UNVERIFIED stable-list
  history)
- [Phase 5] Only caller `moxart_request()`; lock dropped; 1s timeout
- [Phase 5] Sibling `sh_mmcif` uses the same terminate/error pattern
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
  kernelversion` = 7.3.0-rc4
- [Phase 6] Buggy DMA wait present before the commit; patch already on
  HEAD
- [Phase 7] `CONFIG_MMC_MOXART` / ARCH_MOXART (UC-7112-LX family)
- [Phase 8] Failure mode: false MMC success + DMA unmap while active;
  HIGH
- UNVERIFIED: frequency of DMA timeouts on real MOXA hardware; stable
  mailing-list discussion

The patch is small, correct, and fixes a real DMA-timeout path that can
complete I/O as success and unmap live buffers. That meets stable
criteria for this tree.

**YES**

 drivers/mmc/host/moxart-mmc.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index 3dd8f232052f6..4a4d8b19b18cc 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -264,6 +264,7 @@ static void moxart_transfer_dma(struct mmc_data *data, struct moxart_host *host)
 	u32 len, dir_slave;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *dma_chan;
+	long timeout;
 
 	if (host->data_len == data->bytes_xfered)
 		return;
@@ -296,11 +297,17 @@ static void moxart_transfer_dma(struct mmc_data *data, struct moxart_host *host)
 		dma_async_issue_pending(dma_chan);
 	}
 
-	wait_for_completion_interruptible_timeout(&host->dma_complete,
-						  host->timeout);
+	timeout = wait_for_completion_interruptible_timeout(&host->dma_complete,
+							    host->timeout);
+	if (timeout <= 0) {
+		dmaengine_terminate_sync(dma_chan);
+		data->error = timeout ?: -ETIMEDOUT;
+		goto unmap;
+	}
 
 	data->bytes_xfered = host->data_len;
 
+unmap:
 	dma_unmap_sg(dma_chan->device->dev,
 		     data->sg, data->sg_len,
 		     mmc_get_dma_dir(data));
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.18] mmc: rtsx_usb_sdmmc: suppress false CD after init timeout
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.12] mmc: moxart: report DMA completion timeout Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
  2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] mmc: sdhci-of-ma35d1: add missing MODULE_DEVICE_TABLE() Sasha Levin
  2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] mmc: block: reject invalid perdev_minors before division Sasha Levin
  5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
  To: patches, stable
  Cc: Sean Rhodes, Greg Kroah-Hartman, Ulf Hansson, Sasha Levin,
	linux-mmc, linux-kernel

From: Sean Rhodes <sean@starlabs.systems>

[ Upstream commit 7185b6d33b170cae54fb562ead44452810b6c231 ]

Some Realtek USB SD readers with a tray keep raw SD_CD asserted
when an empty tray is inserted. The MMC core then repeatedly tries to
initialize non-existent media, sees command timeouts, calls ->get_cd()
again, and starts the same detect cycle over.

Do not qualify media by open-coding MMC commands in ->get_cd().
Instead, let the normal MMC rescan path probe the card. If an
initialization command times out before a card has been attached,
suppress the raw SD_CD signal so the host can settle and the USB
parent can autosuspend.

Clear the suppression only when raw SD_CD drops. On the affected tray
reader, changing media requires removing and reinserting the tray, so
a low CD transition is the signal that a new insertion attempt can be
trusted again.

Signed-off-by: Sean Rhodes <sean@starlabs.systems>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The change
is a hardware quirk in `drivers/mmc/host/rtsx_usb_sdmmc.c`: tray-style
Realtek USB readers can keep `SD_CD` asserted with no media, so MMC init
times out, `->get_cd()` still says present, and the detect cycle never
stops.

===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================

Step 1.1: PARSE THE SUBJECT LINE
Record: **[mmc/rtsx_usb_sdmmc] [suppress] [stop false card-detect after
init timeout]**
- Subsystem: `mmc: rtsx_usb_sdmmc`
- Verb: “suppress” (workaround / prevent a false-positive CD loop)
- Intent: do not keep reporting media present after an init timeout

Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- **Fixes:** none (expected for this review)
- **Reported-by / Tested-by / Reviewed-by / Cc: stable:** none in the
  commit
- **Acked-by:** Greg Kroah-Hartman `<gregkh@linuxfoundation.org>`
- **Signed-off-by:** Sean Rhodes `<sean@starlabs.systems>` (author); Ulf
  Hansson `<ulfh@kernel.org>` (MMC maintainer)
- **Link:** none on the commit; mailing-list thread recovered via `b4
  dig` (Phase 4)
- Notable: Greg’s Ack is on the series, not a `Cc: stable` tag. Absence
  of `Fixes:` / `Cc: stable` is not treated as a negative.

Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- **Bug:** Some Realtek USB SD readers with a tray leave raw `SD_CD`
  asserted when the tray is empty.
- **Symptom:** MMC core keeps initializing non-existent media, hits
  command timeouts, calls `->get_cd()` again, and repeats. Cover letter
  (Phase 4) also reports USB reset churn and the USB parent never
  autosuspending.
- **Root cause:** `->get_cd()` trusts raw `SD_CD` and does not
  distinguish “tray in, no card” from real media.
- **Fix approach:** Do not probe from `->get_cd()`. After an init
  timeout with no attached `mmc->card`, suppress raw `SD_CD` until it
  actually drops (tray remove/reinsert).
- **Version info in the message:** none. Cover letter tested on Linux
  7.0.0-27-generic, RTS5129 `0bda:0129`, StarLite Mk V.

Step 1.4: DETECT HIDDEN BUG FIXES
Record: **Not hidden.** It is an explicit hardware-quirk / detect-loop
fix. “Do not qualify media by open-coding MMC commands in `->get_cd()`”
refers to dropping an RFC approach, not cleanup-for-its-own-sake.

===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================

Step 2.1: INVENTORY THE CHANGES
Record:
- **File:** `drivers/mmc/host/rtsx_usb_sdmmc.c` only
- **Scope:** +12 / −1, single-file surgical change
- **Functions / sites:**
  - `struct rtsx_usb_sdmmc`: add `bool suppress_cd`
  - `sdmmc_get_cd()`: honor/clear suppression
  - `sdmmc_request()`: set suppression on pre-attach `-ETIMEDOUT`
  - `rtsx_usb_init_host()`: initialize the flag

Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record:
1. **`sdmmc_get_cd()`:** Before: `val & SD_CD` → `card_exist = true`,
   return 1. After: if CD is low, clear `suppress_cd` and report no
   card; if CD is high but `suppress_cd` is set, fall through to
   `no_card` (return 0).
2. **`sdmmc_request()` error path:** Before: any `cmd->error` re-ran
   `sdmmc_get_cd()`. After: if `!mmc->card && cmd->error == -ETIMEDOUT`,
   set `suppress_cd` first, then re-run `get_cd()` so the next detect
   sees no media.
3. **Init:** `suppress_cd = false` in `rtsx_usb_init_host()`.

Step 2.3: IDENTIFY THE BUG MECHANISM
Record: **[Hardware workaround + logic/correctness]**
- Broken: raw `SD_CD` is a false present signal on empty-tray
  RTS5129-class readers.
- MMC core (`mmc_rescan`) treats `get_cd()==1` as “probe now”; init
  times out; `sdmmc_request` calls `get_cd()` again; still 1; with
  `MMC_CAP_NEEDS_POLL` set from `runtime_resume`, detect is rescheduled
  every `HZ` (`drivers/mmc/core/core.c` `out:` path).
- Fix: after a timeout before `mmc->card` exists, lie about CD until the
  line drops, so rescan takes the `get_cd()==0` power-off path instead
  of retrying init.

Step 2.4: ASSESS THE FIX QUALITY
Record:
- **Quality:** Minimal and readable. `WRITE_ONCE`/`READ_ONCE` are
  justified: `sdmmc_get_cd()` is also called from `mmc_rescan` and
  `rtsx_usb_sdmmc_runtime_resume`, not only from `sdmmc_request`.
- **Regression risk (low–medium, contained):** a real card that times
  out on first init with `SD_CD` still high is hidden until CD drops.
  That matches tray hardware (remove/reinsert tray). For slot readers,
  CD normally drops on removal, so a retry is still “pull and reinsert.”
- **Not** a new API, not a new driver, no userspace ABI.

===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================

Step 3.1: BLAME THE CHANGED LINES
Record:
- Buggy `if (val & SD_CD)` / `sdmmc_request` timeout→`sdmmc_get_cd()`
  path: **c7f6558d84afe** (“mmc: Add realtek USB sdmmc host driver”,
  2014), first in **v3.16-rc1**.
- Nearby OCP updates (`d2e6fb2c31a07`, `6fb942b85a1ab`, 2025) are
  unrelated to CD polarity; this tree has them.

Step 3.2: FOLLOW THE FIXES: TAG
Record: **No `Fixes:` tag on this commit.** Companion series patch 1/2
(`483c948324a38`) has `Fixes: bb400d2120bd` (“mfd: rtsx_usb: Defer
autosuspend while card exists”); `bb400d2120bd` **is** an ancestor of
this tree. This commit does not depend on that `Fixes:` target in its
own diff.

Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record:
- **4dad599b8b5d1** — drop always-on poll; set `MMC_CAP_NEEDS_POLL` only
  while runtime-resumed. That is why a stuck-high CD becomes a permanent
  rescan loop.
- **235b630eda072** — same author, earlier `rtsx_usb` interrupt-detect
  restore, **`Cc: stable`**, S3 delay from `mmc_rescan`. Same hardware
  family.
- **No follow-ups** on `rtsx_usb_sdmmc.c` / `rtsx_usb.c` after
  `7185b6d33b170` in this tree.
- This is **patch 2/2** of “rtsx_usb: fix tray-reader false card detect
  and autosuspend”. Companion **1/2** is `483c948324a38` and is already
  in this tree.

Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record: Sean Rhodes (Star Labs) is a regular contributor for this laptop
hardware, not the MMC maintainer. Related work: `483c948324a38` (series
1/2), `235b630eda072` (stable-bound detect/PM). Maintainer weight is
Ulf’s application + Greg’s Ack.

Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record:
- Diff is self-contained in `rtsx_usb_sdmmc.c` (`suppress_cd` only).
- Full “USB parent autosuspends with empty tray” behavior is the
  **matched pair** with 1/2; this commit alone still stops the init-
  timeout storm.
- In **this** checkout both patches are present. No missing helper/API.

===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================

Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- `b4 dig -c 7185b6d33b170`: **[PATCH v2 2/2]**
  https://patch.msgid.link/1d6f22a3777e78d6232cbe7dc3aa17ded8fc32e2.1783
  352430.git.sean@starlabs.systems
- `b4 dig -a`: v2 series “rtsx_usb: fix tray-reader false card detect
  and autosuspend” (2026-07-06). RFC (not in this mbox) open-coded MMC
  commands from `->get_cd()`; v2 dropped that.
- Thread saved to mbox (5 messages). lore.kernel.org WebFetch returned
  **403**; discussion taken from `b4 dig -m`.
- **Ulf Hansson (2026-07-10):** queued for -next for more testing;
  *“Potentially we should also tag these for stable kernels, but let's
  get it tested a bit more in next first.”* No NAK.
- **Greg KH:** “No objection from me” + Acked-by. No stable NAK.
- Applied version is v2 (latest matching patch-id). No later unapplied
  revision found.

Step 4.2: CHECK WHO REVIEWED THE PATCH
Record (`b4 dig -w`): To Ulf Hansson; Cc Greg KH, Arnd, Ricky Wu
(Realtek), Lee Jones, Roger Tseng, Dan Carpenter, Jisheng Zhang, linux-
mmc, linux-kernel. MMC maintainer picked it; Greg acked the series.

Step 4.3: SEARCH FOR THE BUG REPORT
Record: No syzbot/bugzilla. Author is a hardware vendor. Cover letter
gives a concrete repro (empty tray vs card vs S3) and three minutes of
empty-tray soak with no timeout/reset churn. Severity from reporter:
detect livelock, USB reset churn, autosuspend never happens.

Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Standalone as a CD-suppression fix; series 1/2 is the parent
autosuspend/cache change. This tree has both. No later fixup series
found.

Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: `lore.kernel.org/stable/` fetch **403 (UNVERIFIED via lore)**.
In-tree/mbox: Ulf deferred `Cc: stable` until -next soak. Commit landed
in **mmc-v7.3** (pulled to Linus for 7.3). From July -next through
**7.3-rc4** (this HEAD) there are **no follow-up fixes**. That is the
soak Ulf asked for.

===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================

Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
Record: `sdmmc_get_cd()`, `sdmmc_request()`, `rtsx_usb_init_host()`;
field `suppress_cd`.

Step 5.2: TRACE CALLERS
Record:
- `sdmmc_get_cd` ← `rtsx_usb_sdmmc_ops.get_cd` ← `mmc_rescan()` /
  `_mmc_detect_card_removed()`; also `sdmmc_request()` on command error;
  also `rtsx_usb_sdmmc_runtime_resume()`.
- `sdmmc_request` ← `mmc_host_ops.request` (MMC I/O / init commands).
- Reachable from USB card-reader use (plug, tray insert, rescan work),
  not a dead path.

Step 5.3: TRACE CALLEES
Record: `rtsx_usb_get_card_status()`, `rtsx_usb_write_register()` on OCP
clear, `mmc_request_done()`. No new allocations or locks.

Step 5.4: FOLLOW THE CALL CHAIN
Record: USB parent resumes child on apparent insert → `runtime_resume`
sets `MMC_CAP_NEEDS_POLL` and may `mmc_detect_change` → `mmc_rescan` →
`get_cd()==1` → init cmds → `sdmmc_request` timeout → without this patch
`get_cd` still 1 → rescan forever. Userspace does not need a syscall;
inserting an empty tray is enough.

Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: No other `suppress_cd` in-tree. PCI Realtek host
(`rtsx_pci_sdmmc.c`) has its own `sdmmc_get_cd`; this quirk is USB-tray-
specific. Same author’s prior `rtsx_usb` interrupt-detect restore is the
closest sibling.

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

Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record: **Yes.** File and functions exist. Pre-fix logic (trust raw
`SD_CD`) has been in the driver since v3.16. This checkout is
**7.3.0-rc4**; commit `7185b6d33b170` is already an ancestor of HEAD
(merged via `mmc-v7.3`). The bug class is old; the fix is present here.
That does **not** substitute for a technical-merit decision.

Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: **Clean apply on this tree.** Hunks match current `sdmmc_get_cd`
/ `sdmmc_request` / `rtsx_usb_init_host`. OCP fields already exist.
`WRITE_ONCE`/`READ_ONCE` available. Companion 1/2 already here.

Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: No alternate fix for empty-tray false CD. `4dad599b8b5d1` made
poll runtime-gated (necessary context, not a substitute). No later
revert.

===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================

Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: **MMC host driver** (`CONFIG_MMC_REALTEK_USB`, RTS5129/39 USB
readers) on top of `MISC_RTSX_USB`. **IMPORTANT** for affected laptops
(very common Realtek USB readers), not core mm/VFS. Peripheral relative
to the whole kernel, but not staging.

Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: Mature driver (2014) with occasional PM/detect/OCP work. Not a
newly added subsystem.

===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================

Step 8.1: DETERMINE WHO IS AFFECTED
Record: **Driver-specific / config-specific**
(`CONFIG_MMC_REALTEK_USB`). Strongest hit: tray-style RTS5129
(`0bda:0129`) as on Star Labs StarLite Mk V; other rtsx USB readers
share `get_cd`. Slot-only devices keep working as long as CD drops on
removal.

Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record: Insert empty tray (or leave tray in with no card) so `SD_CD`
stays high. Common on that hardware. Not a userspace exploit path.
Timing is deterministic, not a thin race.

Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record:
- Repeated MMC init + command timeouts
- `MMC_CAP_NEEDS_POLL` rescan every second
- USB reset churn (author, cover letter)
- USB parent stuck out of autosuspend (power)
- Related earlier bug: long S3 entry while `mmc_rescan` is frozen
- **Severity: HIGH** for affected machines (livelock of detect + USB
  churn + PM), not a panic/UAF. Not LOW: the host never settles.

Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- **Benefit:** High for tray-reader laptops; stops detect storm and
  enables autosuspend (with 1/2, already here).
- **Risk:** Low. +12/−1, local flag, no locking change beyond
  `WRITE_ONCE`. Residual risk is “timeout on first init hides a present
  card until CD drops,” which is the intended tray semantics.
- **Ratio:** Benefit > risk.

===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================

Step 9.1: COMPILE THE EVIDENCE

**FOR backporting to this tree:**
- Real hardware bug on shipping RTS5129 tray readers; vendor-tested
  (insert, empty tray soak, S3).
- Failure is a detect livelock plus USB reset churn / failed autosuspend
  — user-visible stability and power, not cosmetics.
- Hardware-quirk class (false CD), which stable explicitly takes.
- Tiny, obviously structured, no new API.
- Driver and buggy `get_cd` path have existed since v3.16; this tree has
  that code.
- Greg Ack; MMC maintainer applied; Ulf only delayed stable tagging for
  -next soak — now past -next and through 7.3-rc4 with **zero** follow-
  up fixes.
- Same author already got a related rtsx_usb detect/PM fix into stable
  (`235b630eda072`).

**AGAINST:**
- Not a KASAN/crash/CVE.
- Ulf did not `Cc: stable` at merge (wanted -next first).
- Series 2/2: full autosuspend story wants 1/2 (already in this tree).
- Possible hide-until-reinsert if a real card times out during first
  init.

**UNRESOLVED:**
- Could not query lore.kernel.org/stable (HTTP 403). Relied on `b4` mbox
  + git history instead.

Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — v2 after RFC, runtime log on
   StarLite Mk V, Acked by Greg, merged by Ulf, no regressing follow-ups
   through 7.3-rc4.
2. Fixes a real bug that affects users? **PASS** — empty-tray false CD,
   observed detect loop and USB churn.
3. Important issue? **PASS** — detect livelock / USB reset churn / PM
   stuck (HIGH), hardware quirk.
4. Small and contained? **PASS** — one file, +12/−1.
5. No new features or APIs? **PASS** — internal flag only.
6. Can apply to the local tree? **PASS** — file/functions present;
   companion 1/2 present; applies cleanly.

Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: **Hardware quirk / workaround** for broken CD on tray readers
(same bucket as USB/PCI quirks).

Step 9.4: MAKE YOUR DECISION
This tree is 7.3.0-rc4. The driver and false-CD loop exist here; the
patch is small, tested, and a real hardware workaround. Ulf’s “test in
-next first” condition has been met (no revert, no fixup). It belongs in
this tree on technical merit.

**What it solves for users of this tree:** empty-tray Realtek USB SD
readers no longer hammer MMC init and USB, so the host can settle and
autosuspend.

**Risk vs benefit:** low code risk vs high impact on that hardware.

**Concerns:** none that block this tree; 1/2 is already here; no missing
prerequisites.

Verification:
- [Phase 1] Subject/tags: suppress false CD; Acked-by Greg KH; SOB Sean
  Rhodes + Ulf Hansson; no Fixes/Reported-by/Cc:stable
- [Phase 1] Body: empty-tray `SD_CD` stuck high → init timeout →
  `get_cd` loop
- [Phase 2] Diff: +12/−1 in `rtsx_usb_sdmmc.c`; `suppress_cd`; timeout
  in `sdmmc_request` when `!mmc->card`
- [Phase 2] Confirmed `mmc_rescan` uses `ops->get_cd` and reschedules on
  `MMC_CAP_NEEDS_POLL` (`core.c` ~2284 and 2334–2336)
- [Phase 2] Confirmed `runtime_resume` sets `MMC_CAP_NEEDS_POLL` and
  calls `sdmmc_get_cd`
- [Phase 3] `git blame` on parent: CD logic from c7f6558d84afe
  (v3.16-rc1)
- [Phase 3] `git merge-base --is-ancestor 7185b6d33b170 HEAD` → yes;
  companion `483c948324a38` is parent
- [Phase 3] No follow-ups on these files after the commit
- [Phase 3] Related stable-bound detect fix: 235b630eda072
- [Phase 3] `bb400d2120bd` is in this tree (companion Fixes target)
- [Phase 4] `b4 dig -c/-a/-w/-m`: v2 series, Ulf “potentially stable
  after -next”, Greg Ack, no NAK
- [Phase 4] lore WebFetch 403; used mbox + openwall/lkml snippets
  instead
- [Phase 4] Cover letter: RTS5129 0bda:0129, empty tray →
  `runtime_status=suspended`, no timeout/reset churn
- [Phase 5] Callers of `sdmmc_get_cd`: ops.get_cd, `sdmmc_request`,
  `runtime_resume`
- [Phase 6] `make kernelversion` / `git describe HEAD`: 7.3.0-rc4 /
  v7.3-rc4-27-gf0100363d8c37
- [Phase 6] Buggy `get_cd` present; patch applies; no alternate fix
- [Phase 7] `CONFIG_MMC_REALTEK_USB` — RTS5129/39 USB SD/MMC
- [Phase 8] Failure: detect livelock + USB churn + failed autosuspend,
  severity HIGH for that hardware
- UNVERIFIED: lore.kernel.org/stable search (HTTP 403)

**YES**

 drivers/mmc/host/rtsx_usb_sdmmc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c b/drivers/mmc/host/rtsx_usb_sdmmc.c
index 84674659a84d4..bf37fab233cf0 100644
--- a/drivers/mmc/host/rtsx_usb_sdmmc.c
+++ b/drivers/mmc/host/rtsx_usb_sdmmc.c
@@ -44,6 +44,7 @@ struct rtsx_usb_sdmmc {
 	bool			double_clk;
 	bool			host_removal;
 	bool			card_exist;
+	bool			suppress_cd;
 	bool			initial_mode;
 	bool			ddr_mode;
 
@@ -774,6 +775,7 @@ static int sdmmc_get_cd(struct mmc_host *mmc)
 	struct rtsx_ucr *ucr = host->ucr;
 	int err;
 	u16 val;
+	bool cd;
 
 	if (host->host_removal)
 		return -ENOMEDIUM;
@@ -791,8 +793,14 @@ static int sdmmc_get_cd(struct mmc_host *mmc)
 
 	/* get OCP status */
 	host->ocp_stat = (val >> 4) & 0x03;
+	cd = val & SD_CD;
 
-	if (val & SD_CD) {
+	if (!cd) {
+		WRITE_ONCE(host->suppress_cd, false);
+		goto no_card;
+	}
+
+	if (!READ_ONCE(host->suppress_cd)) {
 		host->card_exist = true;
 		return 1;
 	}
@@ -874,6 +882,8 @@ static void sdmmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		 * detect card when fail to update card existence state and
 		 * speed up card removal when retry
 		 */
+		if (!mmc->card && cmd->error == -ETIMEDOUT)
+			WRITE_ONCE(host->suppress_cd, true);
 		sdmmc_get_cd(mmc);
 		dev_dbg(sdmmc_dev(host), "cmd->error = %d\n", cmd->error);
 	}
@@ -1359,6 +1369,7 @@ static void rtsx_usb_init_host(struct rtsx_usb_sdmmc *host)
 
 	host->power_mode = MMC_POWER_OFF;
 	host->ocp_stat = 0;
+	host->suppress_cd = false;
 }
 
 static int rtsx_usb_sdmmc_drv_probe(struct platform_device *pdev)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.18-6.12] mmc: sdhci-of-ma35d1: add missing MODULE_DEVICE_TABLE()
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
                   ` (3 preceding siblings ...)
  2026-10-19 13:48 ` [PATCH AUTOSEL 6.18] mmc: rtsx_usb_sdmmc: suppress false CD after init timeout Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
  2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] mmc: block: reject invalid perdev_minors before division Sasha Levin
  5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
  To: patches, stable
  Cc: Pengpeng Hou, Ulf Hansson, Sasha Levin, adrian.hunter, ychuang3,
	schung, linux-mmc, linux-arm-kernel, linux-kernel

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

[ Upstream commit 9b3732cce41d177acac731e0b8024d72f0dfde38 ]

The driver has an OF match table wired to .of_match_table, but does
not export the table with MODULE_DEVICE_TABLE().

Add the missing MODULE_DEVICE_TABLE(of, ...) entry so module alias
information is generated for OF based module autoloading.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable backport analysis: `mmc: sdhci-of-ma35d1: add missing
MODULE_DEVICE_TABLE()`

Local tree: **v7.3-rc4** (`7.3.0-rc4`, `VERSION=7 PATCHLEVEL=3
SUBLEVEL=0 EXTRAVERSION=-rc4`).
Candidate: `9b3732cce41d177acac731e0b8024d72f0dfde38`.
The driver and the missing-alias bug exist in this tree; the one-line
fix is already present here as a mainline commit.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[mmc / sdhci-of-ma35d1] [add] [export OF match table via
MODULE_DEVICE_TABLE so the module can autoload]**

**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected)
- **Reported-by / Tested-by / Reviewed-by / Acked-by:** none
- **Link:** none in the committed message
- **Cc: stable:** none (expected)
- **Signed-off-by:** Pengpeng Hou (author); Ulf Hansson (MMC maintainer,
  applied it)
- Ignore any pipeline SOBs

Notable: maintainer SOB; no user report; no syzbot.

**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** OF match table is wired to `.of_match_table` but not exported
  with `MODULE_DEVICE_TABLE(of, ...)`.
- **Symptom:** `depmod`/`modinfo` get no `of:` aliases, so udev/kmod
  will not autoload `sdhci-ma35` when DT has `compatible =
  "nuvoton,ma35d1-sdhci"`. Built-in (`=y`) matching still works.
- **Version info:** none in the message. Driver introduced in v6.12-rc1.
- **Root cause:** missing `MODULE_DEVICE_TABLE` so `file2alias.c` never
  publishes the table.

Original mailing-list body (stripped before commit) said this is a
source-level fix with no hardware reproduction; evidence is the table
itself.

**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** It is an explicit autoload/alias fix. Same class
as “add a device ID”: the ID exists; userspace was never told.

---

## PHASE 2: DIFF ANALYSIS

**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/mmc/host/sdhci-of-ma35d1.c` — **+1 / −0**
- Function: none; one statement after `sdhci_ma35_dt_ids[]`
- Scope: **single-file, one-line surgical fix**

**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `sdhci_ma35_dt_ids` is only used as `.of_match_table` (in-
  kernel match when the module is already loaded or built-in).
- **After:** same, plus `MODULE_DEVICE_TABLE(of, sdhci_ma35_dt_ids)` so
  modpost emits `MODULE_ALIAS("of:N*T*Cnuvoton,ma35d1-sdhci")`.
- Path: **module build / autoload**, not probe/error/init runtime logic.

**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Hardware enablement / missing module alias]** — category
**(h) plus logic for autoload**. Not UAF, leak, race, or crash. Without
the macro, a modular build never gets OF aliases. Confirmed by
`include/linux/module.h`: `MODULE_DEVICE_TABLE` creates a `used` alias
so `scripts/mod/file2alias.c` can find the table.

**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct; this is the documented way to export
  OF IDs.
- **Regression risk:** negligible. Macro only adds a static alias
  symbol; no locking, no API, no runtime path change except autoload
  starts working.
- **Red flags:** none on the diff. Author is doing a wide
  `MODULE_DEVICE_TABLE` sweep (rtc, vdpa, gpio, iio, …); that does not
  make *this* line wrong. MMC maintainer applied it after trimming the
  disclaimer.

---

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Table and driver: `addc9ecb9ddb3` (“Add Nuvoton MA35D1 SDHCI driver”),
  Shan-Chun Hung, 2024-07-16, committed 2024-08-26.
- First appeared: **v6.12-rc1** (`git describe --contains addc9ecb9ddb3`
  → `v6.12-rc1~158^2~42`).
- Original driver had the match table and `.of_match_table` and **no**
  `MODULE_DEVICE_TABLE` from day one.
- Bug age: ~v6.12 through this July 2026 fix.

**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Buggy commit is the original driver add,
verified by blame and by `git show addc9ecb9ddb3` (tail of the file has
the table without the macro). That commit **is** an ancestor of this
tree.

**Step 3.3: CHECK FILE HISTORY**
Record (this file only):
- `addc9ecb9ddb3` — add driver
- `078e548af9c3a` — `remove_new` → `remove`
- `01bc623f9a5c2` — drop `sdhci_pltfm_free()`
- `995832b2cebe6` — include-header churn
- `9b3732cce41d1` — this fix

No prerequisite refactor for this one line. **Standalone.**

**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Pengpeng Hou is not the MMC maintainer; many similar “add
missing MODULE_DEVICE_TABLE()” commits across subsystems. Ulf Hansson
applied this one. Related MMC precedent by others: `6e540da4c1db7`
(“mmc: sdhci-of-aspeed: fix module autoloading”) is the same one-line
change **and had `Fixes:` + `Cc: stable@vger.kernel.org`**.

**Step 3.5: DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Only needs `sdhci_ma35_dt_ids` and `module.h` (already
included). Can apply standalone on any tree that has this driver.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 9b3732cce41d1` →
  https://patch.msgid.link/20260704124115.2874-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: **v1 only** (2026-07-04). Applied version is that v1
  (commit message cleaned).
- Thread (mbox): Ulf Hansson, 2026-07-06: **“Applied for next (by
  cleanup of the commit message a bit), thanks!”**
- No NAK, no reviewer-requested `Cc: stable`, no risk discussion.
- lore.kernel.org HTTP fetch returned 403 (Anubis); thread recovered via
  `b4 dig -m`.

**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included Ulf Hansson (MMC), Adrian Hunter
(SDHCI), Nuvoton (Jacky Huang, Shan-Chun Hung), `linux-mmc`, `linux-arm-
kernel`. Right people. Only maintainer reply was “applied”.

**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No user/syzbot/bugzilla report.** Author stated it was
inferred from source. The failure mode is still deterministic for `=m`
builds.

**Step 4.4: RELATED PATCHES AND SERIES**
Record: **Standalone 1/1.** Same-week author activity is a tree-wide
missing-alias sweep, not a multi-patch MMC series.

**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** via lore.kernel.org/stable (Anubis 403). No `Cc:
stable` on this patch. Closest analogue in-tree (`sdhci-of-aspeed`
autoload) *was* explicitly nominated for stable.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: KEY FUNCTIONS**
Record: **No function body changed.** Touches `sdhci_ma35_dt_ids` and
the `sdhci_ma35_driver` registration site.

**Step 5.2: TRACE CALLERS**
Record:
- `.probe = ma35_probe` — driver-core platform probe.
- `.of_match_table = sdhci_ma35_dt_ids` — OF matching when the driver is
  present.
- `MODULE_DEVICE_TABLE` is not a runtime caller; it feeds **modpost**.
- Impact surface: modular `CONFIG_MMC_SDHCI_OF_MA35D1=m` on
  `nuvoton,ma35d1-sdhci` DT.

**Step 5.3: TRACE CALLEES**
Record: Unchanged. `ma35_probe` still does `sdhci_pltfm_init` / host
setup. Macro expands to a static alias (verified in
`include/linux/module.h`).

**Step 5.4: FOLLOW THE CALL CHAIN**
Record: DT node `compatible = "nuvoton,ma35d1-sdhci"` → kmod alias
lookup → load `sdhci-ma35` → `ma35_probe`. Reachable on MA35D1 boot if
the driver is a module. Not a syscall/UAF path. **Userspace-visible:**
missing MMC/SD host until manual `modprobe`.

**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Other `sdhci-of-*.c` drivers already have
`MODULE_DEVICE_TABLE(of, ...)`. Historical MMC autoload fixes: aspeed,
am654, at91, mediatek. This driver was the outlier.

Kconfig: `MMC_SDHCI_OF_MA35D1` is **tristate** (`depends on ARCH_MA35 ||
COMPILE_TEST`), so `=m` is a supported config. Help text: “say Y or M
here.”

---

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

**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** File exists; table and `.of_match_table` exist; the
missing alias was the original 6.12 code. This checkout already contains
the fix line at `drivers/mmc/host/sdhci-of-ma35d1.c:293`. `git merge-
base --is-ancestor addc9ecb9ddb3 HEAD` and `9b3732cce41d1 HEAD` both
succeed. Bug introduced **v6.12**, not after this tree.

**Step 6.2: BACKPORT COMPLICATIONS**
Record: **Clean apply.** One added line next to a table that has been
there since the driver was added. Later churn (`remove`,
`sdhci_pltfm_free`, includes) does not conflict with this hunk.

**Step 6.3: RELATED FIXES ALREADY HERE**
Record: This *is* the fix. No earlier equivalent. Sibling OF SDHCI
drivers already export their tables.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **MMC host / SDHCI platform driver.** Criticality:
**PERIPHERAL** (Nuvoton MA35D1 only), but it is the SD/MMC host for that
SoC — root-on-SD/eMMC is a realistic layout.

**Step 7.2: SUBSYSTEM ACTIVITY**
Record: `drivers/mmc/host/` is active (many recent fixes). This
particular file is low-churn (five commits total).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: WHO IS AFFECTED**
Record: **Platform-specific / config-specific:** `ARCH_MA35` +
`CONFIG_MMC_SDHCI_OF_MA35D1=m`. Built-in (`=y`) is unaffected. Distro-
style modular kernels are the users who benefit.

**Step 8.2: TRIGGER CONDITIONS**
Record: Every boot/enumeration of `nuvoton,ma35d1-sdhci` when the driver
is a module. Not a race. Unprivileged users do not trigger it;
**boot/udev does**. Common *on that hardware* if distros ship the driver
as `=m`.

**Step 8.3: FAILURE MODE SEVERITY**
Record: **Missing driver autoload → SD/MMC host never probes → no
card/root disk until manual load.** Not oops/UAF. Severity **MEDIUM**,
**HIGH** if root is on that host and the module is not in the initramfs
by name. Same user-visible class as a missing device ID.

**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium for MA35D1 modular builds (storage may not
  appear).
- **Risk:** **very low** (1 line, alias-only).
- **Ratio:** favorable. Same pattern already taken for stable on other
  SDHCI OF drivers.

---

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: EVIDENCE**

**For backporting:**
- Real, deterministic autoload bug when `MMC_SDHCI_OF_MA35D1=m`.
- One-line, obviously correct, no new API.
- Driver has been in this lineage since v6.12; table was never exported.
- MMC maintainer applied it; SDHCI/Nuvoton were on Cc; no NAK.
- Same change as `mmc: sdhci-of-aspeed: fix module autoloading`, which
  carried `Cc: stable`.
- Matches the stable-rules “device ID” / hardware-enablement exception
  in spirit: publish an ID the driver already matches.
- Applies cleanly; no prerequisites.

**Against:**
- No `Reported-by` / no hardware reproduction (author said so).
- Peripheral SoC, not a core path.
- Not a crash, UAF, or corruption.
- No `Cc: stable` on this specific patch (Ulf took it for *next*, not as
  an urgent fix).
- Author is running a tree-wide alias sweep (quality of *this* hunk is
  still fine).

**Unresolved:**
- Whether linux-stable already discussed this (lore.kernel.org/stable
  blocked).
- How many shipping MA35D1 kernels use `=m` vs `=y` (unverified).

**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — standard macro; maintainer
   applied. “Tested” is source-level, not lab hardware.
2. Fixes a real bug that affects users? **PASS** — modular autoload is
   broken; Kconfig allows `=m`.
3. Important issue? **PASS** (as device-ID / hardware-enablement, not as
   a crash). Storage host may never appear on boot.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS** — only module aliases for an
   existing compatible string.
6. Can apply to the local tree? **PASS** — driver present; hunk is the
   existing table site.

**Step 9.3: EXCEPTION CATEGORIES**
Record: **Device ID / module alias publication for an existing driver**
(same practical class as adding PCI/USB/OF IDs). Not a new driver.

**Step 9.4: DECISION**
This tree has the MA35D1 SDHCI driver and had the missing alias from
introduction until this commit. The patch is a one-line, zero-runtime-
risk fix that makes the already-supported compatible string autoload
when the driver is a module. That is stable-appropriate hardware
enablement, not a feature or refactor.

---

## What the commit solves for stable users

On Nuvoton MA35D1, `sdhci-of-ma35d1` already matches
`nuvoton,ma35d1-sdhci` **if loaded**. Without `MODULE_DEVICE_TABLE`, a
modular kernel never generates the OF alias, so the MMC/SD controller is
silent until someone runs `modprobe`. Backporting restores autoload.
Risk is essentially zero.

---

## Verification
- [Phase 1] Parsed commit `9b3732cce41d1`: no Fixes/Reported-
  by/Cc:stable; SOBs Pengpeng Hou + Ulf Hansson
- [Phase 2] Diff: +1 line `MODULE_DEVICE_TABLE(of, sdhci_ma35_dt_ids)`
  in `sdhci-of-ma35d1.c`
- [Phase 2] `MODULE_DEVICE_TABLE` expansion verified in
  `include/linux/module.h:255-257`
- [Phase 3] `git blame`: table from `addc9ecb9ddb3` (v6.12-rc1), no
  MODULE_DEVICE_TABLE until this commit
- [Phase 3] `git show addc9ecb9ddb3`: original driver had
  `.of_match_table` only
- [Phase 3] `git merge-base --is-ancestor addc9ecb9ddb3 HEAD` and
  `9b3732cce41d1 HEAD`: both true
- [Phase 3] File log: 5 commits; no extra prereqs for this line
- [Phase 3] Precedent `6e540da4c1db7` (aspeed autoload): same one-liner,
  had `Cc: stable`
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1-only; lore msgid
  `20260704124115.2874-1-pengpeng@iscas.ac.cn`; Ulf “Applied for next”
- [Phase 4] WebFetch lore/patch.msgid.link: 403; thread read from mbox
  instead
- [Phase 5] Kconfig: `MMC_SDHCI_OF_MA35D1` is tristate, `depends on
  ARCH_MA35 || COMPILE_TEST`
- [Phase 5] Other `sdhci-of-*.c` already have `MODULE_DEVICE_TABLE`
- [Phase 6] `git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`; `make
  kernelversion` = `7.3.0-rc4`
- [Phase 6] Current file line 293 already has the macro
- [Phase 8] Failure mode: module not autoloaded; no oops
- UNVERIFIED: linux-stable list discussion (Anubis); how common `=m` is
  on MA35D1 distro kernels

**YES**

 drivers/mmc/host/sdhci-of-ma35d1.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-of-ma35d1.c b/drivers/mmc/host/sdhci-of-ma35d1.c
index 287026422616c..73f772db04148 100644
--- a/drivers/mmc/host/sdhci-of-ma35d1.c
+++ b/drivers/mmc/host/sdhci-of-ma35d1.c
@@ -291,6 +291,7 @@ static const struct of_device_id sdhci_ma35_dt_ids[] = {
 	{ .compatible = "nuvoton,ma35d1-sdhci" },
 	{}
 };
+MODULE_DEVICE_TABLE(of, sdhci_ma35_dt_ids);
 
 static struct platform_driver sdhci_ma35_driver = {
 	.driver	= {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] mmc: block: reject invalid perdev_minors before division
       [not found] <20261019135441.1420968-1-sashal@kernel.org>
                   ` (4 preceding siblings ...)
  2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] mmc: sdhci-of-ma35d1: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
  5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
  To: patches, stable; +Cc: Xu Rao, Ulf Hansson, Sasha Levin, linux-mmc, linux-kernel

From: Xu Rao <raoxu@uniontech.com>

[ Upstream commit 5253aee00a4383cb0482d5e99bde48b8054ae2c0 ]

The mmcblk.perdev_minors module parameter is parsed as a signed int and
is used during mmc_blk_init() to compute the number of supported block
devices.

Passing perdev_minors=0 makes the init path divide by zero when it
computes max_devices. Negative values are invalid as well and would make
max_devices negative before it is later used as an IDA limit.

Reject non-positive perdev_minors values before registering any mmcblk
resources.

Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

I have the bug mechanism, history, and review thread. Compiling the
nine-phase verdict for this 7.3-rc4 tree.

## Phase 1: Commit message forensics

**Step 1.1: Subject line**
- Record: **[mmc/block] [reject] reject invalid `perdev_minors` before
  division**
- Subsystem: `mmc: block:`. Verb: **reject** (guard / prevent invalid
  input). Intent: do not use a non-positive module parameter as a
  divisor.

**Step 1.2: Tags**
- Record:
  - `Signed-off-by: Xu Rao <raoxu@uniontech.com>` (author)
  - `Signed-off-by: Ulf Hansson <ulfh@kernel.org>` (MMC maintainer;
    committed 2026-07-10)
  - **No** `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`,
    `Acked-by:`, `Link:`, or `Cc: stable@vger.kernel.org`
  - Absence of `Fixes:` / `Cc: stable` is expected for this review and
    is not a negative signal
- SHA in this tree: `5253aee00a4383cb0482d5e99bde48b8054ae2c0`

**Step 1.3: Body**
- Record:
  - **Bug:** `mmcblk.perdev_minors` is a signed `int` module parameter.
    `mmc_blk_init()` computes `max_devices` as `(1 << MINORBITS) /
    perdev_minors`.
  - **Symptoms:** `perdev_minors=0` → divide-by-zero in init. Negative
    values → negative `max_devices`, later used as an IDA upper bound.
  - **Version info:** none in the message.
  - **Root cause:** no validation before division / IDA use. Fix: reject
    `<= 0` before registering any mmcblk resources.

**Step 1.4: Hidden bug fix?**
- Record: **No disguise.** This is an explicit crash/UB fix, not
  cleanup-as-fix.

## Phase 2: Diff analysis

**Step 2.1: Inventory**
- Record:
  - File: `drivers/mmc/core/block.c` — **+5 / −0**
  - Function: `mmc_blk_init()`
  - Scope: **single-file surgical fix**

**Step 2.2: Code flow**
- Record: **Before:** `mmc_blk_init()` registers the RPMB bus and chrdev
  region, then does `max_devices = min(MAX_DEVICES, (1 << MINORBITS) /
  perdev_minors)` with no check. **After:** if `perdev_minors <= 0`, log
  and `return -EINVAL` **before** any of those registrations (no leak on
  the new error path).

Verified parent of the fix (no check):

```
max_devices = min(MAX_DEVICES, (1 << MINORBITS) / perdev_minors);
```

**Step 2.3: Bug mechanism**
- Record: **Logic / correctness + memory-safety (div-by-zero, invalid
  IDA limit)**
  - `perdev_minors == 0`: integer division by zero in `mmc_blk_init()`
    (oops / exception on init).
  - `perdev_minors < 0`: signed division yields a negative
    `max_devices`; `ida_alloc_max(..., max_devices - 1, ...)` takes
    `unsigned int max`, so the negative value **wraps** to a huge limit
    (`include/linux/idr.h`). Probe then uses `devidx * perdev_minors`
    for `first_minor`.
  - Same divisor is also used in `mmc_get_devidx()` (`disk->first_minor
    / perdev_minors`).

**Step 2.4: Fix quality**
- Record: **Obviously correct and minimal.** Matches Kconfig
  (`MMC_BLOCK_MINORS` range 4–256). Valid defaults (8) unchanged.
  Regression risk **very low**: only rejects values that already crash
  or mis-size the IDA. Early return happens before `bus_register`, so no
  new leak.

## Phase 3: Git history

**Step 3.1: Blame**
- Record:
  - `perdev_minors` module param: `5e71b7a64cb4c` (“mmc: make number of
    mmcblk minors configurable”, Olof Johansson, 2010) —
    **v2.6.37-rc1**.
  - Division `(1 << MINORBITS) / perdev_minors`: `a26eba614afff0` (“mmc:
    block: Increase max_devices”, Ben Hutchings, 2014) — **v3.19-rc1**.
  - Bug is over a decade old in this lineage, not new 7.x code.

**Step 3.2: Fixes: tag**
- Record: **No `Fixes:` tag.** Introducing commits above are in this
  tree (ancient ancestors of HEAD).

**Step 3.3: File history**
- Record: Recent `block.c` work is unrelated (RPMB unregister, MQ
  retry). `git log --grep=perdev_minors -- drivers/mmc/` shows only this
  fix plus old param/prefix commits. **Standalone, not a series.** No
  follow-ups on `block.c` after `5253aee`.

**Step 3.4: Author**
- Record: Xu Rao has one other recent MMC commit (`mmc: spi: reset
  bytes_xfered...`). Not the subsystem maintainer. **Ulf Hansson** (MMC
  maintainer) applied it.

**Step 3.5: Dependencies**
- Record: **None.** The check uses existing `perdev_minors` and
  `pr_err`. Applies on its own.

## Phase 4: Mailing list / external

**Step 4.1: Original discussion**
- Record: `b4 dig -c 5253aee00a438` matched patch-id
  `291b9622c960b2f250c74c590d7e9916013372a3`.
  - Lore/msgid: `https://patch.msgid.link/C7A17066496D0ABA+2026070609250
    1.853376-1-raoxu@uniontech.com`
  - **v1 only** (`b4 dig -a`); committed version is that v1.
  - Ulf Hansson (10 Jul 2026): **“Applied for next, thanks!”** — no NAK,
    no requested respin.
  - Direct lore.kernel.org fetch returned **403**; thread confirmed via
    b4 + LKML/openwall mirrors.

**Step 4.2: Recipients (`b4 dig -w`)**
- Record: `ulfh@kernel.org`, `linux-mmc@vger.kernel.org`, Avri Altman,
  Bean Huo, Shawn Lin, Jens Wiklander, **Kees Cook**, plus others.
  Maintainer was To: and merged the patch. No reviewer `Reviewed-by` in
  git.

**Step 4.3: Bug report**
- Record: **No `Reported-by` / syzbot / bugzilla.** Author describes the
  mechanism; no public crash log found.

**Step 4.4: Series**
- Record: **Standalone single patch**, not N/M.

**Step 4.5: Stable list**
- Record: Web search of `lore.kernel.org/stable` for `perdev_minors` did
  **not** turn up a nomination or a rejection of this patch. Direct lore
  fetch was 403. **UNVERIFIED** that nobody ever asked stable@; no
  evidence of a known NAK for stable either.

## Phase 5: Semantic / call graph

**Step 5.1: Functions**
- Record: **`mmc_blk_init` only** (plus it protects later use of
  `perdev_minors` in `mmc_get_devidx`, `mmc_blk_alloc_req`,
  `mmc_blk_alloc_rpmb_part`).

**Step 5.2: Callers**
- Record: `module_init(mmc_blk_init)` only — boot (builtin,
  `CONFIG_MMC_BLOCK=y` default) or `modprobe mmcblk`. Not a hot path,
  not IRQ, not a syscall except via privileged module load / kernel
  cmdline.

**Step 5.3: Callees**
- Record: After the new check: `bus_register`, `alloc_chrdev_region`,
  `register_blkdev`, `mmc_register_driver`. The new path calls none of
  these.

**Step 5.4: Reachability**
- Record: **Yes, from admin/boot configuration.**
  `module_param(perdev_minors, int, 0444)` — set at load
  (`mmcblk.perdev_minors=0`) or on the kernel command line if builtin.
  **Not** writable later (0444). **Unprivileged users cannot set it** on
  a normal system. Kconfig `range 4 256` cannot produce 0; only the
  module/boot param can.

**Step 5.5: Similar patterns**
- Record: `perdev_minors` exists only in this file. No sibling mmc
  driver with the same hole. Pattern is the usual “signed module_param
  used as divisor without a positivity check.”

## Phase 6: This checkout

**Step 6.1: Buggy code in this tree?**
- Record:
  - `git describe HEAD` → **`v7.3-rc4-27-gf0100363d8c37`**
  - `make kernelversion` → **7.3.0-rc4**
  - The mmc block driver and the division **are present**. The
    **unfixed** sequence is the parent of `5253aee`; **HEAD already
    contains the fix** (`git merge-base --is-ancestor 5253aee00a438
    HEAD` → yes).
  - Commit first landed via MMC for **v7.3** (`git describe 5253aee` →
    `v7.2-rc2-25-g5253aee00a438`; **not** an ancestor of `v7.2`). Bug
    **introduction** is v3.19-era, long before this branch.
  - Being an ancestor of this mainline/rc HEAD is expected for a
    candidate; it is not “already selected for stable.”

**Step 6.2: Backport difficulty**
- Record: **Clean apply on this tree** (hunk matches `mmc_blk_init`
  exactly). No extra helpers required.

**Step 6.3: Related fixes already here**
- Record: This **is** the fix. No alternate/earlier validation of
  `perdev_minors`. No follow-up revert.

## Phase 7: Subsystem context

**Step 7.1:**
- Record: **`drivers/mmc/core` (MMC block)** — **IMPORTANT** (eMMC/SD
  block devices; `CONFIG_MMC_BLOCK` defaults to `y`). Not core mm/VFS,
  not staging.

**Step 7.2:**
- Record: File is actively maintained (RPMB, MQ recovery, etc.). The
  **bug site** (`max_devices = … / perdev_minors`) has been stable since
  2014.

## Phase 8: Impact and risk

**Step 8.1: Who is affected**
- Record: Users of `mmcblk` who pass **`mmcblk.perdev_minors=0` or a
  negative value** (cmdline or `modprobe`). Default Kconfig path is
  fine. Population: **config/admin-specific**, not universal.

**Step 8.2: Trigger**
- Record: One-shot at `mmc_blk_init`. **Not** timing-dependent. **Not**
  triggerable by an unprivileged syscall on a typical system.
  Commonness: **low** (must override a param whose Kconfig range is
  already 4–256).

**Step 8.3: Failure mode**
- Record:
  - `= 0`: divide-by-zero → **oops / exception during MMC block init**.
    Builtin + `panic_on_oops` → boot failure; otherwise mmcblk fails and
    eMMC/SD never appear (root-on-eMMC can fail to boot).
  - `< 0`: negative `max_devices` → **unsigned wrap in
    `ida_alloc_max`**, wrong minors, possible further corruption of
    device numbering.
  - Severity: **HIGH** if triggered (crash / failed storage), **MEDIUM
    overall** because the trigger is an invalid privileged parameter.

**Step 8.4: Risk–benefit**
- Record: **Benefit: high for a tiny hardening/crash fix** (prevents
  init oops; clean `-EINVAL`). **Risk: very low** (5 lines, error path
  only, no API/lock/struct change). Ratio: **favorable**.

## Phase 9: Synthesis

**Step 9.1: Evidence**

**For backporting**
- Real divide-by-zero in `mmc_blk_init` when `perdev_minors=0`.
- Negative values produce a negative `max_devices` and wrap in
  `ida_alloc_max()`.
- 5-line, obviously correct check before any resource registration.
- Bug in code since ~v3.19; param since v2.6.37; MMC block present in
  this 7.3-rc4 tree.
- No dependencies; standalone; maintainer applied it with no objections.
- `CONFIG_MMC_BLOCK` is default y — driver is widely built.

**Against**
- No `Reported-by` / syzbot; no evidence of production hits.
- Trigger is privileged (boot arg / `modprobe`), param is 0444.
- No `Reviewed-by`; v1 merged as-is.
- Not a hot-path race or unprivileged UAF.

**Unresolved**
- lore.kernel.org HTTP 403; used b4 + LKML mirrors.
- No hit on stable@ for this subject (search quality limited).

**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — trivial positivity check;
   maintainer merged to MMC next. No extra Tested-by, but the change is
   reviewable by inspection.
2. Fixes a real bug that affects users? **PASS** — div-by-zero / invalid
   IDA limit if the param is set wrong.
3. Important (crash/security/corruption/deadlock)? **PASS** — kernel
   oops on init; storage devices never register. Not unprivileged
   security.
4. Small and contained? **PASS** — +5 lines, one function.
5. No new features/APIs? **PASS**.
6. Can apply to this tree? **PASS** — same file/function; no missing
   prereqs.

**Step 9.3: Exception category**
- Record: **None** (not a device ID/quirk/DT/docs-only change). It is a
  normal small bugfix.

**Step 9.4: Decision**

This tree is **7.3.0-rc4**. The MMC block driver and the unvalidated
division exist here; the patch is self-contained and matches local
`mmc_blk_init()`. A 5-line reject of non-positive `perdev_minors`
prevents a real init-time divide-by-zero and a broken IDA limit, with
negligible regression risk. That meets stable-kernel-rules for this
checkout.

### Why it matters here
Invalid `mmcblk.perdev_minors` is enough to oops while bringing up
mmcblk (or to wrap the IDA max). On systems with builtin mmcblk (the
default), that is boot-time. The fix fails init with `-EINVAL` instead.

### Risk vs benefit
Benefit is avoiding a crash/failed MMC probe; risk is a check that does
not run for any Kconfig-legal default.

### Concerns
No in-the-wild report; trigger needs a bad privileged param. That lowers
urgency but does not make the patch unsafe or feature-like. No backport
rework needed on this tree.

---

**Verification:**
- [Phase 1] Parsed message/tags: author + Ulf SOB only; no
  Fixes/Reported-by/Cc stable. SHA `5253aee00a438`.
- [Phase 2] Diff: +5 in `mmc_blk_init()`; `git show
  5253aee^:drivers/mmc/core/block.c` confirmed unvalidated `(1 <<
  MINORBITS) / perdev_minors`.
- [Phase 2] `ida_alloc_max()` in `include/linux/idr.h` takes `unsigned
  int max` — negative `max_devices - 1` wraps (verified).
- [Phase 2] `MINORBITS` is 20 in `include/linux/kdev_t.h` (verified).
- [Phase 2] Kconfig `MMC_BLOCK_MINORS` `range 4 256`, `default 8`;
  `CONFIG_MMC_BLOCK` `default y` (verified).
- [Phase 3] `git blame`: param `5e71b7a64cb4c` (v2.6.37-rc1); division
  `a26eba614afff0` (v3.19-rc1).
- [Phase 3] `git log --grep=perdev_minors -- drivers/mmc/`: this commit
  + old param commits only; no series/follow-up.
- [Phase 3] `git log --author='Xu Rao' -- drivers/mmc/`: this + one
  unrelated spi fix.
- [Phase 4] `b4 dig -c 5253aee00a438`: msgid
  `C7A17066496D0ABA+20260706092501.853376-1-raoxu@uniontech.com`; v1
  only; Ulf CC’d.
- [Phase 4] LKML: Ulf “Applied for next, thanks!” — no NAK.
- [Phase 4] lore.kernel.org WebFetch: **403**. Stable@ search: **no
  relevant thread found**.
- [Phase 5] `mmc_blk_init` only via `module_init`; `module_param(...,
  0444)` — admin/boot only (verified in `block.c`).
- [Phase 5] Call sites of `perdev_minors`: init division,
  `mmc_get_devidx`, `md->disk->minors` / `first_minor` (grep verified).
- [Phase 6] `git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`; `make
  kernelversion` = `7.3.0-rc4`.
- [Phase 6] `git merge-base --is-ancestor 5253aee00a438 HEAD` = yes;
  `git merge-base --is-ancestor 5253aee00a438 v7.2` = not in v7.2.
- [Phase 6] HEAD already has the `if (perdev_minors <= 0)` block at
  `block.c:3340` (this is the mainline commit under review, not a
  stable-queue marker).
- [Phase 8] Failure mode: div-by-zero oops at mmcblk init, or IDA limit
  wrap for negatives.
- UNVERIFIED: full lore.kernel.org thread HTML (403); whether stable@
  ever discussed this hash.

**YES**

 drivers/mmc/core/block.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index e8e45b161a398..5077842b286fe 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -3407,6 +3407,11 @@ static int __init mmc_blk_init(void)
 {
 	int res;
 
+	if (perdev_minors <= 0) {
+		pr_err("mmcblk: invalid minors per device: %d\n", perdev_minors);
+		return -EINVAL;
+	}
+
 	res  = bus_register(&mmc_rpmb_bus_type);
 	if (res < 0) {
 		pr_err("mmcblk: could not register RPMB bus type\n");
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-19 14:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:44 ` [PATCH AUTOSEL 6.18-5.10] mmc: cqhci: fix to missed endian conversions Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] mmc: dw_mmc: stop and complete DMA also in STATE_DATA_BUSY state Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.12] mmc: moxart: report DMA completion timeout Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18] mmc: rtsx_usb_sdmmc: suppress false CD after init timeout Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] mmc: sdhci-of-ma35d1: add missing MODULE_DEVICE_TABLE() Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] mmc: block: reject invalid perdev_minors before division Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox