* [PATCH AUTOSEL 6.18] spi: qpic-snand: free OOB buffer with ECC context
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm-qspi: return error from failed controller suspend Sasha Levin
` (7 subsequent siblings)
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Mark Brown, Sasha Levin, linux-spi, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 5504e50ffbcf97b3b6ed3740532497922c96a1f1 ]
qcom_spi_ecc_init_ctx_pipelined() allocates qspi->oob_buf as part of the
pipelined ECC context setup. The buffer is then used by the raw and ECC
page I/O paths through the NAND ECC context.
The matching cleanup callback only frees the qpic_ecc structure, and the
init error path also skips the OOB buffer after it has been allocated.
Free qspi->oob_buf on both paths and clear the pointer so the ECC context
teardown owns all resources allocated during init.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260616151129.76060-1-pengpeng@iscas.ac.cn
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
Record: `[spi/qpic-snand] [free] free OOB buffer with ECC context`
**Step 1.2 — Tags**
Record:
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Link:
https://patch.msgid.link/20260616151129.76060-1-pengpeng@iscas.ac.cn`
- `Signed-off-by: Mark Brown <broonie@kernel.org>` (SPI maintainer)
- Pipeline-only tags ignored: `[Upstream commit 5504e50ffbcf…]`,
`Signed-off-by: Sasha Levin`
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Cc:
stable` (absence expected)
**Step 1.3 — Body**
Record: `qcom_spi_ecc_init_ctx_pipelined()` allocates `qspi->oob_buf`.
Cleanup only frees `qpic_ecc`. Init error path after allocation also
skips the OOB buffer. Symptom: leaked kmalloc buffer on ECC teardown and
on init failure after the buffer exists. Root cause: allocation is not
owned by the ECC cleanup/error path.
**Step 1.4 — Hidden bug fix?**
Record: Not disguised. Explicit resource-leak fix (`free` / missing
`kfree`).
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
Record: `drivers/spi/spi-qpic-snand.c` only; **+5 / −1**. Functions:
`qcom_spi_ecc_init_ctx_pipelined`, `qcom_spi_ecc_cleanup_ctx_pipelined`.
Single-file surgical fix. Trailing newline at EOF removed (harmless).
**Step 2.2 — Flow**
Record:
- **err_free_ecc_cfg:** previously `kfree(ecc_cfg)` only → also
`kfree(oob_buf)` and NULL it. Covers failures after `kmalloc` (notably
`qcom_alloc_bam_transaction()` `-ENOMEM`) and is a no-op if `oob_buf`
is still NULL.
- **cleanup_ctx:** previously only `kfree(ecc_cfg)` → get `snandc` via
`nand_to_qcom_snand()`, free `oob_buf`, NULL it, then free `ecc_cfg`.
**Step 2.3 — Bug mechanism**
Record: **resource leak**. `oob_buf` is `kmalloc(mtd->writesize +
mtd->oobsize)` (typically a few KB). `qspi` itself is `devm_kzalloc`, so
the buffer is **not** device-managed. `nanddev_find_ecc_configuration()`
does **not** call `cleanup_ctx` when `init_ctx` fails, so the error-path
`kfree` is required. Successful attach then `spinand_cleanup()` / probe-
error after ECC init also leak without the cleanup change.
**Step 2.4 — Fix quality**
Record: Obviously correct. `kfree(NULL)` is safe because `qspi` is
zeroed in probe (`devm_kzalloc`). NULLing after free avoids a later
double-free if cleanup runs after the error path. Regression risk is
very low: no locking, API, or I/O-path change. `QPIC_QSPI_NUM_CS` is 1,
so no multi-chip sharing of `oob_buf`.
## Phase 3: Git history
**Step 3.1 — Blame**
Record: Driver added in `7304d1909080` (2025-02-24, Md Sadre Alam):
allocated `oob_buf` in init and never freed it in cleanup.
`d450cdd9c4398` (2025-03) only freed `ecc_cfg` when OOB alloc failed.
`65cb56d49f6ede` (in v6.18.53) unified `err_free_ecc_cfg` still only
`kfree(ecc_cfg)`. Bug present since the driver landed.
**Step 3.2 — Fixes: tag**
Record: None. Introducer is `7304d1909080`; `git merge-base --is-
ancestor 7304d1909080 v6.18.53` → yes. The earlier leak fix
`d450cdd9c4398` is also in v6.18.53 and is incomplete for `oob_buf`.
**Step 3.3 — File history**
Record: Related: `d450cdd9c4398` (ecc_cfg leak), `f48d805035042`
(kzalloc→kmalloc; still no free). Standalone; not `N/M`. Later autosel
commits (quad mode, ECC strength) are not prerequisites.
**Step 3.4 — Author**
Record: Pengpeng Hou has this one SPI qpic commit here; not the original
driver author. SPI maintainer Mark Brown applied it to `spi/for-7.3`.
**Step 3.5 — Dependencies**
Record: Needs `nand_to_qcom_snand()`, `qspi->oob_buf`,
`err_free_ecc_cfg`, and `qcom_spi_ecc_cleanup_ctx_pipelined()`. All
exist in v6.18.53. Standalone.
## Phase 4: Mailing list
**Step 4.1**
Record: `b4 dig -c 5504e50ffbcf` →
https://patch.msgid.link/20260616151129.76060-1-pengpeng@iscas.ac.cn
`b4 dig -a`: **v1 only**. Thread: author submission + Mark Brown
“Applied to … for-7.3”. No NAK, no requested respin.
**Step 4.2**
Record: `b4 dig -w`: To Mark Brown; Cc linux-spi, linux-kernel.
Maintainer applied it.
**Step 4.3**
Record: No bugzilla/syzbot. Leak found by code inspection.
**Step 4.4**
Record: Single-patch series, self-contained.
**Step 4.5**
Record: **UNVERIFIED** — lore.kernel.org/stable blocked by Anubis (403).
No local evidence this was rejected for stable.
## Phase 5: Semantic analysis
**Step 5.1**
Record: `qcom_spi_ecc_init_ctx_pipelined`,
`qcom_spi_ecc_cleanup_ctx_pipelined`.
**Step 5.2**
Record: Wired as `.init_ctx` / `.cleanup_ctx` on
`qcom_spi_ecc_engine_ops_pipelined`. Call chain: `nand_ecc_init_ctx` /
`nand_ecc_cleanup_ctx` → `nanddev_ecc_engine_init` /
`nanddev_ecc_engine_cleanup` → `spinand_init` error paths and
`spinand_cleanup` from `spinand_probe` failure and `spinand_remove`.
**Step 5.3**
Record: Init: `kzalloc(ecc_cfg)`, `kmalloc(oob_buf)`,
`qcom_free_bam_transaction` / `qcom_alloc_bam_transaction`. Cleanup:
`kfree` only.
**Step 5.4**
Record: Reachable on Qualcomm IPQ9574 SPI-NAND attach/detach (boot
device on many boards). Unprivileged userspace cannot typically unbind
without privilege; leak still hits rmmod, unbind, and failed probe after
ECC init.
**Step 5.5**
Record: Same driver’s earlier `ecc_cfg` leak (`d450cdd9c4398`) used the
same pattern. `data_buf` is an op pointer, not a matching leak. MTK SNFI
cleanup only frees its ecc cfg (different allocation model).
## Phase 6: This tree (v6.18.53 / 6.18.y)
**Step 6.1**
Record: `git describe HEAD` → `v6.18.53-5049-g0f0bdc2b0dffb`. `make
kernelversion` → 6.18.53.
`git merge-base --is-ancestor f320c3a8b1119 v6.18.53` → **NO** (not in
baseline).
v6.18.53 still has:
```
err_free_ecc_cfg:
kfree(ecc_cfg);
return ret;
…
static void qcom_spi_ecc_cleanup_ctx_pipelined(...)
{
struct qpic_ecc *ecc_cfg = nand_to_ecc_ctx(nand);
kfree(ecc_cfg);
}
```
and still allocates `snandc->qspi->oob_buf`. Driver is in v6.18
(`7304d1909080` is an ancestor of `v6.18`).
**Step 6.2**
Record: `patch --dry-run` of this diff onto v6.18.53’s file: **clean
apply**. Context around `err_free_ecc_cfg` / cleanup matches.
**Step 6.3**
Record: Incomplete leak fix `d450cdd9c4398` is already here; it does not
free `oob_buf`. No other equivalent fix in v6.18.53.
## Phase 7: Subsystem
**Step 7.1**
Record: `drivers/spi` QPIC SPI-NAND (`CONFIG_SPI_QPIC_SNAND`,
`ARCH_QCOM`). Compatible `qcom,ipq9574-snand`. **PERIPHERAL** but real
embedded/router hardware; NAND is often boot/rootfs.
**Step 7.2**
Record: Active in 6.16–6.18 (ECC validation, BAM txn, 8-bit ECC,
unregister-on-remove). Leak is original, not a new-code regression.
## Phase 8: Impact and risk
**Step 8.1**
Record: Qualcomm IPQ9574 QPIC SPI-NAND users only.
**Step 8.2**
Record: Triggers: (1) `init_ctx` failure after `kmalloc` (BAM realloc
`-ENOMEM`) — cleanup not called; (2) later `spinand_init` failure; (3)
`spinand_remove` / unbind / rmmod. Not a hot-path unbounded leak. Not
unprivileged.
**Step 8.3**
Record: Failure mode: leaked page-sized+OOB kmalloc. Severity **MEDIUM**
(not oops/corruption; real leak on teardown and some probe errors).
**Step 8.4**
Record: Benefit: medium for this hardware (correct lifetime of ECC-init
allocations; matches prior leak fix in the same function). Risk: **very
low** (5 lines, `kfree`+NULL). Ratio favors backport.
## Phase 9: Synthesis
**For:** Real `kmalloc` leak in this 6.18 tree; error path cannot rely
on cleanup; 5-line obviously correct fix; applies cleanly; no new API;
maintainer accepted v1; same class as `d450cdd9c4398` already in 6.18.
**Against:** Not a crash/security issue; no reporter; driver is
Qualcomm-specific; leak is small and not a hot-path OOM.
**Unresolved:** Stable-list discussion (lore blocked).
**Stable rules:**
1. Obviously correct and tested? **PASS** — `kfree`/`NULL`; maintainer
applied v1.
2. Real user-visible bug? **PASS** — leak on probe-error and remove for
IPQ9574 SPI-NAND.
3. Important? **PASS** — resource leak on teardown/error; same pattern
routinely taken for stable (not a crash; still a real leak of non-
devm memory).
4. Small/contained? **PASS** — 5 lines, one file.
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS** — clean on v6.18.53.
**Exception category:** none (ordinary leak fix).
This tree is **6.18.y**. The buggy allocation and missing free are
present. The patch is a minimal, correct leak fix with negligible
regression risk.
### What it solves for stable users
On IPQ9574 QPIC SPI-NAND, ECC init allocates an OOB scratch buffer that
is never freed. Failed attach after that allocation, and every
unbind/rmmod, leaks that buffer. 6.18 already had a partial leak fix for
`ecc_cfg` only.
### Risk vs benefit
Benefit is a real leak fix for in-tree hardware. Risk is a handful of
`kfree`s on error/teardown.
---
Verification:
- [Phase 1] Subject/tags: free OOB buffer; SOB Hou + Brown; Link msgid;
no Fixes/Reported-by
- [Phase 2] Diff: +5/−1 in `spi-qpic-snand.c`; `kfree(oob_buf)` + NULL
on `err_free_ecc_cfg` and cleanup
- [Phase 2] v6.18.53 still lacks those `kfree`s;
`kmalloc(writesize+oobsize)` present
- [Phase 2] `qspi = devm_kzalloc` at probe → `oob_buf` starts NULL;
`kfree(NULL)` safe
- [Phase 3] Introducer `7304d1909080` is ancestor of v6.18 and v6.18.53
- [Phase 3] `d450cdd9c4398` in v6.18.53 frees only `ecc_cfg` on OOB
alloc failure
- [Phase 3] `65cb56d49f6ede` in v6.18.53; `err_free_ecc_cfg` still only
frees `ecc_cfg`
- [Phase 3] Candidate `f320c3a8b1119` is **not** an ancestor of v6.18.53
- [Phase 4] `b4 dig`: lore thread, v1 only; Brown applied to
spi/for-7.3; no NAK
- [Phase 4] WebFetch lore/msgid 403; curl lore/stable hit Anubis —
UNVERIFIED stable-list history
- [Phase 5] Callers: `nand_ecc_cleanup_ctx` →
`nanddev_ecc_engine_cleanup` → `spinand_cleanup` (probe err + remove)
- [Phase 5] `nanddev_find_ecc_configuration()` returns on `init_ctx`
failure **without** `cleanup_ctx`
- [Phase 5] `QPIC_QSPI_NUM_CS` is 1
- [Phase 6] `git describe HEAD` = v6.18.53-5049-g0f0bdc2b0dffb;
kernelversion 6.18.53
- [Phase 6] `patch --dry-run` onto v6.18.53 file: success
- [Phase 6] `nand_to_qcom_snand()` exists in v6.18.53
- [Phase 7] `CONFIG_SPI_QPIC_SNAND` depends on `ARCH_QCOM`; OF
`qcom,ipq9574-snand`
- [Phase 8] Failure: kmalloc leak, MEDIUM; not oops/corruption
- UNVERIFIED: whether linux-stable ever discussed this patch (lore
blocked)
**YES**
drivers/spi/spi-qpic-snand.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
index 222e4aa7ee488..919e847c061db 100644
--- a/drivers/spi/spi-qpic-snand.c
+++ b/drivers/spi/spi-qpic-snand.c
@@ -394,14 +394,19 @@ static int qcom_spi_ecc_init_ctx_pipelined(struct nand_device *nand)
return 0;
err_free_ecc_cfg:
+ kfree(snandc->qspi->oob_buf);
+ snandc->qspi->oob_buf = NULL;
kfree(ecc_cfg);
return ret;
}
static void qcom_spi_ecc_cleanup_ctx_pipelined(struct nand_device *nand)
{
+ struct qcom_nand_controller *snandc = nand_to_qcom_snand(nand);
struct qpic_ecc *ecc_cfg = nand_to_ecc_ctx(nand);
+ kfree(snandc->qspi->oob_buf);
+ snandc->qspi->oob_buf = NULL;
kfree(ecc_cfg);
}
@@ -1660,4 +1665,3 @@ module_platform_driver(qcom_spi_driver);
MODULE_DESCRIPTION("SPI driver for QPIC QSPI cores");
MODULE_AUTHOR("Md Sadre Alam <quic_mdalam@quicinc.com>");
MODULE_LICENSE("GPL");
-
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] spi: bcm-qspi: return error from failed controller suspend
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18] spi: qpic-snand: free OOB buffer with ECC context Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] spi: nxp-fspi: disable runtime PM on probe failures Sasha Levin
` (6 subsequent siblings)
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Jiawen Liu, Kamal Dasu, Mark Brown, Sasha Levin, linux-spi,
linux-kernel
From: Jiawen Liu <1298662399@qq.com>
[ Upstream commit 44d09ef0ba690ac46d17eb05c12a47a5dd75e231 ]
spi_controller_suspend() can fail if the SPI core cannot stop the
controller. bcm_qspi_suspend() ignored that error, disabled the
controller clock, uninitialized the hardware, and returned success.
Return the suspend error before tearing down the clock and hardware
state.
Signed-off-by: Jiawen Liu <1298662399@qq.com>
Reviewed-by: Kamal Dasu <kamal.dasu@broadcom.com>
Link: https://patch.msgid.link/tencent_21BF5F9512F56D45FD9018BAF14ED2805808@qq.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.18.53** (`autosel`). The buggy ignore-and-teardown
path is in that base, and this is a small PM error-path fix that belongs
there.
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[spi / bcm-qspi] [return] [propagate
`spi_controller_suspend()` failure instead of tearing down
clocks/hardware and reporting success]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Fixes:` — **absent** (expected for autosel review; not a negative)
- `Reported-by:` — **absent**
- `Tested-by:` — **absent**
- `Reviewed-by: Kamal Dasu <kamal.dasu@broadcom.com>` — **driver
maintainer** (MAINTAINERS: “BROADCOM SPI DRIVER”)
- `Acked-by:` — none on this commit; sibling patch 2/3 was Acked-by
William Zhang (Broadcom)
- `Link: https://patch.msgid.link/tencent_21BF5F9512F56D45FD9018BAF14ED2
805808@qq.com`
- `Cc: stable@` — **absent** (expected)
- `Signed-off-by: Jiawen Liu <1298662399@qq.com>` (author)
- `Signed-off-by: Mark Brown <broonie@kernel.org>` (SPI maintainer)
- Pipeline tags (`[Upstream commit 44d09ef0ba69…]`, Sasha Levin SOB)
**ignored** per instructions
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `spi_controller_suspend()` can fail when the SPI core cannot
stop the controller. `bcm_qspi_suspend()` discarded that return value.
- **Symptom:** On that failure the driver still
`clk_disable_unprepare()`s, calls `bcm_qspi_hw_uninit()`, and
**returns 0**, so PM continues as if suspend succeeded.
- **Root cause (author):** ignored error, then hardware teardown.
- **Version info in message:** none.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** It is an explicit error-propagation fix: keep
clocks/HW up and fail device suspend if the controller cannot be
stopped.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/spi/spi-bcm-qspi.c` — **+5 / −1**
- Function: `bcm_qspi_suspend()`
- Scope: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: **Before:** always call `spi_controller_suspend()`, then disable
the clock, uninit HW, return 0. **After:** if `spi_controller_suspend()`
returns non-zero, return that error **before** clock disable /
`bcm_qspi_hw_uninit()`. Normal success path unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Logic / PM error-path]**
`spi_controller_suspend()` → for queued controllers, `spi_stop_queue()`.
That polls up to 500 × ~10 ms and returns **`-EBUSY`** if the queue is
still non-empty or `ctlr->busy`. This driver sets `host->transfer_one =
bcm_qspi_transfer_one`, so `spi_register_controller()` calls
`spi_controller_initialize_queue()` and sets `ctlr->queued = true`.
Failure is therefore reachable.
Old code then:
1. Disables the controller clock
2. `bcm_qspi_hw_uninit()` writes MSPI registers (clears `MSPI_SPCR2`,
write lock, status)
3. Reports success so system suspend proceeds
That can interrupt an in-flight QSPI transfer (this hardware is used as
SPI NOR on Broadcom STB / iProc).
**Step 2.4: ASSESS THE FIX QUALITY**
Record: **Obviously correct** and matches the standard pattern already
used by many SPI drivers (`spi-rockchip`, `spi-qup`, `spi-dw-core`,
`spi-bcm63xx`, etc.). **Regression risk is low:** 5 lines, error path
only. Residual SPI-core quirk: `spi_controller_suspend()` still calls
`__spi_mark_suspended()` even on `spi_stop_queue()` failure; that is
pre-existing core behavior, not introduced here, and is the same
contract other drivers already rely on.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Ignore-and-teardown dates to **`fa236a7ef24048`** (“spi: bcm-qspi: Add
Broadcom MSPI driver”, Kamal Dasu, 2016) which called
`spi_master_suspend()` and always returned 0.
- Rename to `spi_controller_suspend(qspi->host)`: **`ec271c04ae93b8`**
(2023), no functional change.
- Clock path: **`1b7ad8c405c3dc`** (2020) switched to
`clk_disable_unprepare()`.
- Bug has been in the driver since introduction (**v4.9-era**), not new
6.18 code.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No `Fixes:` tag.** Buggy behavior is original suspend
implementation, not a later regression.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Standalone among bcm-qspi changes. Same author submitted a
3-patch series of the same pattern:
- `spi: bcm63xx: return error from failed controller suspend`
- `spi: bcm63xx-hsspi: …`
- **this patch (3/3)**
- later sibling: `spi: bcmbca-hsspi: …`
Each driver is independent; this patch does not need the others.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Jiawen Liu is a regular SPI error-handling contributor, not this
driver’s maintainer. Weight comes from **Reviewed-by Kamal Dasu**
(listed maintainer) and **SOB Mark Brown**.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Uses `qspi->host` and `spi_controller_suspend()`, both
in v6.18.53. Standalone.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 44d09ef0ba69` → **https://patch.msgid.link/tencent_21BF5F95
12F56D45FD9018BAF14ED2805808@qq.com**
- Subject: `[PATCH 3/3] spi: bcm-qspi: return error from failed
controller suspend`
- `b4 dig -a`: **v1 only** (no v2/v3)
- Thread: Kamal Dasu **Reviewed-by**; no NAK; no request for further
changes
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To Mark Brown (`broonie@`); Cc `linux-spi`, Kamal
Dasu, `bcm-kernel-feedback-list`, Jonas Gorski, Kursad Oney, William
Zhang. Right maintainers were on the patch.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No bugzilla/syzbot/user crash report.** Bug is derived from
code review of ignored return value.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: 3/3 series; each patch is a per-driver copy of the same check.
This commit is self-contained.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** — `lore.kernel.org/stable/` HTTP 403 from this
environment. SPI list thread (via `b4 dig -m`) had no stable nomination
and no objection.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `bcm_qspi_suspend` (modified). Callees of interest:
`spi_controller_suspend`, `clk_disable_unprepare`, `bcm_qspi_hw_uninit`.
**Step 5.2: TRACE CALLERS**
Record: Wired via `bcm_qspi_pm_ops` (`SIMPLE_DEV_PM_OPS` /
`DEFINE_SIMPLE_DEV_PM_OPS`), exported and used by:
- `drivers/spi/spi-brcmstb-qspi.c` (`brcm,spi-brcmstb-qspi` / `brcm,spi-
brcmstb-mspi`)
- `drivers/spi/spi-iproc-qspi.c`
Called from the PM core on system suspend, not from a syscall directly.
**Step 5.3: TRACE CALLEES**
Record: `spi_controller_suspend` → `spi_stop_queue` (queued) → `-EBUSY`
after ~5s; then `__spi_mark_suspended`. `bcm_qspi_hw_uninit` performs
MMIO to stop MSPI.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: **Userspace-reachable indirectly:** suspend-to-idle/S3 with in-
flight SPI (SPI-NOR, userspace spidev, etc.). Not a syscall crash path;
a **system-suspend** path. If QSPI backs system flash, interrupting a
write/erase is a **data-corruption** risk.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same ignore-return bug was fixed in sibling Broadcom SPI drivers
in the same series. Vast majority of in-tree SPI drivers already check
`spi_controller_suspend()`’s return.
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `git describe HEAD` → `v6.18.53-5049-g0f0bdc2b0dffb`;
`make kernelversion` → **6.18.53**.
`v6.18.53:drivers/spi/spi-bcm-qspi.c` still has:
```c
spi_controller_suspend(qspi->host);
clk_disable_unprepare(qspi->clk);
bcm_qspi_hw_uninit(qspi);
return 0;
```
Original driver commit `fa236a7ef24048` **is** an ancestor of
`v6.18.53`.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** to this 6.18.y file: the pre-image hunk is byte-
identical in `v6.18.53`. No `master` vs `host` rename needed (already
`qspi->host`).
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This exact fix is **not** in `v6.18.53` (`git log v6.18.53
--grep="return error from failed controller suspend" -- drivers/spi/` is
empty). No alternate fix in the file.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **[drivers/spi, Broadcom QSPI]** **[IMPORTANT for that hardware,
PERIPHERAL globally]**. Not core mm/net, but this controller commonly
backs SPI-NOR on Broadcom STB and iProc boards.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature driver (2016), still maintained (Kamal Dasu). SPI core
still actively developed. This is a long-standing hole, not churn from a
new subsystem.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific / platform-specific:** Broadcom STB and iProc
QSPI users who suspend. Config: `CONFIG_SPI_BCM_QSPI` and SoC glue
drivers.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: System suspend while the SPI message queue cannot drain within
~5s (`-EBUSY`). Unprivileged users cannot trigger it except insofar as
they can generate SPI traffic and then suspend (typically privileged).
**Not common** on a healthy idle bus; **realistic** if a transfer is
stuck or messages keep arriving during freeze.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Failure mode:** clock off + MMIO uninit while the controller
is still busy, then suspend proceeds. Outcomes: hung MMIO, incomplete
SPI NOR program/erase, **possible flash corruption**, later resume with
HW already torn down. **Severity: HIGH** (data corruption / crash on a
real suspend error path). Not a panic on every boot.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit: medium–high** for affected platforms (correct abort of
suspend; no teardown while busy).
- **Risk: very low** (5 lines, error path, maintainer-reviewed, same
pattern as many drivers).
- **Ratio: favorable.**
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real ignored-error bug: teardown + success after
`spi_controller_suspend()` fails
- Driver is queued (`transfer_one`), so `-EBUSY` is a real return
- Teardown while busy can corrupt SPI-NOR or touch unclocked MMIO
- 5-line, obviously correct, matches subsystem convention
- Reviewed by the listed Broadcom SPI maintainer; applied by Mark Brown
- Buggy code present since 2016; present in **this** 6.18.53 tree
- Applies cleanly; no prerequisites
**AGAINST:**
- No user `Reported-by`, syzbot, or `Fixes:`
- Trigger is the uncommon queue-stop timeout, not every suspend
- Affects only Broadcom QSPI platforms
- No explicit `Cc: stable` (not a negative under these rules)
**UNRESOLVED:**
- Could not query `lore.kernel.org/stable/` (HTTP 403)
- No public reproducer beyond the code path
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — trivial check; maintainer
Reviewed-by; in mainline via Mark Brown
2. Fixes a real bug that affects users? **PASS** — ignored PM error then
unsafe HW teardown
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
HIGH: possible SPI-NOR corruption / MMIO after clock gate if queue
stop fails
4. Small and contained? **PASS** — +5/−1, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — hunk matches `v6.18.53`
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Not an exception category** (not a new
ID/quirk/DT/build/docs). It is a normal small bugfix.
**Step 9.4: MAKE YOUR DECISION**
This should be backported to **this 6.18.y tree**. The bug exists here,
the fix is surgical, and failing to stop the SPI queue must not be
reported as a successful suspend.
---
### What the commit solves for stable users
On Broadcom QSPI, suspend used to disable the clock and uninit MSPI even
when the SPI core could not stop the queue, then return success. That
lets suspend continue with the controller still busy. The patch fails
the device suspend instead, which is what every modern SPI driver
already does.
### Stable rules and risk
Meets all six criteria: real PM bug, small, no new API, clean apply on
6.18.53. Risk is far below benefit.
### Concerns
No user reports; trigger is an error path. Sibling series patches are
independent and not required. Residual `__spi_mark_suspended()` on
failure is a pre-existing SPI-core issue, not a reason to skip this
driver fix.
### Verification
- **[Phase 1]** Parsed tags: Reviewed-by Kamal Dasu; Link to msgid; no
Fixes/Reported-by/Cc: stable
- **[Phase 2]** Diff: +5/−1 in `bcm_qspi_suspend()`; check
`spi_controller_suspend()` before `clk_disable_unprepare` /
`bcm_qspi_hw_uninit`
- **[Phase 2]** `spi_stop_queue()` in `drivers/spi/spi.c` returns
`-EBUSY` after 500 polls
- **[Phase 2]** `host->transfer_one` set in `bcm_qspi_probe`;
`spi_controller_initialize_queue()` sets `queued = true`
- **[Phase 3]** Blame: ignore-return since `fa236a7ef24048` (2016);
rename `ec271c04ae93b8` (2023)
- **[Phase 3]** `git merge-base --is-ancestor fa236a7ef24048 v6.18.53` —
yes
- **[Phase 3]** Series is 3 independent per-driver patches; this one is
standalone
- **[Phase 4]** `b4 dig -c 44d09ef0ba69` → lore msgid above; v1 only;
Kamal Reviewed-by; no NAK
- **[Phase 4]** `b4 dig -w`: Mark Brown + linux-spi + Broadcom list
- **[Phase 4]** UNVERIFIED: stable@ lore search (403)
- **[Phase 5]** Callers: `spi-brcmstb-qspi.c`, `spi-iproc-qspi.c` via
`bcm_qspi_pm_ops`
- **[Phase 6]** `git describe HEAD`: `v6.18.53-5049-g0f0bdc2b0dffb`;
`make kernelversion`: 6.18.53
- **[Phase 6]** `git show v6.18.53:drivers/spi/spi-bcm-qspi.c` still has
the buggy sequence; pre-image matches
- **[Phase 6]** `git log v6.18.53 --grep="return error from failed
controller suspend" -- drivers/spi/` empty
- **[Phase 8]** Failure mode: suspend proceeds after clock-off/HW uninit
while queue busy → HIGH (corruption/crash risk)
**YES**
drivers/spi/spi-bcm-qspi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index cfdaa5eaec76d..ae5545be0a45c 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -1694,13 +1694,17 @@ EXPORT_SYMBOL_GPL(bcm_qspi_remove);
static int __maybe_unused bcm_qspi_suspend(struct device *dev)
{
struct bcm_qspi *qspi = dev_get_drvdata(dev);
+ int ret;
/* store the override strap value */
if (!bcm_qspi_bspi_ver_three(qspi))
qspi->s3_strap_override_ctrl =
bcm_qspi_read(qspi, BSPI, BSPI_STRAP_OVERRIDE_CTRL);
- spi_controller_suspend(qspi->host);
+ ret = spi_controller_suspend(qspi->host);
+ if (ret)
+ return ret;
+
clk_disable_unprepare(qspi->clk);
bcm_qspi_hw_uninit(qspi);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-6.12] spi: nxp-fspi: disable runtime PM on probe failures
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18] spi: qpic-snand: free OOB buffer with ECC context Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm-qspi: return error from failed controller suspend Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx: return error from failed controller suspend Sasha Levin
` (5 subsequent siblings)
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Jiawen Liu, Mark Brown, Sasha Levin, han.xu, haibo.chen,
linux-spi, imx, linux-kernel
From: Jiawen Liu <1298662399@qq.com>
[ Upstream commit 5d14285d60ba95561609a293064654a0c7d7d928 ]
nxp_fspi_probe() enables runtime PM and autosuspend before
several operations that can fail.
Some failure paths returned directly before the devm cleanup
action was installed, leaving runtime PM enabled.
Route those failures through a common runtime PM cleanup path.
Use pm_runtime_resume_and_get() for the initial clock enable.
Signed-off-by: Jiawen Liu <1298662399@qq.com>
Link: https://patch.msgid.link/tencent_8FC0B8DFAF4AE67AEBA20548045D53A77707@qq.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough to decide: the leak is in this 6.18 tree, the fix is small
and self-contained, and it does not depend on later rework.
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: **[spi: nxp-fspi]** **[disable]** disable runtime PM on probe
failures. Action is a cleanup of leftover runtime PM after
`nxp_fspi_probe()` fails.
**Step 1.2: Tags**
Record:
- `Signed-off-by: Jiawen Liu <1298662399@qq.com>` (author)
- `Link: https://patch.msgid.link/tencent_8FC0B8DFAF4AE67AEBA20548045D53
A77707@qq.com`
- `Signed-off-by: Mark Brown <broonie@kernel.org>` (SPI maintainer
applied it)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence is expected)
- Pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit
5d14285d60ba...]` ignored as candidate-prep markers
**Step 1.3: Body**
Record:
- **Bug:** `nxp_fspi_probe()` calls `pm_runtime_enable()` + autosuspend,
then can fail before `devm_add_action_or_reset(..., nxp_fspi_cleanup,
...)`.
- **Symptom:** runtime PM left enabled after a failed probe; no
`nxp_fspi_cleanup()` to disable it.
- **Root cause:** early `return` on clock get/put, IRQ request, and
mutex init.
- **Fix:** common `err_disable_pm` path; switch initial get to
`pm_runtime_resume_and_get()`.
- **Version info in message:** none.
**Step 1.4: Hidden bug fix?**
Record: Not hidden. This is an explicit probe error-path resource-leak
fix.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record:
- File: `drivers/spi/spi-nxp-fspi.c` (+22 / −9)
- Function: `nxp_fspi_probe()` only
- Scope: single-file surgical error-path fix
**Step 2.2: Code flow**
Record (verified against parent of `03a9171de3f13`):
Before:
1. `pm_runtime_enable` / `use_autosuspend`
2. `pm_runtime_get_sync()` — on error, **return** (RPM stays enabled;
usage count stays incremented because `get_sync` increments even on
failure)
3. `pm_runtime_put_sync()` — on error, **return** (RPM stays enabled)
4. `devm_request_irq()` — on error, **return**
5. `devm_mutex_init()` — on error, **return**
6. Only later `devm_add_action_or_reset(..., nxp_fspi_cleanup)` disables
RPM
After: those four failures `goto err_disable_pm`, which does
`pm_runtime_dont_use_autosuspend()` + `pm_runtime_disable()`. Initial
get uses `pm_runtime_resume_and_get()` so a failed resume does **not**
leave the usage count up.
`devm_add_action_or_reset` failure still returns directly: on add
failure that helper already runs `nxp_fspi_cleanup()`, which disables
RPM. That path is correct as-is.
**Step 2.3: Bug mechanism**
Record: **Error-path resource leak** (runtime PM left enabled; usage-
count leak on failed `get_sync`). Also a small **correctness** change:
`get_sync` → `resume_and_get`, matching the kernel header guidance that
`get_sync` leaves the counter incremented on error.
**Step 2.4: Fix quality**
Record: Obviously correct and minimal. Regression risk is low: only
probe-failure paths plus the get API swap. Remaining gaps (unchecked
`nxp_fspi_default_setup()`, unchecked get in `nxp_fspi_cleanup()`) are
pre-existing and not introduced here.
## Phase 3: Git history
**Step 3.1: Blame**
Record: The buggy `pm_runtime_enable` / `get_sync` / early returns come
from `97be4b919a609` ("spi: spi-nxp-fspi: enable runtime pm for fspi",
Haibo Chen, 2025-04-28). Present in v6.16 and in this tree. Cleanup
action is `9f7cd1bcb6363` ("use devm instead of remove"), also in this
tree.
**Step 3.2: Fixes: tag**
Record: No `Fixes:` tag. Introducer is `97be4b919a609` (verified via
blame). `git merge-base --is-ancestor 97be4b919a609 v6.18` succeeds.
**Step 3.3: File history**
Record: Recent work is SoC clock/DTR features, not RPM probe cleanup. No
later commit on this file after the candidate. Standalone vs this file.
**Step 3.4: Author**
Record: Jiawen Liu submitted a 4-patch SPI PM error-path series
(atcspi200, bcmbca-hsspi, fsl-dspi, nxp-fspi). Not the driver
maintainer; NXP people (Han Xu, Haibo Chen) maintain this driver. Mark
Brown applied the series.
**Step 3.5: Dependencies**
Record: Cover letter says the four patches are independent. This hunk
only needs `pm_runtime_enable`/`use_autosuspend` (present) and
`pm_runtime_resume_and_get()` (present in `include/linux/pm_runtime.h`).
Can apply standalone. Applies cleanly to this tree’s probe function.
## Phase 4: Mailing list
**Step 4.1: Original discussion**
Record:
- `b4 dig -c 5d14285d60ba95561609a293064654a0c7d7d928` → https://patch.m
sgid.link/tencent_8FC0B8DFAF4AE67AEBA20548045D53A77707@qq.com
- Series: `[PATCH 0/4] spi: fix PM error handling in several drivers`
(2026-06-20), **v1 only** for this commit
- Cover letter: patches are independent; local error paths only
- Frank Li: style comment only — use `PM_RUNTIME_ACQUIRE` instead of
`goto` (not a NAK of the leak fix)
- Mark Brown applied the series to `for-7.3` as-is, including `[4/4]` as
`5d14285d60ba`
- sashiko-bot: this patch still leaves clocks on if `put_sync` fails;
also flagged pre-existing `default_setup` / cleanup get issues. Those
did **not** block merge.
**Step 4.2: Recipients (`b4 dig -w`)**
Record: Mark Brown, linux-spi, Han Xu, Haibo Chen, Yogesh Gaur,
imx@lists.linux.dev, linux-arm-kernel. Right maintainers were on Cc.
**Step 4.3: Bug report**
Record: No user/syzbot report. Found by code review of probe error
paths.
**Step 4.4: Related series**
Record: Later `[PATCH v2 0/3] spi: nxp-fspi: fix runtime PM error
handling` (2026-07-28) *reworks* this fix with
`devm_pm_runtime_enable()` / `PM_RUNTIME_ACQUIRE_AUTOSUSPEND()`, plus
`default_setup` and cleanup checks. That follow-up is **not** in this
tree (`rg` found no `devm_pm_runtime_enable` in the driver). Haibo Chen
rejected `PM_RUNTIME_ACQUIRE` in cleanup. The later series is extra
hardening/style, not a revert of this leak fix.
**Step 4.5: Stable list**
Record: lore.kernel.org search returned 403; no stable-thread evidence
obtained. UNVERIFIED whether anyone nominated this on stable@
separately.
## Phase 5: Semantic analysis
**Step 5.1: Functions**
Record: `nxp_fspi_probe()` only.
**Step 5.2: Callers**
Record: Platform `.probe = nxp_fspi_probe` in this file. Runs at device
bind / deferred probe, not from a syscall hot path.
**Step 5.3: Callees**
Record: `pm_runtime_resume_and_get`, `pm_runtime_put_sync`,
`devm_request_irq`, `devm_mutex_init`,
`pm_runtime_dont_use_autosuspend`, `pm_runtime_disable`. Runtime resume
enables clocks via `nxp_fspi_clk_prep_enable()`.
**Step 5.4: Reachability**
Record: Yes, from device probe on `CONFIG_SPI_NXP_FLEXSPI`
(i.MX8/i.MX9/Layerscape FlexSPI). Trigger is a **failed** probe after
RPM enable (clock resume fail, put fail, IRQ request fail, mutex init
fail). Not unprivileged userspace on a working board.
**Step 5.5: Similar patterns**
Record: Same author fixed other SPI PM error paths. `spi-rockchip.c`
already has `err_disable_pm_runtime`. This is a known driver pattern.
## Phase 6: Local tree (this checkout)
**Step 6.1: Does the bug exist here?**
Record:
- Tree: `v6.18.53-5049-g0f0bdc2b0dffb`, `Makefile` 6.18.53, branch
`autosel`
- Buggy sequence exists in `03a9171de3f13^` (`pm_runtime_get_sync` +
early returns after `pm_runtime_enable`)
- Introduced by `97be4b919a609`, ancestor of v6.18
- Driver file and RPM enablement are present
**Step 6.2: Backport difficulty**
Record: **Clean apply**. Context matches this tree (`device_set_node`
and DTR caps are already here). `pm_runtime_resume_and_get()` exists.
**Step 6.3: Related fixes already here?**
Record: No other probe RPM-disable fix. Later v2 rework is not present.
`nxp_fspi_cleanup()` still disables RPM only after successful
`devm_add_action_or_reset`.
## Phase 7: Subsystem context
**Step 7.1:** `drivers/spi/` — NXP FlexSPI (`CONFIG_SPI_NXP_FLEXSPI`).
Criticality: **IMPORTANT** for i.MX/Layerscape boards (often the
boot/SPI-NOR controller), **PERIPHERAL** for everyone else.
**Step 7.2:** Actively maintained (clock/DTR/DLL work through
2025–2026). RPM itself is from 6.16-era, so the leak has been in 6.18
for the life of this driver feature.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
Record: Users of NXP FlexSPI (`nxp,imx8mm-fspi`, `imx8mp`, `imx8ulp`,
`imx93`, `imx95`, `lx2160a-fspi`, etc.) when probe fails after RPM is
enabled.
**Step 8.2: Trigger**
Record: Failed probe after `pm_runtime_enable()`. Not every boot. Clock
resume failure is the important case: old `get_sync` leaves `usage_count
> 0` and RPM enabled, so a later bind hits `pm_runtime_enable()` on an
already-enabled device (WARN). IRQ/mutex failures after a successful put
leave RPM enabled on an unbound device.
Cannot be triggered by an unprivileged user on a healthy probe path.
**Step 8.3: Failure mode**
Record:
- Runtime PM enabled after failed bind
- Failed `get_sync`: usage-count leak, device stuck “active”
- Re-probe: `pm_runtime_enable()` WARN
- Autosuspend still armed if put succeeded before a later failure
Severity: **MEDIUM** (probe error path, not a success-path
crash/corruption). Still a real leak/WARN class that stable regularly
takes.
**Step 8.4: Risk-benefit**
Record:
- **Benefit:** Medium for i.MX/Layerscape; stops leftover RPM state and
usage-count leak on failed probe.
- **Risk:** Low (~20 lines, error path only, recommended PM helper).
- Ratio supports backport.
## Phase 9: Synthesis
**Step 9.1: Evidence**
FOR:
- Real probe error-path leak: RPM enabled before `nxp_fspi_cleanup` is
installed
- `pm_runtime_get_sync()` on failure leaks the usage count;
`resume_and_get` + disable is the right fix
- Small, obviously correct, no new API
- Bug present since RPM was added (`97be4b919a609`), which is in this
6.18 tree
- SPI maintainer applied it; cover letter says the series is independent
- Driver is used on common NXP boards
AGAINST:
- No crash/syzbot/user report; probe-failure only
- Incomplete vs later review (ignored `nxp_fspi_default_setup()`,
cleanup get)
- Frank Li preferred helpers over `goto` (style; not a functional NAK)
UNRESOLVED:
- Could not fetch lore.kernel.org (403); discussion recovered via `b4
dig -m` and lkml.iu.edu
- No stable@ thread found
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — maintainer applied; logic
matches PM docs
2. Real user-visible bug? **PASS** — leftover RPM / usage-count leak /
re-probe WARN
3. Important? **PASS** — resource leak that can WARN and leave PM state
wrong; not a success-path panic, but standard driver-core leak
4. Small and contained? **PASS** — one function, +22/−9
5. No new features/APIs? **PASS**
6. Applies to this tree? **PASS** — buggy code and helpers are here
**Step 9.3: Exception category**
Record: Not a device-ID/quirk/DT/build/docs exception. Ordinary error-
path leak fix.
**Step 9.4: Decision**
This 6.18 tree has the leak. The patch is a small, correct disable of
runtime PM on probe failures, with the right `resume_and_get` pairing.
Later optional rework is not a reason to skip this.
### What it solves for stable users
On NXP FlexSPI, probe currently enables runtime PM, then can return on
clock/IRQ/mutex failure before the devm cleanup action. That leaves RPM
enabled (and, with `get_sync`, an elevated usage count). A later bind
can WARN. The patch disables RPM on those paths.
### Risk vs benefit
Low risk, clear benefit on a widely used embedded SPI-NOR controller. No
prerequisites beyond code already in 6.18.
---
Verification:
- [Phase 1] Parsed tags: author Jiawen Liu, Link msgid, SOB Mark Brown;
no Fixes/Reported-by/Cc stable
- [Phase 2] Diff: `nxp_fspi_probe()` +22/−9; four early returns →
`err_disable_pm`; `get_sync` → `resume_and_get`
- [Phase 2] Confirmed `pm_runtime_get_sync` increments usage even on
error (`include/linux/pm_runtime.h`); `resume_and_get` does not
- [Phase 3] `git describe HEAD` / `make kernelversion`: **6.18.53**,
branch `autosel`
- [Phase 3] `git blame` on parent: leak from `97be4b919a609`
(2025-04-28)
- [Phase 3] `git merge-base --is-ancestor 97be4b919a609 v6.18`: yes
(also in v6.16)
- [Phase 3] Parent probe still has `get_sync` + bare returns after
`pm_runtime_enable`
- [Phase 4] `b4 dig -c 5d14285d60ba`: PATCH 4/4, lore msgid above
- [Phase 4] `b4 dig -a`: v1 only for this commit
- [Phase 4] `b4 dig -w`: Mark Brown, Han Xu, Haibo Chen, linux-spi, imx
list
- [Phase 4] mbox: Frank Li style comment; Mark Brown applied 4/4 to
for-7.3; cover letter says independent
- [Phase 4] Later v2 rework (Jul 2026) not in this tree
- [Phase 5] Probe is platform `.probe`; Kconfig `SPI_NXP_FLEXSPI` for
i.MX/Layerscape
- [Phase 6] `pm_runtime_resume_and_get` exists; patch context matches
local probe
- [Phase 8] Failure mode: RPM leak / usage-count leak / re-probe WARN,
not a hot-path oops
- UNVERIFIED: lore.kernel.org HTTP 403; no dedicated stable@ discussion
found
**YES**
drivers/spi/spi-nxp-fspi.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index b504a5c86692a..0300f4ae1455d 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -1426,9 +1426,11 @@ static int nxp_fspi_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(dev);
/* enable clock */
- ret = pm_runtime_get_sync(f->dev);
- if (ret < 0)
- return dev_err_probe(dev, ret, "Failed to enable clock");
+ ret = pm_runtime_resume_and_get(f->dev);
+ if (ret < 0) {
+ ret = dev_err_probe(dev, ret, "Failed to enable clock");
+ goto err_disable_pm;
+ }
/* Clear potential interrupts */
reg = fspi_readl(f, f->iobase + FSPI_INTR);
@@ -1438,18 +1440,24 @@ static int nxp_fspi_probe(struct platform_device *pdev)
nxp_fspi_default_setup(f);
ret = pm_runtime_put_sync(dev);
- if (ret < 0)
- return dev_err_probe(dev, ret, "Failed to disable clock");
+ if (ret < 0) {
+ ret = dev_err_probe(dev, ret, "Failed to disable clock");
+ goto err_disable_pm;
+ }
init_completion(&f->c);
ret = devm_request_irq(dev, irq,
nxp_fspi_irq_handler, 0, pdev->name, f);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to request irq\n");
+ if (ret) {
+ ret = dev_err_probe(dev, ret, "Failed to request irq\n");
+ goto err_disable_pm;
+ }
ret = devm_mutex_init(dev, &f->lock);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to initialize lock\n");
+ if (ret) {
+ ret = dev_err_probe(dev, ret, "Failed to initialize lock\n");
+ goto err_disable_pm;
+ }
ctlr->bus_num = -1;
ctlr->num_chipselect = NXP_FSPI_MAX_CHIPSELECT;
@@ -1467,6 +1475,11 @@ static int nxp_fspi_probe(struct platform_device *pdev)
return ret;
return devm_spi_register_controller(&pdev->dev, ctlr);
+
+err_disable_pm:
+ pm_runtime_dont_use_autosuspend(dev);
+ pm_runtime_disable(dev);
+ return ret;
}
static int nxp_fspi_runtime_suspend(struct device *dev)
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx: return error from failed controller suspend
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] spi: nxp-fspi: disable runtime PM on probe failures Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18] spi: loongson: pm: add .freeze .poweroff .thaw .restore Sasha Levin
` (4 subsequent siblings)
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Jiawen Liu, Mark Brown, Sasha Levin, jonas.gorski, linux-spi,
linux-kernel
From: Jiawen Liu <1298662399@qq.com>
[ Upstream commit bbbc1ef5b35341c5122418a14f8924688a4bb954 ]
spi_controller_suspend() can fail if the SPI core cannot stop the
controller. bcm63xx_spi_suspend() ignored that error, disabled the
controller clock, and returned success.
Return the suspend error before tearing down the clock.
Signed-off-by: Jiawen Liu <1298662399@qq.com>
Link: https://patch.msgid.link/tencent_0BD7D4091B90EC17A8B2BA5EBA8803725905@qq.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The tree is **6.18.53** (`v6.18.53`, Makefile `VERSION=6 PATCHLEVEL=18
SUBLEVEL=53`). The buggy suspend path is in this checkout; the upstream
fix is not in `v6.18.53`. Verdict is for this tree only.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[spi: bcm63xx:]` `[return]` Return error from failed
`spi_controller_suspend()` instead of ignoring it and disabling the
clock.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Signed-off-by: Jiawen Liu <1298662399@qq.com>` (author)
- `Link: https://patch.msgid.link/tencent_0BD7D4091B90EC17A8B2BA5EBA8803
725905@qq.com`
- `Signed-off-by: Mark Brown <broonie@kernel.org>` (SPI maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence expected for this review)
- Author SOB only besides Mark Brown (Sasha Levin SOB ignored per rules)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- [Bug] `bcm63xx_spi_suspend()` ignores `spi_controller_suspend()`
failure, then `clk_disable_unprepare()` and returns 0.
- [Symptom] Suspend reported success with the controller clock already
torn down after a failed queue stop.
- [Version info] None in the message. Code has been this way since the
driver was added.
- [Root cause] `spi_controller_suspend()` can return `-EBUSY` when
`spi_stop_queue()` cannot drain the queue; the driver treated that as
success.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. It is an explicit error-propagation / PM correctness
fix (clock must not be gated if the core could not stop the controller).
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/spi/spi-bcm63xx.c` (+4 / -1)
- Function: `bcm63xx_spi_suspend()`
- Scope: single-file surgical PM error-path fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: Before: call `spi_controller_suspend(host)` and ignore the
result, then always `clk_disable_unprepare(bs->clk)` and `return 0`.
After: capture `ret`; if non-zero, return it immediately and leave the
clock enabled.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: [Logic / PM error handling] The SPI core can fail to stop the
queued pump (`spi_stop_queue()` polls ~5s then returns `-EBUSY`).
Continuing to ungated-clock teardown and reporting success leaves the
kthread able to touch the controller with the clock off (hang /
imprecise abort) and lets system suspend proceed with an active/busy SPI
block.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct and matches the long-standing pattern in other
in-tree SPI drivers (`spi-atmel`, `spi-rockchip`, `spi-bcm63xx-hsspi`,
etc.). Minimal. Regression risk is low: on failure, suspend aborts and
the clock stays on. Residual SPI-core quirk: `spi_controller_suspend()`
still calls `__spi_mark_suspended()` even on `spi_stop_queue()` failure
(pre-existing; not introduced here).
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record: Ignore-and-disable-clock dates to `b42dfed83d95a3` (“spi: add
Broadcom BCM63xx SPI controller driver”, 2012-03-09). Present in this
tree. `spi_controller_suspend()` has returned `spi_stop_queue()` errors
and marked `SPI_CONTROLLER_SUSPENDED` since `bef4a48f4ef798`
(2023-11-10, already in `v6.18.53`).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. The behavioral gap exists
because the driver never checked a return value that can fail.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Related in this tree: `0acbfd61aee18` (“spi: bcm63xx: disable
clock on resume failure”) is already in `v6.18.53` (explicit `Cc:
stable`). Same author’s series: `spi-bcm63xx-hsspi` and `spi-bcm-qspi`
get the same suspend check; each patch is per-driver and standalone.
This is patch 1/3, self-contained.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Jiawen Liu is not the driver maintainer; Jonas Gorski is listed
as Odd Fixes. Mark Brown applied the series. Author also sent the
sibling Broadcom SPI suspend checks.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Uses existing `spi_controller_suspend()` /
`clk_disable_unprepare()`. Dry-run apply of this hunk onto
`v6.18.53:drivers/spi/spi-bcm63xx.c` succeeded (`rc 0`).
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c bbbc1ef5b35341c5122418a14f8924688a4bb954` → https://patch.m
sgid.link/tencent_0BD7D4091B90EC17A8B2BA5EBA8803725905@qq.com
- `-a`: v1 only, `[PATCH 1/3]`
- Thread: submission + Mark Brown “Applied to for-7.3” (no NAK, no
requested respin, no reviewer `Cc: stable`)
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): `broonie@kernel.org`, `linux-spi@vger.kernel.org`,
Broadcom list, `jonas.gorski@gmail.com`, Broadcom SPI contacts.
Maintainer applied it.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by`, syzbot, or bugzilla. Code-inspection fix. No
evidence of a field crash report.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Series is three independent driver patches (bcm63xx, bcm63xx-
hsspi, bcm-qspi). This one does not need the others. Resume-side
counterpart (`spi: bcm63xx: disable clock on resume failure`) already
landed in `v6.18.53`.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No stable-list discussion found for this subject.
lore.kernel.org WebFetch returned 403; thread recovered via `b4 dig -m`.
UNVERIFIED: exhaustive lore.kernel.org/stable search via HTTP.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `bcm63xx_spi_suspend()`; callees `spi_controller_suspend()`,
`clk_disable_unprepare()`.
**Step 5.2: TRACE CALLERS**
Record: Only wired through `DEFINE_SIMPLE_DEV_PM_OPS(bcm63xx_spi_pm_ops,
...)` → platform driver `.pm`. System-sleep path, not a syscall.
Triggered on suspend/hibernate of BCM63xx/BMIPS boards with this
controller.
**Step 5.3: TRACE CALLEES**
Record: `spi_controller_suspend()` (`drivers/spi/spi.c`): if
`ctlr->queued`, `spi_stop_queue()`; always `__spi_mark_suspended()`;
returns `-EBUSY` on timeout. `spi_stop_queue()` waits up to 500 × ~10
ms.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: `bcm63xx` sets `host->transfer_one_message` and does not set
`host->transfer`, so `spi_register_controller()` calls
`spi_controller_initialize_queue()` and `ctlr->queued = true`. The
failure path is reachable. Userspace can keep the queue busy (SPI flash,
sensors); a stuck IRQ/transfer makes `-EBUSY` realistic.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same ignore-then-ungate bug was fixed in sibling Broadcom SPI
drivers. Many other SPI hosts already check and return
`spi_controller_suspend()` errors.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes, at `v6.18.53` in `bcm63xx_spi_suspend()`:
```c
spi_controller_suspend(host);
clk_disable_unprepare(bs->clk);
return 0;
```
`git merge-base --is-ancestor bbbc1ef5b35341c5122418a14f8924688a4bb954
v6.18.53` → not an ancestor. Bug present since 2012; fail-able
`spi_controller_suspend()` since 2023 (in this tree).
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply. Surrounding `bcm63xx_spi_resume()` already matches
current mainline (resume error path already backported).
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: Resume-side clock rollback is already in `v6.18.53`. No earlier
version of this suspend check.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/spi/` Broadcom BCM6348/BCM6358 controller. Criticality:
PERIPHERAL (SoC-specific), but real OpenWrt/BMIPS users.
`CONFIG_SPI_BCM63XX` depends on `BCM63XX || BMIPS_GENERIC ||
COMPILE_TEST`. Maintainer status: Odd Fixes.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: File still gets real fixes in 6.18.y (shared reset, CS,
deregistration, resume clock). Mature driver, not staging.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Driver-specific / config-specific: BCM63xx and BMIPS platforms
using this SPI host, on system suspend.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: System suspend while the SPI message pump is still busy after
~5s. Not every boot; can be hit if a transfer is stuck or the queue
cannot drain. Unprivileged users can keep SPI busy; they cannot directly
call the PM callback (needs suspend permission).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Clock gated while the queued pump may still run → MMIO with
clock off, possible bus hang, incomplete in-flight SPI (flash). PM
reports success, so the system continues suspend. Severity: HIGH (hang /
possible device corruption on a rare path). Not a universal panic.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: Benefit: high for affected boards (do not sleep with SPI still
active and clock off). Risk: very low (4 insertions, error return only).
Residual: on failure the core still sets `SPI_CONTROLLER_SUSPENDED` and
PM skips resume for this device (`is_suspended` stays false), so SPI may
stay `-ESHUTDOWN` until a later successful suspend/resume. That core
behavior already exists; other hosts already return the error the same
way. Ratio: favor backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real PM bug: ignore failure, ungated clock, return success
- Failure mode is hang / in-flight SPI with clock off, not cosmetic
- Driver is queued, so `spi_controller_suspend()` can return `-EBUSY`
- 5-line, obviously correct, same pattern as other SPI hosts and the
sibling series
- SPI maintainer applied it; driver maintainer was CCed
- Buggy code is in `v6.18.53`; patch applies cleanly; no prerequisites
- Resume-side analogue already taken for this stable tree
AGAINST:
- No user/syzbot report; inspection-only
- Trigger is uncommon (queue-stop timeout)
- Peripheral SoC driver, Odd Fixes
- Sent to for-7.3 merge window, not as an urgent -rc fix
- SPI core still marks suspended on failure (pre-existing)
UNRESOLVED:
- HTTP fetch of lore.kernel.org/stable returned 403 (thread obtained via
`b4`)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — mechanical return-value
check; maintainer applied v1 with no NAK
2. Fixes a real bug that affects users? **PASS** — clock off + fake
success on failed controller stop
3. Important issue? **PASS** — hang / inconsistent hardware on suspend,
not a new feature
4. Small and contained? **PASS** — +4/-1, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — dry-run clean on `v6.18.53`
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not an ID/quirk/DT/build/docs exception). Qualifies as a
normal small bugfix.
**Step 9.4: DECISION**
Backport. The bug exists on this 6.18.53 tree, the fix is surgical, and
leaving suspend success + clock off after a failed queue stop is worse
than aborting suspend.
------------------------------------------------
What the commit solves for this tree
`bcm63xx_spi_suspend()` must not disable the controller clock or report
success if `spi_controller_suspend()` failed. On BCM63xx this host is
queued, so that call can return `-EBUSY`. Continuing would let the pump
access the block with the clock off and let system suspend proceed
anyway.
Stable rules
Meets all six criteria: small, obviously correct, no new API, real PM
bug, applies cleanly here.
Risk vs benefit
Benefit is avoiding a hang/inconsistent SPI across suspend on these
SoCs. Risk is a 4-line error return that matches the rest of the SPI
stack.
Concerns
No field report; rare trigger; Odd Fixes driver. None of that outweighs
a clean, standard PM error-path fix whose resume counterpart is already
in this tree.
Verification:
- [Phase 1] Subject/tags parsed from the supplied message and `git show
df107c1912ab6` / upstream `bbbc1ef5b353`
- [Phase 2] Diff is +4/-1 in `bcm63xx_spi_suspend()` only
- [Phase 3] `git blame` / `git show b42dfed83d95a3`: ignore-clock-off
since 2012; `bef4a48f4ef798` in `v6.18.53`
- [Phase 3] `git merge-base --is-ancestor bbbc1ef5… v6.18.53` → not
present; buggy code confirmed via `git show v6.18.53:drivers/spi/spi-
bcm63xx.c`
- [Phase 3] `0acbfd61aee18` resume fix already in `v6.18.53`
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 1/3, Mark Brown applied, no NAK;
lore HTTP 403
- [Phase 5] `spi_register_controller()` queues this driver;
`spi_stop_queue()` can return `-EBUSY`
- [Phase 6] `patch --dry-run` of this commit onto `v6.18.53` file: rc 0
- [Phase 6] `git describe HEAD` / `make kernelversion`: 6.18.53
- [Phase 7] MAINTAINERS: Odd Fixes, Jonas Gorski; Kconfig `SPI_BCM63XX`
- [Phase 8] PM core: failed suspend does not set `is_suspended`, so this
device’s resume is skipped — pre-existing core mark-suspended-on-error
- UNVERIFIED: exhaustive lore.kernel.org/stable HTTP search (403)
**YES**
drivers/spi/spi-bcm63xx.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index 8d152e906daac..f73dc14533dcc 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -643,8 +643,11 @@ static int bcm63xx_spi_suspend(struct device *dev)
{
struct spi_controller *host = dev_get_drvdata(dev);
struct bcm63xx_spi *bs = spi_controller_get_devdata(host);
+ int ret;
- spi_controller_suspend(host);
+ ret = spi_controller_suspend(host);
+ if (ret)
+ return ret;
clk_disable_unprepare(bs->clk);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18] spi: loongson: pm: add .freeze .poweroff .thaw .restore
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx: return error from failed controller suspend Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx-hsspi: return error from failed controller suspend Sasha Levin
` (3 subsequent siblings)
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Li Jun, Mark Brown, Sasha Levin, zhuyinbo, linux-spi,
linux-kernel
From: Li Jun <lijun01@kylinos.cn>
[ Upstream commit a9c2f0d401fd1e11ce59e4243946a59bd818e8c5 ]
after execute s4, the spi error,
[ 1104.754246][ 4] [ T1] tpm_tis_spi spi-SMO0768:00: SPI transfer failed: -110
[ 1104.761503][ 4] [ T1] spi_master spi1: failed to transfer one message from queue
[ 1104.769201][ 4] [ T1] spi_master spi1: noqueue transfer failed
[ 1104.776344][ 4] [ T1] tpm_tis_spi spi-SMO0768:00: SPI transfer failed: -110
[ 1104.783609][ 4] [ T1] spi_master spi1: failed to transfer one message from queue
[ 1104.791308][ 4] [ T1] spi_master spi1: noqueue transfer failed
[ 1104.797446][ 4] [ T1] gttadd tpm_chip_start1 ret = -110
and in s4 the loongson_spi_resume&suspend are not
called at all. use DEFINE_SIMPLE_DEV_PM_OPS() add .freeze .poweroff
.thaw .restore, after s4 the spi communication is normal.
Signed-off-by: Li Jun <lijun01@kylinos.cn>
Link: https://patch.msgid.link/20260820092351.101605-1-lijun01@kylinos.cn
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The buggy PM ops are
present here; the candidate is not. The rest of the write-up is the full
nine-phase review for this tree only.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[spi: loongson: pm] [add] [hibernation callbacks
.freeze/.poweroff/.thaw/.restore]**
The verb is “add”, but the body is a hibernation bug report (S4 SPI
timeout), not a feature.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review)
- **Reported-by:** none
- **Tested-by:** none (author states it was tested after S4)
- **Reviewed-by:** none on the committed message
- **Acked-by:** none
- **Link:**
https://patch.msgid.link/20260820092351.101605-1-lijun01@kylinos.cn
- **Cc: stable:** none (expected)
- **Signed-off-by:** Li Jun \<lijun01@kylinos.cn\> (author); Mark Brown
\<broonie@kernel.org\> (SPI maintainer)
- **Ignored pipeline tags:** none present in the provided message beyond
the usual evaluation context
Notable: SPI maintainer SOB; mailing-list Link; no syzbot.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** After S4 (hibernate), Loongson SPI transfers fail with **-110
(`ETIMEDOUT`)**. `tpm_tis_spi` device `spi-SMO0768:00` cannot talk to
the controller; `tpm_chip_start` returns -110.
- **Symptom:** SPI master `spi1` “failed to transfer one message from
queue” / “noqueue transfer failed”.
- **Root cause (author):** `loongson_spi_suspend`/`loongson_spi_resume`
are **not called at all during S4**, because only `.suspend`/`.resume`
were registered.
- **Fix:** `DEFINE_SIMPLE_DEV_PM_OPS()` so the same save/restore path is
also used for `.freeze`, `.poweroff`, `.thaw`, `.restore`.
- **Version info:** none in the message; dmesg is a real hibernate-
resume failure.
- **Author test:** “after s4 the spi communication is normal.”
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** It is a real hibernation PM bug. “add …
callbacks” is the usual wording for filling missing `dev_pm_ops` slots;
it is not a new API or new driver.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/spi/spi-loongson-core.c`
- **Scope:** ~2 insertions, ~4 deletions (single struct rewrite)
- **Functions:** `loongson_spi_suspend` / `loongson_spi_resume`
unchanged; only `loongson_spi_dev_pm_ops` definition changes
- **Classification:** single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `dev_pm_ops` has only `.suspend` and `.resume`. Kernel
hibernation uses `.freeze`/`.thaw`/`.poweroff`/`.restore`
(`include/linux/pm.h`). Those were NULL, so S4 never saved/restored
SPI registers or called
`spi_controller_suspend`/`spi_controller_resume`.
- **After:** `DEFINE_SIMPLE_DEV_PM_OPS` → `SYSTEM_SLEEP_PM_OPS`, which
sets all six sleep callbacks to the existing suspend/resume pair via
`pm_sleep_ptr()`.
- **Path affected:** system hibernation (S4) and poweroff-after-image.
S3 still uses the same functions.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** logic/correctness in PM ops (missing hibernation
callbacks); hardware state not restored after S4
- **Mechanism:** After hibernate, SPI controller registers are not
rewritten (`SPCR`/`SPER`/`SPSR`/`PARA`/`SFCS`/`TIMI`). Transfers time
out. TPM on that bus (`SMO0768`) fails.
- **Not** UAF/leak/race/overflow.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** Obviously correct. Same pattern as `SYSTEM_SLEEP_PM_OPS`
/ `SET_SYSTEM_SLEEP_PM_OPS` used across `drivers/spi/` (e.g. `spi-
sifive.c`, `spi-bcm63xx.c`, many `SET_SYSTEM_SLEEP_PM_OPS` users).
- **Minimal:** no unrelated edits.
- **Regression risk:** Low. S3 still uses the same two functions.
Hibernation now runs the save/restore that S3 already used.
`RUNTIME_PM_OPS(NULL,NULL,NULL)` matches the old struct (no runtime
PM).
- **Style note (not a blocker):** `include/linux/pm.h` prefers
`EXPORT_NS_GPL_SIMPLE_DEV_PM_OPS` when the symbol is exported.
Mainline kept `DEFINE_SIMPLE_DEV_PM_OPS` + existing
`EXPORT_SYMBOL_NS_GPL`. Functionally equivalent on this tree: the
macro is non-static `const struct dev_pm_ops`.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Buggy ops introduced in **`6c7a864007b66`** (“spi: loongson: add bus
driver for the loongson spi controller”, Yinbo Zhu, 2023-06-13;
committed 2023-07-31).
- **`git merge-base --is-ancestor 6c7a864007b66 HEAD`** succeeded.
- Present since **v6.6** (`v6.5` does not contain it; `v6.6` does).
Unchanged since then (`git blame` on lines 232–276 is entirely that
commit).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The introducing commit is the original driver,
and it **is** in this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- `git log --oneline -- drivers/spi/spi-loongson-core.c` shows **only**
`6c7a864007b66`.
- No later refactor of these PM ops in this tree.
- Standalone; subject is `[PATCH v3]`, not `X/Y`.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Author Li Jun (Kylinos) has **no** other commits under `drivers/spi/`
in this checkout.
- Original driver author is Yinbo Zhu (Loongson). Maintainer who applied
the fix is Mark Brown.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Needs `DEFINE_SIMPLE_DEV_PM_OPS` — present at `include/linux/pm.h:418`
in this tree.
- Needs existing `loongson_spi_suspend`/`loongson_spi_resume` — present.
- **Standalone.** No new helpers.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **v1:** https://lore.kernel.org/linux-
spi/20260818104241.850042-1-lijun01@kylinos.cn/ — manually added four
callback pointers.
- **v2:** https://lore.kernel.org/linux-
spi/20260819014552.1986077-1-lijun01@kylinos.cn/ —
`SIMPLE_DEV_PM_OPS()` after review.
- **v3 (committed):** https://lore.kernel.org/linux-
spi/20260820092351.101605-1-lijun01@kylinos.cn/ —
`DEFINE_SIMPLE_DEV_PM_OPS()` (non-deprecated helper).
- **`b4 mbox -c`** on v3: no newer revision.
- **Reviewer:** Mark Brown on v1: “This could be done using
SIMPLE_DEV_PM_OPS().” No NAK. He applied v3 to `broonie/spi.git
for-7.3` (`a9c2f0d401fd`).
- No `Cc: stable` in review (not a negative signal).
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record:
- **To/Cc:** Li Jun, Yinbo Zhu (original driver author), Mark Brown,
linux-spi.
- SPI maintainer reviewed and applied. Original driver author was CCed;
no objection in the threads retrieved.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record:
- No bugzilla/syzbot. Evidence is the author’s dmesg plus “after s4 …
normal.”
- ACPI id **SMO0768** is in this tree:
`drivers/char/tpm/tpm_tis_spi_main.c` (`acpi_tis_spi_match`). `-110`
is `ETIMEDOUT` (`include/uapi/asm-generic/errno.h`).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Three revisions of **one** patch. v3 is the applied form. No
other patches in the series.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search for “loongson spi freeze” returned
only unrelated “freeze” hits in stable review series. **No dedicated
stable discussion** of this bug (search via lore `?q=` HTML, HTTP 200).
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: Symbol `loongson_spi_dev_pm_ops` only. Call targets
`loongson_spi_suspend` and `loongson_spi_resume` are unchanged.
**Step 5.2: TRACE CALLERS**
Record:
- `drivers/spi/spi-loongson-plat.c`: platform `.driver.pm =
&loongson_spi_dev_pm_ops`
- `drivers/spi/spi-loongson-pci.c`: PCI `.driver.pm =
&loongson_spi_dev_pm_ops`
- Invoked by the PM core on system sleep/hibernation, not from a syscall
directly.
**Step 5.3: TRACE CALLEES**
Record (`loongson_spi_suspend`/`resume` in this tree):
- Suspend: `spi_controller_suspend()`, then read 6 controller registers
into `struct loongson_spi`.
- Resume: write those 6 registers back, then `spi_controller_resume()`.
- `spi_controller_suspend` (`drivers/spi/spi.c`) stops a queued
controller and sets `SPI_CONTROLLER_SUSPENDED`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
- Userspace (typically root): `echo disk > /sys/power/state` or distro
hibernate.
- PM core → driver `.freeze`/`.poweroff`/`.restore` → save/restore SPI
HW → later `tpm_tis_spi` transfers.
- **Reachable** on Loongson machines that hibernate with this controller
and an SPI TPM (or any SPI child).
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: In this tree, almost every other SPI driver uses
`SET_SYSTEM_SLEEP_PM_OPS` or `DEFINE_SIMPLE_DEV_PM_OPS`. Loongson was an
outlier with only `.suspend`/`.resume`. That matches the S4-only
failure.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `drivers/spi/spi-loongson-core.c` lines 272–275:
```c
const struct dev_pm_ops loongson_spi_dev_pm_ops = {
.suspend = loongson_spi_suspend,
.resume = loongson_spi_resume,
};
```
Bug since driver addition (**v6.6**). This tree is **6.12.111**.
Candidate not in HEAD (`git log --grep='add .freeze' -- drivers/spi/spi-
loongson-core.c` empty).
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Minor context, clean functional apply.**
- This tree still has `__maybe_unused` on suspend/resume; the upstream
hunk context dropped it.
- This tree exports `SPI_LOONGSON_CORE` unquoted; upstream uses
`"SPI_LOONGSON_CORE"`.
- **`git apply --check` of the verbatim upstream hunk failed.**
- An adapted hunk using local context **applied cleanly** (`git apply
--check` exit 0).
- `DEFINE_SIMPLE_DEV_PM_OPS` exists here.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No other loongson SPI PM/hibernation fix in this file’s history.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record:
- **Subsystem:** SPI controller driver (`drivers/spi/`),
Loongson/LoongArch.
- **Criticality:** **PERIPHERAL** for the kernel as a whole,
**IMPORTANT** for Loongson users (SPI TPM after hibernate).
- Kconfig: `SPI_LOONGSON_CORE` / `_PCI` / `_PLATFORM`, `depends on
LOONGARCH || COMPILE_TEST`.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: This driver is **mature/quiet** in 6.12 (single commit). The bug
is long-standing, not a new-subsystem race.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Platform-specific / driver-specific:** LoongArch systems with
Loongson SPI (`loongson,ls2k1000-spi` or PCI 0x7a0b/0x7a1b) that
hibernate. Reporter path includes Infineon SPI TPM ACPI `SMO0768`. Not
universal.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: **hibernate (S4)**, then resume. Not every boot; every
successful S4 on this hardware.
- Unprivileged users cannot normally hibernate (needs CAP_SYS_BOOT /
root). Not a local-DoS from unprivileged userspace.
- Not a timing race; deterministic missing callbacks.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- **Failure:** SPI I/O timeouts; TPM start fails (`tpm_chip_start`
-110). Controller left unrestored after image restore.
- **Severity: HIGH** (not a panic, but broken SPI/TPM after hibernate is
a serious PM regression). TPM unusable can break measured boot / TPM-
backed secrets after S4.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit: high** for affected Loongson/S4 users (restores SPI after
hibernate).
- **Risk: very low** (~2-line helper swap, standard PM pattern, S3 path
unchanged).
- **Ratio:** strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real S4 failure with dmesg; author confirmed fix.
- Buggy incomplete `dev_pm_ops` present in this 6.12 tree since v6.6.
- Tiny, standard `DEFINE_SIMPLE_DEV_PM_OPS` change; SPI maintainer
requested the helper and applied v3.
- TPM (`SMO0768`) is a real child device in-tree.
- `DEFINE_SIMPLE_DEV_PM_OPS` exists here; adapted patch applies.
**AGAINST:**
- LoongArch/Loongson-only; hibernate-only (not every boot).
- Subject says “add”; no `Fixes:`/`Cc: stable` (not used as a no).
- Verbatim upstream hunk needs a one-line context tweak (namespace
quotes / `__maybe_unused`).
- Could have used `EXPORT_NS_GPL_SIMPLE_DEV_PM_OPS` (style only).
**UNRESOLVED:** Could not open lore via WebFetch (403); threads were
retrieved with `b4 mbox` and lore HTML search instead. Exact mainline
SHA of the evaluation candidate was not in this checkout (`a9c2f0d401fd`
is Mark’s spi.git apply of v3).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — standard helper; author
tested S4; maintainer applied.
2. Fixes a real bug that affects users? **PASS** — SPI/TPM broken after
hibernate.
3. Important issue? **PASS** — HIGH: I/O failure and TPM unusable after
S4 (not a panic, still production-breaking PM).
4. Small and contained? **PASS** — one struct, one file.
5. No new features or APIs? **PASS** — missing hibernation ops, not a
new API.
6. Can apply to the local tree? **PASS** — with trivial context
adjustment; macros and driver exist.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It is a normal
small PM bugfix.
**Step 9.4: MAKE YOUR DECISION**
This should be backported to **this 6.12 tree**. The driver has only
`.suspend`/`.resume`, so hibernation never restores the controller; that
matches the S4 TPM timeouts. The fix is the usual
`DEFINE_SIMPLE_DEV_PM_OPS` mapping, already used by other SPI drivers
here, with very low regression risk.
The problem for 6.12 users is that Loongson SPI (and the SPI TPM on that
bus) stays dead after hibernate because hibernation callbacks were never
wired. That is a real, tested, important PM bug. It meets the stable
rules: small, obviously correct, no new API, and it applies here with a
trivial context tweak.
Verification:
- [Phase 1] Parsed subject/tags/body from the provided commit and v1–v3
mboxes; no Fixes/Reported-by/Cc:stable; Link msgid
`20260820092351.101605-1-lijun01@kylinos.cn`
- [Phase 2] Diff: replace 4-line `dev_pm_ops` with
`DEFINE_SIMPLE_DEV_PM_OPS`; functions unchanged
- [Phase 2] Confirmed `SYSTEM_SLEEP_PM_OPS` sets
suspend/resume/freeze/thaw/poweroff/restore
(`include/linux/pm.h:312-318`); `-110` is `ETIMEDOUT`
- [Phase 3] `git blame` lines 232–276 → `6c7a864007b66`; `git merge-base
--is-ancestor 6c7a864007b66 HEAD` true; `v6.5` no, `v6.6` yes
- [Phase 3] `git log --oneline -- drivers/spi/spi-loongson*.c` only the
original driver commit
- [Phase 4] `b4 mbox` v1/v2/v3; Mark Brown asked for
`SIMPLE_DEV_PM_OPS()` on v1; v3 uses `DEFINE_SIMPLE_DEV_PM_OPS`; `b4
mbox -c` no newer rev
- [Phase 4] lore.kernel.org/stable `?q=loongson+spi+freeze` HTTP 200, no
dedicated thread
- [Phase 5] Callers: `spi-loongson-plat.c:39`, `spi-loongson-pci.c:48`;
ACPI `SMO0768` in `tpm_tis_spi_main.c:342`
- [Phase 6] `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion` → `6.12.111`; buggy ops at `spi-loongson-
core.c:272-275`
- [Phase 6] Original hunk `git apply --check` failed; adapted hunk
(unquoted ns + `__maybe_unused` context) succeeded
- [Phase 7] Kconfig `SPI_LOONGSON_*` depends on `LOONGARCH ||
COMPILE_TEST`
- [Phase 8] Failure: SPI timeout / TPM start -110 after S4; severity
HIGH; risk very low
- UNVERIFIED: WebFetch to lore/git.kernel.org returned 403; used `b4`
and curl HTML instead
- UNVERIFIED: Did not boot-test hibernate on Loongson hardware; relied
on author’s dmesg and code
**YES**
drivers/spi/spi-loongson-core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-loongson-core.c b/drivers/spi/spi-loongson-core.c
index b46f072a0387b..f07ee432f96b5 100644
--- a/drivers/spi/spi-loongson-core.c
+++ b/drivers/spi/spi-loongson-core.c
@@ -270,10 +270,8 @@ static int __maybe_unused loongson_spi_resume(struct device *dev)
return 0;
}
-const struct dev_pm_ops loongson_spi_dev_pm_ops = {
- .suspend = loongson_spi_suspend,
- .resume = loongson_spi_resume,
-};
+DEFINE_SIMPLE_DEV_PM_OPS(loongson_spi_dev_pm_ops, loongson_spi_suspend, loongson_spi_resume);
+
EXPORT_SYMBOL_NS_GPL(loongson_spi_dev_pm_ops, "SPI_LOONGSON_CORE");
MODULE_DESCRIPTION("Loongson SPI core driver");
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx-hsspi: return error from failed controller suspend
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18] spi: loongson: pm: add .freeze .poweroff .thaw .restore Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] spi: fsl-dspi: clean up after failed suspend and resume Sasha Levin
` (2 subsequent siblings)
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Jiawen Liu, William Zhang, Mark Brown, Sasha Levin, kursad.oney,
jonas.gorski, linux-spi, linux-kernel
From: Jiawen Liu <1298662399@qq.com>
[ Upstream commit 3b5414d76a0381db77b9694537254af7acfa187a ]
spi_controller_suspend() can fail if the SPI core cannot stop the
controller. bcm63xx_hsspi_suspend() ignored that error, disabled the
PLL and core clocks, and returned success.
Return the suspend error before disabling the clocks.
Signed-off-by: Jiawen Liu <1298662399@qq.com>
Acked-by: William Zhang <william.zhang@broadcom.com>
Link: https://patch.msgid.link/tencent_B5A06807924A77C8690730EBF7A052AABE05@qq.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable backport analysis: `spi: bcm63xx-hsspi: return error from
failed controller suspend`
Local tree is **6.18.53** (`v6.18.53-5049-g0f0bdc2b0dffb`, Makefile
`VERSION=6` / `PATCHLEVEL=18` / `SUBLEVEL=53`). The verdict is only for
this tree.
Upstream commit: `3b5414d76a0381db77b9694537254af7acfa187a` (Mark Brown,
2026-06-30). It is not an ancestor of `v6.18` or `v6.18.53`. The buggy
suspend path **is** present at `v6.18.53`.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 — Subject**
Record: `[spi] [return/propagate] return error from failed controller
suspend` — subsystem `spi: bcm63xx-hsspi:`; verb “return” meaning “stop
ignoring a failure.”
**Step 1.2 — Tags**
Record:
- `Signed-off-by: Jiawen Liu <1298662399@qq.com>` (author)
- `Acked-by: William Zhang <william.zhang@broadcom.com>` (listed
maintainer of this driver)
- `Link: https://patch.msgid.link/tencent_B5A06807924A77C8690730EBF7A052
AABE05@qq.com`
- `Signed-off-by: Mark Brown <broonie@kernel.org>` (SPI maintainer)
- No `Fixes:`, no `Reported-by:`, no `Cc: stable` (expected for this
review)
- Ignore pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit ...]`
**Step 1.3 — Body**
Record:
- **Bug:** `bcm63xx_hsspi_suspend()` ignores `spi_controller_suspend()`
failure, then disables PLL and core clocks and returns 0.
- **Symptom:** system suspend is reported successful while the SPI queue
may still be running and clocks are gated.
- **Root cause:** missing return-value check on
`spi_controller_suspend()`.
- **Version info:** none in the message. Bug is in the original 2013
driver (`142168eba9dc5`).
**Step 1.4 — Hidden bug fix?**
Record: Not hidden. It is an explicit error-path correctness fix
(ignored failure → clocks torn down anyway).
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1 — Inventory**
Record: one file, `drivers/spi/spi-bcm63xx-hsspi.c`, **+5 / −1**.
Function: `bcm63xx_hsspi_suspend()`. Surgical, single-file.
**Step 2.2 — Flow**
Record: **Before:** call `spi_controller_suspend(host)` and ignore the
result; always `clk_disable_unprepare()` on PLL and core clocks; return
0. **After:** if `spi_controller_suspend()` fails, return that error and
leave clocks enabled.
**Step 2.3 — Bug mechanism**
Record: **Logic / PM error-path.** `spi_controller_suspend()` for a
queued controller calls `spi_stop_queue()`, which polls up to 500 × ~10
ms and returns `-EBUSY` if the queue is still busy. The core still
`__spi_mark_suspended()` and returns that error. The driver then gated
clocks and lied to PM with success.
This driver sets `host->transfer_one_message` and does **not** set
`host->transfer`, so `spi_register_controller()` runs
`spi_controller_initialize_queue()` and sets `ctlr->queued = true`. The
failure path is reachable.
**Step 2.4 — Fix quality**
Record: Obviously correct; matches the established SPI driver pattern
(`spi-bcm63xx.c`, `spi-rockchip.c`, `spi-atmel.c`, etc.). Minimal.
Regression risk is low: on failure, suspend is aborted instead of
proceeding with clocks off. Pre-existing SPI-core behavior
(`__spi_mark_suspended()` even on queue-stop failure) is unchanged; this
patch does not make that worse, and it is the same contract every other
checked driver uses.
---
## PHASE 3: GIT HISTORY
**Step 3.1 — Blame (at `v6.18.53`)**
Record: ignored `spi_controller_suspend(host)` line from `2c40be6bc95bd`
(rename `master`→`host`, 2023). Original ignore-and-return-0 suspend is
`142168eba9dc5` (2013-12-04, “add bcm63xx HSSPI driver”), ancestor of
this tree.
**Step 3.2 — Fixes: tag**
Record: none. Buggy code has been in the driver since introduction; that
commit is in this tree.
**Step 3.3 — Related file history**
Record: `v6.18.53` already contains `b782a7cb0a424` (“disable clocks on
resume failure”, Can Peng, `Cc: stable`, `Fixes: 142168eba9dc`). That is
the resume-side counterpart of the same class of bug. No later revert of
this suspend fix.
**Step 3.4 — Author**
Record: Jiawen Liu; same-week series of the same one-line check on `spi-
bcm63xx.c`, `spi-bcm-qspi.c`, `spi-bcmbca-hsspi.c`. Not the driver
maintainer; **William Zhang**, who is, acked this patch.
**Step 3.5 — Prerequisites**
Record: none. Needs only `spi_controller_suspend()` (present since the
original driver) and the existing clock disable sequence. Standalone;
series 2/3 touches only this file. 1/3 is `spi-bcm63xx.c`, 3/3 is `spi-
bcm-qspi.c`.
---
## PHASE 4: MAILING LIST / EXTERNAL
**Step 4.1 — Original discussion**
Record: `b4 dig -c 3b5414d76a0381db77b9694537254af7acfa187a` → https://p
atch.msgid.link/tencent_B5A06807924A77C8690730EBF7A052AABE05@qq.com
`b4 dig -a`: **v1 only**, `[PATCH 2/3]`. Thread saved via `b4 dig -m`.
No NAK, no requested respin.
**Step 4.2 — Recipients (`b4 dig -w`)**
Record: `broonie@kernel.org`, `linux-spi@vger.kernel.org`,
`william.zhang@broadcom.com`, `kursad.oney@broadcom.com`,
`jonas.gorski@gmail.com`, `kamal.dasu@broadcom.com`, `bcm-kernel-
feedback-list@broadcom.com`. Right maintainers were on Cc.
**Step 4.3 — Bug report**
Record: no syzbot/bugzilla. Found by code review of ignored
`spi_controller_suspend()` returns.
**Step 4.4 — Series**
Record: 2/3 in a three-driver series. Each patch is a self-contained
per-driver check. William Zhang: `Acked-by` on this patch. Kamal Dasu:
`Reviewed-by` on 3/3 (`spi-bcm-qspi`). Mark Brown applied it.
**Step 4.5 — Stable list**
Record: lore.kernel.org search via WebFetch returned **403**. No stable-
thread evidence found. Absence of `Cc: stable` is expected for this
pipeline.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1 — Functions**
Record: `bcm63xx_hsspi_suspend()` only.
**Step 5.2 — Callers**
Record: wired through `SIMPLE_DEV_PM_OPS(bcm63xx_hsspi_pm_ops, ...)` /
`platform_driver.driver.pm`. Invoked from the PM core on **system
sleep**, not a syscall hot path. Requires `CONFIG_SPI_BCM63XX_HSSPI` and
`CONFIG_PM_SLEEP`.
**Step 5.3 — Callees**
Record: `spi_controller_suspend()` → `spi_stop_queue()` (can return
`-EBUSY`); then `clk_disable_unprepare()` on PLL and core clocks.
**Step 5.4 — Reachability**
Record: Yes, on suspend of BCM63xx/BCMBCA boards that use this
controller (`brcm,bcm6328-hsspi`, `brcm,bcmbca-hsspi-v1.0` in MIPS
bcm63xx and ARM/ARM64 bcmbca DT). Trigger is suspend while the SPI queue
stays busy for ~5 seconds.
**Step 5.5 — Similar patterns**
Record: same ignore-error bug in sibling Broadcom SPI drivers; same
return-check already used in many in-tree SPI drivers. Systematic, not a
one-off style tweak.
semcode MCP tools were not available; this used `git grep` and direct
reads of `spi.c` / the driver.
---
## PHASE 6: LOCAL TREE CROSS-CHECK
**Step 6.1 — Buggy code in this tree?**
Record: **Yes**, at `v6.18.53` `bcm63xx_hsspi_suspend()`:
```c
spi_controller_suspend(host);
clk_disable_unprepare(bs->pll_clk);
clk_disable_unprepare(bs->clk);
return 0;
```
Bug introduced 2013, present for the whole 6.18.y lifetime.
**Step 6.2 — Backport difficulty**
Record: **clean apply with +6 line offset** (`patch --dry-run` succeeded
against the `v6.18.53` file). Surrounding `remove()` differs from later
mainline; the suspend hunk itself matches.
**Step 6.3 — Related fixes already here?**
Record: resume-path clock cleanup (`b782a7cb0a424`) **is** in
`v6.18.53`. This suspend-path check is **not**. No alternate fix for the
ignored suspend error.
---
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1**
Record: **SPI driver** (`drivers/spi/spi-bcm63xx-hsspi.c`). Criticality:
**IMPORTANT** (platform SPI, often SPI flash on Broadcom CPE/BCA SoCs),
not core MM/VFS. Not staging.
**Step 7.2**
Record: Mature driver (2013), still maintained (William Zhang / Kursad
Oney / Jonas Gorski). Recent PM fix already landed in this stable
series.
---
## PHASE 8: IMPACT AND RISK
**Step 8.1 — Who is affected**
Record: users of `CONFIG_SPI_BCM63XX_HSSPI` on BCM63XX / BMIPS /
ARCH_BCMBCA (routers, gateways, BCA ARM64 boards). Not universal.
**Step 8.2 — Trigger**
Record: system suspend while `spi_stop_queue()` times out (`-EBUSY`
after ~5s of a non-empty queue or `ctlr->busy`). Unprivileged users
cannot hit it directly unless they can suspend. Timing-dependent; real
when a transfer is stuck or a long SPI flash op overlaps suspend.
**Step 8.3 — Failure mode**
Record: clocks gated while the controller may still be busy; PM is told
suspend succeeded. Possible bus hang, incomplete SPI transaction, or
SPI-flash corruption if a NOR/NAND op is in flight. Severity: **HIGH**
(hang / possible corruption on the error path), not a guaranteed oops on
every suspend.
**Step 8.4 — Risk/benefit**
Record: **Benefit: medium-high** for those platforms (correct suspend
abort, clocks stay on). **Risk: very low** (5 lines, error path only,
same pattern as peer drivers; resume counterpart already in this tree).
Ratio favors backport.
---
## PHASE 9: SYNTHESIS
**Step 9.1 — Evidence**
For:
- Real PM bug: ignored `-EBUSY`, then clock gate + success.
- Queued controller, so `spi_stop_queue()` can actually fail.
- Possible hang / in-flight SPI (including flash) with clocks off.
- 5-line, obviously correct, no new API.
- Acked by driver maintainer; applied by SPI maintainer.
- Resume-side analogue already in 6.18.53 as a `Cc: stable` fix.
- Applies to `v6.18.53` with only a line-number offset.
- Bug present since 2013 in this tree.
Against:
- No `Reported-by` / syzbot / crash log.
- Trigger is the uncommon queue-stop timeout, not every suspend.
- SPI core still marks the controller suspended on failure (pre-
existing; this patch does not fix or worsen that).
- Peripheral SoC driver, not core kernel.
Unresolved: lore.kernel.org HTML/WebFetch was 403; discussion recovered
via `b4 dig -m` instead.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — trivial check; acked by
maintainer; in mainline.
2. Real bug affecting users? **PASS** — wrong suspend result and clocks-
off while busy.
3. Important (crash/security/corruption/deadlock)? **PASS** — hang /
possible SPI-flash corruption if suspend proceeds after queue-stop
failure; same class as the resume fix already in this tree.
4. Small and contained? **PASS** — +5/−1, one function.
5. No new features/APIs? **PASS**.
6. Can apply to this tree? **PASS** — buggy code present; applies with
offset 6.
**Step 9.3 — Exception category**
Record: none (not a device ID / quirk / DT / build / docs exception). It
is a normal small bugfix.
**Step 9.4 — Decision**
This belongs in 6.18.y: it stops the driver from disabling HSSPI clocks
and reporting successful suspend when the SPI core could not stop the
queue. The matching resume cleanup is already here; this is the suspend-
side hole that lacked a `Cc: stable` tag.
---
## What the commit solves (for this tree)
On Broadcom BCM63xx/BCMBCA HSSPI, system suspend called
`spi_controller_suspend()` and ignored `-EBUSY` from `spi_stop_queue()`.
It then turned off the PLL and core clocks and returned 0, so the PM
core continued suspend while the controller could still be busy. That
can hang the bus or interrupt an in-flight SPI flash transfer. The fix
returns the error and leaves clocks enabled so suspend is aborted.
It meets the stable rules: real bug, important on the failure path, five
lines, no new API, applies to 6.18.53.
Risk vs benefit: very low risk, meaningful benefit on the platforms that
use this driver. No missing prerequisites.
---
## Verification
- **[Phase 1]** Parsed tags: author Jiawen Liu; Acked-by William Zhang;
SOB Mark Brown; Link to msgid; no Fixes/Reported-by/Cc-stable.
- **[Phase 2]** Diff: +5/−1 in `bcm63xx_hsspi_suspend()`; check
`spi_controller_suspend()` before `clk_disable_unprepare`.
- **[Phase 2]** `spi_controller_suspend()` in `drivers/spi/spi.c`: on
`ctlr->queued`, `spi_stop_queue()` can return `-EBUSY`; then
`__spi_mark_suspended()`; return `ret`.
- **[Phase 2]** `spi_stop_queue()`: 500 iterations ×
`usleep_range(10000, 11000)`, then `-EBUSY`.
- **[Phase 2]** Driver sets `transfer_one_message`, not `transfer`; core
sets `queued = true` (also true at `v6.18.53`).
- **[Phase 3]** `git blame` at `v6.18.53`: ignore-error suspend from
original `142168eba9dc5` (2013); `git merge-base --is-ancestor
142168eba9dc5 v6.18.53` → YES.
- **[Phase 3]** `git merge-base --is-ancestor
3b5414d76a0381db77b9694537254af7acfa187a v6.18.53` → NO (fix not in
6.18.53).
- **[Phase 3]** Resume counterpart `b782a7cb0a424` **is** ancestor of
`v6.18.53` (Can Peng, `Cc: stable`).
- **[Phase 3]** No revert of this change in `drivers/spi/spi-bcm63xx-
hsspi.c`.
- **[Phase 4]** `b4 dig -c 3b5414d76a0381…`: lore msgid above; v1 only;
William Zhang Acked-by; no NAK.
- **[Phase 4]** `b4 dig -w`: SPI list + Broadcom maintainers Cc’d.
- **[Phase 4]** `b4 dig -m /tmp/hsspi-thread.mbox`: 4 messages, series
2/3 standalone.
- **[Phase 4]** WebFetch of lore.kernel.org: 403; discussion taken from
b4 mbox instead.
- **[Phase 5]** Callers: `SIMPLE_DEV_PM_OPS` → platform PM; CONFIG
`SPI_BCM63XX_HSSPI` depends on `BCM63XX || BMIPS_GENERIC ||
ARCH_BCMBCA`.
- **[Phase 5]** DT: `brcm,bcm6328-hsspi` / `brcm,bcmbca-hsspi-v1.0` on
bcm63xx MIPS and bcmbca ARM/ARM64.
- **[Phase 6]** `git describe HEAD` / `make kernelversion`: 6.18.53.
- **[Phase 6]** `git show v6.18.53:drivers/spi/spi-bcm63xx-hsspi.c`:
still ignores suspend error.
- **[Phase 6]** `patch --dry-run` against that file: hunk succeeded at
line 895 (offset 6).
- **[Phase 7]** MAINTAINERS: “BROADCOM Broadband SoC High Speed SPI
Controller DRIVER”, M: William Zhang.
- **[Phase 8]** Failure mode verified from `spi.c` + driver suspend
path, not from a user crash report.
- **UNVERIFIED:** live reproduction of `spi_stop_queue()` timeout on BCM
hardware; stable@ lore thread (HTTP 403).
The fix is small, surgical, present as a real bug in this 6.18.53 tree,
and meets the stable criteria.
**YES**
drivers/spi/spi-bcm63xx-hsspi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index b8bf8b5be2cf7..719f0dfd96c96 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -895,8 +895,12 @@ static int bcm63xx_hsspi_suspend(struct device *dev)
{
struct spi_controller *host = dev_get_drvdata(dev);
struct bcm63xx_hsspi *bs = spi_controller_get_devdata(host);
+ int ret;
+
+ ret = spi_controller_suspend(host);
+ if (ret)
+ return ret;
- spi_controller_suspend(host);
clk_disable_unprepare(bs->pll_clk);
clk_disable_unprepare(bs->clk);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18] spi: fsl-dspi: clean up after failed suspend and resume
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx-hsspi: return error from failed controller suspend Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.15] spi: cadence-qspi: Fix status polling with octal DTR chips Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.6] spi: bcmbca-hsspi: return error from failed controller suspend Sasha Levin
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Jiawen Liu, Mark Brown, Sasha Levin, olteanv, linux-spi, imx,
linux-kernel
From: Jiawen Liu <1298662399@qq.com>
[ Upstream commit 2543355f3da56a297e3803ccb5d29f4dce5f18f1 ]
dspi_suspend() disabled the IRQ before spi_controller_suspend(),
but ignored a suspend failure and kept tearing the device down.
Restore the IRQ and return the error if suspend fails.
dspi_resume() also left the clock prepared if controller resume or
hardware init failed. Route those failures through clock cleanup.
Signed-off-by: Jiawen Liu <1298662399@qq.com>
Link: https://patch.msgid.link/tencent_427FA55E3D59112524886E9C931CA0F92F06@qq.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I compared the pre-image to v6.18.53: the buggy suspend/resume path is
there unchanged, so this is a standalone fix for this tree.
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
- Record: `[spi: fsl-dspi:] [clean up] [restore IRQ / clocks after
failed suspend and resume]`
- Subsystem is Freescale DSPI. The verb is “clean up”, but the body
describes ignored errors and missing rollback.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
- Record:
- `Signed-off-by: Jiawen Liu <1298662399@qq.com>` (author)
- `Link: https://patch.msgid.link/tencent_427FA55E3D59112524886E9C931C
A0F92F06@qq.com`
- `Signed-off-by: Mark Brown <broonie@kernel.org>` (SPI maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence of those last two is expected for
this review)
- Pipeline-added `Signed-off-by: Sasha Levin` / `[ Upstream commit
2543355f3da5... ]` ignored per instructions
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
- Record: **Bug:** `dspi_suspend()` calls `disable_irq()` then
`spi_controller_suspend()` but ignores a non-zero return and still
disables the clock and selects the sleep pin state. **Symptom:** on a
failed controller suspend the IRQ stays off, clocks are gated, pins go
to sleep, and suspend is reported as success. **Resume bug:**
`clk_prepare_enable()` is not undone if `spi_controller_resume()` or
`dspi_init()` fails. **Root cause:** missing error checks and missing
rollback. No version range in the message.
**Step 1.4: DETECT HIDDEN BUG FIXES**
- Record: Yes. “Clean up” is error-path recovery: re-enable IRQ and
return the error on suspend failure; `clk_disable_unprepare()` on
resume failure. That is a leak/imbalance fix, not style.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
- Record: `drivers/spi/spi-fsl-dspi.c` only; **+18 / −3**. Functions:
`dspi_suspend()`, `dspi_resume()`. Single-file surgical PM error-path
fix.
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
- Record:
- **suspend before:** disable IRQ → ignore `spi_controller_suspend()`
→ always `clk_disable_unprepare()` → sleep pinctrl → return 0.
- **suspend after:** disable IRQ → if `spi_controller_suspend()`
fails, `enable_irq()` and return `ret` (clocks/pinctrl untouched).
- **resume before:** enable clock → ignore `spi_controller_resume()` →
on `dspi_init()` failure return with clock still prepared.
- **resume after:** those failures go to `disable_clk`
(`clk_disable_unprepare()` + return `ret`). IRQ is still enabled
only on the success path.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
- Record: **Error-path / resource-leak / IRQ imbalance.**
`spi_controller_suspend()` can return `-EBUSY` from `spi_stop_queue()`
after a 5s wait (`drivers/spi/spi.c`). Continuing then gates the clock
under a live controller and leaves the IRQ disabled. Resume can leak a
prepared clock.
**Step 2.4: ASSESS THE FIX QUALITY**
- Record: Minimal and obviously correct; matches other SPI drivers that
already check these returns. Regression risk is low: only error paths
change. Idle suspend still returns 0 after stop-queue succeeds. Pre-
existing IRQ-before-stop-queue ordering is not introduced here
(Sashiko noted it as pre-existing). Resume still does not undo
`spi_controller_resume()` on later `dspi_init()` failure;
`dspi_init()` only fails on invalid `trans_mode` after a successful
probe, so that path is essentially dead.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
- Record: On v6.18.53, suspend/resume blame is `c5412ec5f68773`
(“Reinitialize DSPI regs after resuming for S32G”, 2025-05-22), which
kept the ignore-error pattern. That pattern already existed before
S32G (`git show c5412ec5f68773^`). Original driver `349ad66c0ab0b`
(Vybrid VF610) already ignored `spi_master_suspend()` and always
disabled the clock. Bug is old, not post-6.18.
**Step 3.2: FOLLOW THE FIXES: TAG**
- Record: No `Fixes:` tag. Not applicable beyond that.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
- Record: Related: `3d87b613d6a3c` (“Fix external abort on interrupt in
resume or exit paths”) added `disable_irq()` in suspend because a
shared IRQ after clocks-off caused an external abort on Vybrid. That
makes restoring the IRQ on suspend failure required. `9bd77a9ce31dd`
only fixed a NULL drvdata in PM. This patch is standalone; later
autosel `b6d8099e44987` only switches PM macros.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
- Record: Jiawen Liu is not the SPI maintainer. Same series on this
tree: `spi: atcspi200`, `spi: bcmbca-hsspi`, `spi: bcm-qspi`, `spi:
bcm63xx-hsspi`, `spi: bcm63xx`, plus `spi: nxp-fspi` probe RPM. Each
driver is independent. Mark Brown applied the series.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
- Record: No new helpers. Uses `spi_controller_suspend`/`resume`,
`enable_irq`/`disable_irq`, `clk_disable_unprepare` already in 6.18.
Standalone. Series 3/4 does not need 1/4 or 2/4 for this file.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
- Record: `b4 dig -c 2543355f3da5` → https://patch.msgid.link/tencent_42
7FA55E3D59112524886E9C931CA0F92F06@qq.com
- Series: v1 only, `[PATCH 3/4]`. Mark Brown applied all four to
`broonie/spi.git for-7.3`. No NAK on this patch. Sashiko flagged two
**pre-existing** ordering issues (IRQ before stop-queue; resume queue
before `dspi_init()`), not regressions from this diff. No reviewer
`Cc: stable`. Direct lore fetch returned 403; thread recovered via `b4
dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
- Record: `b4 dig -w`: To Mark Brown; Cc linux-spi, linux-kernel, linux-
arm-kernel, imx@, NXP (olteanv, han.xu, haibo.chen). No human
Reviewed-by on this patch. Maintainer applied it.
**Step 4.3: SEARCH FOR THE BUG REPORT**
- Record: No Reported-by / bugzilla / syzbot. Bug is from code
inspection of ignored returns.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
- Record: 1/4 atcspi200, 2/4 bcmbca-hsspi, 3/4 this, 4/4 nxp-fspi
(different bug: probe RPM). This hunk does not depend on the others.
Frank Li replied only to 4/4.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
- Record: lore.kernel.org/stable search via WebFetch returned 403.
UNVERIFIED whether stable@ discussed this SHA. No in-tree evidence it
was rejected.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
- Record: `dspi_suspend()`, `dspi_resume()`. Callees of interest:
`spi_controller_suspend()`, `spi_controller_resume()`, `dspi_init()`.
**Step 5.2: TRACE CALLERS**
- Record: Only `SIMPLE_DEV_PM_OPS(dspi_pm, dspi_suspend, dspi_resume)`
on v6.18.53 → system-sleep PM for this platform device. Not a syscall
hot path; triggered by suspend/resume on i.MX / Vybrid / LS1021A / S32
/ ColdFire DSPI.
**Step 5.3: TRACE CALLEES**
- Record: `spi_controller_suspend()` → `spi_stop_queue()` (up to 500 ×
~10ms, then `-EBUSY`) then `__spi_mark_suspended()`.
`dspi_transfer_one_message()` waits on `dspi->xfer_done` when
`dspi->irq` is set; completion is from `dspi_interrupt()`. DMA mode
waits on DMA completions, not that IRQ.
**Step 5.4: FOLLOW THE CALL CHAIN**
- Record: Userspace/system suspend → driver PM → `dspi_suspend()`.
Reachable whenever this hardware is present and `CONFIG_PM_SLEEP`. In-
flight IRQ-mode transfer at suspend can make `spi_stop_queue()` fail;
that is the interesting path.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
- Record: Many SPI drivers already check `spi_controller_suspend()`
(omap2-mcspi, atmel, qup, rockchip, stm32, …). Same author fixed the
ignored-return pattern in several Broadcom SPI drivers. Systematic,
not one-off.
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
- Record: Local tree is **6.18.53** (`git describe HEAD` →
`v6.18.53-5049-g0f0bdc2b0dffb`, `VERSION=6 PATCHLEVEL=18
SUBLEVEL=53`). At **v6.18.53**, `dspi_suspend()` still ignores the
suspend return and `dspi_resume()` still returns from `dspi_init()`
failure without `clk_disable_unprepare()`. Candidate `1248ec26ed99f`
is **not** an ancestor of `v6.18.53`. Bug present.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
- Record: Pre-image on v6.18.53 matches the patch (including `#ifdef
CONFIG_PM_SLEEP` / `SIMPLE_DEV_PM_OPS`). Expected apply: **clean**.
`git apply --check` on current autosel HEAD fails only because the
candidate is already sitting on this working branch.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
- Record: `3d87b613d6a3c` (IRQ disable in suspend) **is** in v6.18.53.
No other fix for ignored `spi_controller_suspend()` on this file.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
- Record: `drivers/spi/spi-fsl-dspi.c`. **IMPORTANT** (common
NXP/Freescale SPI; not core mm/vfs). `SPI_FSL_DSPI` depends on
`ARCH_MXC || ARCH_NXP || M5441x || COMPILE_TEST`. Kconfig: S32, VF610,
LS1021A, ColdFire.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
- Record: File still gets platform and DMA work (S32G series in 2025).
PM error handling itself is old and stale.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
- Record: Driver-specific: boards with Freescale DSPI that suspend.
Embedded/i.MX/Layerscape/S32 population, not all Linux systems.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
- Record: System suspend while `spi_stop_queue()` cannot drain
(busy/queued messages for ~5s). IRQ-mode in-flight transfer is a
realistic trigger because `disable_irq()` runs first and
`dspi_transfer_one_message()` uses
`wait_for_completion(&dspi->xfer_done)`. Unprivileged users do not
call this directly; a suspend with SPI activity can. Resume clock
leak: `spi_controller_resume()` `-EBUSY` or `dspi_init()` `-EINVAL`
(latter essentially unreachable after probe).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
- Record: Failed suspend that still gates clocks: possible MMIO abort /
hang (this driver already hit “external abort on non-linefetch” when
the IRQ ran after clocks off — `3d87b613d6a3c`). IRQ left disabled
while the device is still active: SPI completions never fire → hung
transfers. Resume clock leak: extra power / unbalanced clk prepare.
**Severity: HIGH** on the suspend path (hang / abort), **MEDIUM** on
the resume leak.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
- Record: **Benefit:** high for DSPI suspend users — fail suspend
instead of disabling clocks/IRQ incorrectly. **Risk:** low — 21 lines,
error paths only, no API/struct change. If stop-queue fails, aborting
suspend is the intended PM policy.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real ignored-error bug: teardown after failed
`spi_controller_suspend()`.
- IRQ not restored → hung SPI (completion in `dspi_interrupt()`).
- Clock gated while the controller may still be busy → abort/hang; same
class as the already-stable `3d87b613d6a3c` abort fix.
- Resume path clock leak on error.
- Small, local, obviously correct; SPI maintainer applied it.
- Buggy code is in v6.18.53; no extra prerequisites.
- Same class of SPI PM error-path fixes already common in-tree.
AGAINST:
- No user/syzbot report.
- Worded as “clean up”; failure is an error path, not every suspend.
- `dspi_init()` resume failure is almost unreachable.
- Sashiko pre-existing ordering notes (not introduced here).
- Incomplete undo of `spi_controller_resume()` if `dspi_init()` failed
(dead in practice).
UNRESOLVED:
- lore.kernel.org HTTP 403; stable@ discussion not independently fetched
(thread was recovered with b4).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — mechanical error-return
handling; applied by Mark Brown.
2. Fixes a real bug that affects users? **PASS** — IRQ/clock teardown on
failed suspend; clk leak on resume.
3. Important issue? **PASS** — hang / possible bus abort, not a style
nit.
4. Small and contained? **PASS** — +18/−3, one file.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — v6.18.53 pre-image matches.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
- Record: Not a device-ID/quirk/DT/build/doc exception. Qualifies as a
normal bugfix.
**Step 9.4: DECISION**
This should be backported to **this 6.18.53 tree**. The important part
is suspend: do not disable clocks and leave the IRQ off when
`spi_controller_suspend()` fails. That is a real stability bug on a
production NXP SPI controller, small enough for stable, and it applies
cleanly here.
### What the commit solves for stable users
On Freescale DSPI, suspend currently disables the IRQ, then ignores
`spi_controller_suspend()`. If the message queue does not stop
(`-EBUSY`), the driver still turns off the clock and puts pins to sleep
while reporting success. The IRQ stays disabled. That can hang in-flight
transfers (`wait_for_completion` never completes) or MMIO-abort if the
hardware is still live. Resume can also leave `clk_prepare_enable()`
unbalanced. The patch stops teardown on suspend failure and unwinds the
clock on resume failure.
### Risk vs benefit
Benefit is preventing a broken/hung SPI device and possible abort after
a failed controller suspend. Risk is confined to those error returns;
successful idle suspend is unchanged.
### Concerns
No extra commits are required. Do not confuse autosel already having
this SHA with “already selected for 6.18.y”; `v6.18.53` still has the
bug.
Verification:
- [Phase 1] Subject/body/tags from `git log -1` / `git show
1248ec26ed99f`: no Fixes/Reported-by; Link msgid; SOB Jiawen Liu +
Mark Brown
- [Phase 2] Diff: +18/−3 in `dspi_suspend`/`dspi_resume` only
- [Phase 3] `git blame` on v6.18.53: `c5412ec5f68773`; ignore-error PM
since `349ad66c0ab0b`; `3d87b613d6a3c` in this tree
- [Phase 3] `git merge-base --is-ancestor 1248ec26ed99f v6.18.53` → no;
ancestor of HEAD (autosel) → yes
- [Phase 3] Author series: several independent SPI PM error-path patches
- [Phase 4] `b4 dig -c 2543355f3da5`: patch 3/4 v1, lore msgid, applied
to broonie for-7.3; Sashiko pre-existing notes only; no NAK
- [Phase 4] WebFetch lore/stable: 403 — UNVERIFIED stable@ thread
- [Phase 5] `spi_controller_suspend` → `spi_stop_queue` `-EBUSY`; DSPI
IRQ path `wait_for_completion` / `dspi_interrupt`; PM ops only callers
- [Phase 6] `git describe HEAD` / Makefile: 6.18.53; v6.18.53 file
matches patch pre-image
- [Phase 7] `SPI_FSL_DSPI` on ARCH_MXC/NXP
- [Phase 8] Failure: hung SPI / possible external abort; severity HIGH
on suspend error path
**YES**
drivers/spi/spi-fsl-dspi.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 83ea296597e94..1283a193b6fcc 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1464,10 +1464,18 @@ static int dspi_init(struct fsl_dspi *dspi)
static int dspi_suspend(struct device *dev)
{
struct fsl_dspi *dspi = dev_get_drvdata(dev);
+ int ret;
if (dspi->irq)
disable_irq(dspi->irq);
- spi_controller_suspend(dspi->ctlr);
+
+ ret = spi_controller_suspend(dspi->ctlr);
+ if (ret) {
+ if (dspi->irq)
+ enable_irq(dspi->irq);
+ return ret;
+ }
+
clk_disable_unprepare(dspi->clk);
pinctrl_pm_select_sleep_state(dev);
@@ -1485,12 +1493,15 @@ static int dspi_resume(struct device *dev)
ret = clk_prepare_enable(dspi->clk);
if (ret)
return ret;
- spi_controller_resume(dspi->ctlr);
+
+ ret = spi_controller_resume(dspi->ctlr);
+ if (ret)
+ goto disable_clk;
ret = dspi_init(dspi);
if (ret) {
dev_err(dev, "failed to initialize dspi during resume\n");
- return ret;
+ goto disable_clk;
}
dspi_set_mtf(dspi);
@@ -1499,6 +1510,10 @@ static int dspi_resume(struct device *dev)
enable_irq(dspi->irq);
return 0;
+
+disable_clk:
+ clk_disable_unprepare(dspi->clk);
+ return ret;
}
#endif /* CONFIG_PM_SLEEP */
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-5.15] spi: cadence-qspi: Fix status polling with octal DTR chips
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (6 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] spi: fsl-dspi: clean up after failed suspend and resume Sasha Levin
@ 2026-10-19 13:52 ` Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.6] spi: bcmbca-hsspi: return error from failed controller suspend Sasha Levin
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:52 UTC (permalink / raw)
To: patches, stable
Cc: Miquel Raynal, Mark Brown, Sasha Levin, linux-spi, linux-kernel
From: Miquel Raynal <miquel.raynal@bootlin.com>
[ Upstream commit fd99864b0ac936e870c7ae28354c5cd3dad5efd0 ]
Most SPI NOR chips today use 0x05 as ReaD Status Register (RDSR)
opcode. After almost every transaction, the core will wait for the said
transation to be digested by the chip by polling its status
register. This mechanism has been automated by certain chips, such as
the Cadence QSPI controller. Unfortunately, the feature has been
disabled for various (relevant) reasons. Disabling this feature has been
done by setting WRITE_COMPLETION_CTRL_REG.DISABLE_POLLING_FLD, keeping
the rest of the register intact. In this same register, there is also a
field named WRITE_COMPLETION_CTRL_REG.OPCODE_FLD, which by default also
contains 0x05, as primary RDSR opcode.
As a result, both the SPI NOR and SPI NAND cores, primary users through
spi-mem of the spi_mem_ops interface, do send their own RDSR. In this
driver, such command goes through one of the fifth possible data path
specifically crafted for short register-like commands, named STIG. In
STIG mode, the opcode of the operation is written to
OSPI_FLASH_CMD_CTRL_REG. This means that the opcode field of STIG
configuration and the opcode field of the automatic polling
configuration contain the same value.
Unfortunately, the TRM explains in two different places that this
configuration is forbidden, without explaining why it is problematic:
$12.4.2.4.11 OSPI Software-Triggered Instruction Generator (STIG)
```
OSPI_FLASH_CMD_CTRL_REG[31-24] CMD_OPCODE_FLD bits should be set
different than OSPI_DEV_INSTR_RD_CONFIG_REG[7-0] RD_OPCODE_NON_XIP_FLD
and OSPI_DEV_INSTR_WR_CONFIG_REG[7-0] WR_OPCODE_FLD.
```
and
$12.4.2.5.2 Configuring the OSPI Controller for Optimal Use
```
When using the OSPI Controller, the opcodes in
OSPI_DEV_INSTR_RD_CONFIG_REG[7-0] RD_OPCODE_NON_XIP_FLD,
OSPI_DEV_INSTR_WR_CONFIG_REG[7-0] WR_OPCODE_FLD and
OSPI_WRITE_COMPLETION_CTRL_REG[7-0] OPCODE_FLD bit fields shall not
match the opcode in the OSPI_FLASH_CMD_CTRL_REG[31-24] CMD_OPCODE_FLD
bit field.
```
For some reasons, this is only unveiled now when testing in ODTR
mode. Plain SDR modes work flawlessly with the same chips. One possible
reason for this, is that the internal data path in ODTR mode is
very different due to the extra opcode extension/repetition which may be
interpreted differently (this is a wild guess). For sure there is an
interference, as even the TRM states that there is some magic happening:
$12.4.2.4.13 OSPI Command Translation
```
The WREN and the RDSR device instructions are the only ones that are
sent by the controller under the hood.
```
I cannot probe the bus when the problem arises, so I don't know exactly
what is sent on the bus. Practically speaking, the chip was driving the
lines low (reading all 0s), which is not the usual default state when
the chip is not driving the lines (all 1s). Other people on the mailing
list claimed successfully using ODTR chips with this controller. My take
here is that either they were using a variant of the IP without write
completion at all (there is a quirk for it), or they were using chips
slightly more flexible about their opcode sequency or maybe with
different addr/dummy requirements.
Clearing the WRITE_COMPLETION_CTRL_REG.OPCODE_FLD while disabling write
completion polling entirely seems the best compromise.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://patch.msgid.link/20261001-winbond-spi-next-cadence-rdsr-fix-v1-1-ec5ff594ba5d@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This tree is **6.18.57**. The candidate is a 3-line Cadence QSPI
programming fix: stop leaving the default RDSR opcode in
`WRITE_COMPLETION_CTRL` after auto-poll is disabled, which the TRM
forbids and which breaks octal DTR status polling.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[spi: cadence-qspi:] [Fix] [status polling fails with octal
DTR chips because WRITE_COMPLETION_CTRL.OPCODE_FLD still holds 0x05]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** none
- **Acked-by:** none
- **Link:** https://patch.msgid.link/20261001-winbond-spi-next-cadence-
rdsr-fix-v1-1-ec5ff594ba5d@bootlin.com
- **Cc: stable:** none (expected)
- **Signed-off-by:** Miquel Raynal (author; MTD/NAND maintainer); Mark
Brown (SPI maintainer, applied the patch)
- Ignore: `[ Upstream commit fd99864b0ac936e870c7ae28354c5cd3dad5efd0 ]`
and `Signed-off-by: Sasha Levin` (pipeline)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** Auto-HW write-completion polling was disabled by OR-ing
`DISABLE_POLLING_FLD` into `WRITE_COMPLETION_CTRL`, which **kept the
reset-default OPCODE_FLD = 0x05 (RDSR)**. SPI NOR/NAND then issue
their own RDSR via STIG (`OSPI_FLASH_CMD_CTRL_REG`), so STIG opcode
and completion-opcode both become 0x05.
- **TRM:** Cadence OSPI TRM §12.4.2.4.11 and §12.4.2.5.2 forbid STIG
`CMD_OPCODE_FLD` matching `WRITE_COMPLETION_CTRL.OPCODE_FLD` (and the
read/write instruction opcodes).
- **Symptom:** In **octal DTR**, the flash drives the bus low (reads as
all 0s). SDR with the same chips works. Status 0 means WIP=0, so
software can treat a still-busy flash as ready.
- **Root cause (author):** controller still has “under the hood”
WREN/RDSR translation; ODTR’s extra opcode extension/repetition likely
makes the illegal opcode collision actually fire.
- **Versions:** none named. Introduced by the 2021 auto-poll disable,
not by new 7.x code.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** Subject and body describe a real hardware-
programming bug. Not cleanup/refactor.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/spi/spi-cadence-quadspi.c` (**+1 / −2**)
- Function: `cqspi_write_setup()`
- Scope: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `reg = readl(WR_COMPLETION_CTRL); reg |=
DISABLE_AUTO_POLL; writel(...)` — RMW preserves OPCODE_FLD=0x05 and
poll-count/delay/bit fields.
- **After:** `reg = CQSPI_REG_WR_DISABLE_AUTO_POLL; writel(...)` —
writes only BIT(14), so OPCODE_FLD becomes 0 (and other poll fields
0).
- Path: **write setup** for every spi-mem data write when
`cqspi->wr_completion` is true (default). Direct-mode writes stay
disabled, unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** hardware workaround / logic correctness (illegal
controller opcode collision)
- **Mechanism:** disabled auto-poll still advertised RDSR 0x05; STIG
RDSR used the same opcode; TRM-forbidden config; ODTR status reads
return 0
- **Fix:** clear OPCODE_FLD while keeping poll disabled, so STIG 0x05 no
longer matches
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** minimal and aligned with the TRM. Polling was already
disabled; remaining fields only matter when polling is on.
`CQSPI_NO_SUPPORT_WR_COMPLETION` platforms (SoCFPGA, Mobileye) never
write this register.
- **Regression risk:** **low**. SDR already worked with poll disabled;
changing opcode from 0x05 to 0 while poll is off is what the TRM
wants. Slightly broader than clearing only bits [7:0], but with
`DISABLE_POLLING` set that is harmless.
- No API/data-structure/locking changes.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Auto-poll disable + RMW: **`9cb2ff11171264`** (“spi: cadence-quadspi:
Disable Auto-HW polling”, Apurva Nandan, 2021-07-13) — present since
~v5.15
- `if (cqspi->wr_completion)` gate: **`98d948eb833104`** (2021-11)
- DTR support: **`f453f293979fb`** (2021-01-06)
- The RMW was already used for DTR-only in `f453f293979fb`;
`9cb2ff11171264` extended it to all writes. Bug is years old in this
tree.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Buggy pattern comes from `9cb2ff11171264`,
which **is** in this tree (`git merge-base --is-ancestor` succeeded).
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent cadence-qspi work is clocks, PM, DMA, DT parse —
unrelated. Independent of later ODTR opcode-filter commit `bee085476d27`
and its revert (neither is in this tree). **Standalone.**
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Miquel Raynal maintains MTD/NAND/SPI-NAND and contributes
regularly to spi-mem/cadence-qspi. Nearby work is DTR/spi-mem, not a
hidden multi-patch dependency. This submission is **[1/1]**.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Uses only `CQSPI_REG_WR_DISABLE_AUTO_POLL`,
`cqspi->wr_completion`, and `cqspi_write_setup()` — all present at
**v6.18.57**. `git diff v6.18.57 a1536ec7381c3^ -- drivers/spi/spi-
cadence-quadspi.c` is **empty**: the hunk applies cleanly to this
series.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c fd99864b0ac9`: https://patch.msgid.link/20261001-winbond-
spi-next-cadence-rdsr-fix-v1-1-ec5ff594ba5d@bootlin.com
- `b4 dig -a`: **v1 only**, single patch `[PATCH]` / Mark Brown `[1/1]`
- Thread: author cover + Mark Brown “Applied to spi.git **for-7.3**”
- **No NAK, no review objections, no reviewer Cc: stable**
- Cover note: issue is subtle; explanation is long on purpose
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): Mark Brown (applied), Thomas Petazzoni, Santhosh
Kumar K / Vignesh Raghavendra (TI, cadence-qspi), Steam Lin (Winbond),
linux-spi. Maintainer-applied; no extra Reviewed-by.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Author reproduced on real ODTR hardware (bus
stuck low / reads 0). Winbond engineer CC’d. Others on-list said ODTR
worked — author attributes that to IP without write-completion (quirk)
or more tolerant chips.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Series name `winbond-spi-next-cadence-rdsr-fix` is **one
patch**. Not 2/3 of a stack. Later opcode-filter revert is a different
bug and not required here.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: WebSearch of lore.kernel.org/stable for this subject found
**no** stable discussion (and no documented reason to skip it).
---
## PHASE 5: CODE SEMANTIC ANALYSIS
semcode MCP is **not available**; used grep/read.
**Step 5.1: KEY FUNCTIONS**
Record: **`cqspi_write_setup()`** only.
**Step 5.2: TRACE CALLERS**
Record:
- `cqspi_write_setup` ← `cqspi_write` ← `cqspi_mem_process` ←
`cqspi_exec_mem_op` (`spi_controller_mem_ops.exec_op`)
- After a data write, spi-nor/spi-nand poll RDSR via
**`cqspi_command_read()` (STIG)** when the op has no address or is a
small register read (`nbytes <= STIG max`).
- Context: process context, MTD/spi-mem, not IRQ.
**Step 5.3: TRACE CALLEES**
Record: `cqspi_enable_dtr`, `writel` of WR/RD_INSTR and
WR_COMPLETION_CTRL, then size register. No alloc/locks.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace/MTD write/erase → spi-nor `spi_nor_octal_dtr_write` /
page program → exec_op write → `cqspi_write_setup` programs
WR_COMPLETION_CTRL → `spi_nor_read_sr()` issues RDSR (0x05; in 8-8-8 DTR
also dummy/addr and 2 data bytes) → STIG. **Reachable on every flash
write** on Cadence OSPI with wr_completion. Not a syscall UAF, but
boot/rootfs-on-OSPI is a common embedded path.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `WR_COMPLETION_CTRL` is only written here. Platforms with
`CQSPI_NO_SUPPORT_WR_COMPLETION` skip it (SoCFPGA, Mobileye EyeQ5).
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** At **v6.18.57** `cqspi_write_setup()` still does the
RMW. Tree: `git describe HEAD` = **v6.18.57-155-gcdf239ca885b2**, `make
kernelversion` = **6.18.57**. DTR (`cqspi_mem_caps.dtr = true`,
`cqspi_enable_dtr`) and octal (`CQSPI_SUPPORTS_OCTAL` on TI AM654 and
Versal) are here. spi-nor has `SNOR_PROTO_8_8_8_DTR` and
`spi_nor_read_sr()` DTR RDSR.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** to v6.18.57. Surrounding function matches the
hunk exactly.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No other commit in this tree fixes OPCODE_FLD vs STIG. This
candidate is the first.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **drivers/spi** Cadence Quad SPI (`CONFIG_SPI_CADENCE_QUADSPI`).
**IMPORTANT** for embedded: TI K3 OSPI (`ti,am654-ospi`), AMD/Xilinx
Versal OSPI, plus JH7110, Intel LGM, etc. Often **boot/root filesystem
flash**. Not core mm/VFS, not staging.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (PM, clocks, DMA) but this programming
sequence has been stable since 2021.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific**, especially **octal DTR** on controllers
that **have** write-completion (TI AM654/K3 OSPI, Versal OSPI).
SoCFPGA/Mobileye skip the register. SDR of the same chips is reported
working.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Flash write/erase then software RDSR poll in **8-8-8 DTR**.
Needs octal-capable controller + ODTR NOR/NAND. Not unprivileged-attack
surface; typically MTD from kernel/root. Timing-independent once ODTR is
on — author reproduced it.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- **Failure:** RDSR returns 0 (lines held low). WIP=0 can look like “not
busy” while a program/erase is still running → **failed writes or
flash corruption**. Or ops just fail.
- **Severity: HIGH** (flash integrity on OSPI-boot systems). Not a KASAN
crash, but real storage failure.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit: high** for ODTR-on-Cadence-QSPI users (TI K3, Versal, …)
- **Risk: very low** (1–3 lines, poll already off, quirk skips broken
IP)
- **Ratio: strongly favors backport**
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real, reproduced bug: ODTR status poll returns all 0s
- TRM forbids this opcode collision
- Possible **write-completion mis-detect / flash corruption**
- 3-line, obviously correct, no new API
- Code has been wrong in this tree since 2021; DTR/octal already exist
- Applies cleanly to v6.18.57
- Author is MTD maintainer; SPI maintainer applied it
- Hardware-programming workaround (stable exception class)
**AGAINST:**
- No Fixes:/Cc: stable (not a negative per rules)
- No extra reviewers; applied to **for-7.3** not as an -rc hotfix
- Author is not 100% sure of the internal ODTR datapath (“wild guess”)
- Hits ODTR more than SDR; fewer machines than a core-mm bug
- Writes the whole register, not only OPCODE_FLD (still safe with poll
disabled)
**UNRESOLVED:** Exact on-wire ODTR sequence (author could not probe). No
stable-list thread. Unverified whether every Cypress Semper setup
already failed the same way (Winbond exposed it; TRM still applies).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — TRM + reproduced ODTR
failure; maintainer applied
2. Fixes a real bug that affects users? **PASS** — OSPI flash status
poll broken in ODTR
3. Important issue? **PASS** — HIGH: failed program/erase or false
“ready” (corruption risk)
4. Small and contained? **PASS** — +1/−2, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — v6.18.57 has the RMW; empty
diff vs parent file
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Hardware workaround** for Cadence OSPI write-completion vs
STIG RDSR. Not a new driver/API.
**Step 9.4: MAKE YOUR DECISION**
This belongs on 6.18.y: the buggy RMW is in this tree, octal DTR is
supported, the fix is tiny, and the failure is flash status polling — a
data-integrity problem on platforms that boot from OSPI.
---
### Problem and why it matters here
`cqspi_write_setup()` disables controller auto-poll but left
`WRITE_COMPLETION_CTRL.OPCODE_FLD` at the reset default **0x05**.
Software RDSR also uses 0x05 in STIG. The Cadence TRM forbids that
match. In octal DTR, status reads come back 0, so WIP can look clear
while the chip is still busy. On TI AM654/K3 and Versal OSPI that is
boot/storage flash, not a niche sysfs knob.
### Stable rules / risk
Surgical, no new ABI, applies to **this** 6.18.57 tree without
prerequisites. Benefit (correct ODTR status, avoid bad write completion)
dwarfs the risk of writing BIT(14) instead of RMW.
### Verification
- **[Phase 1]** Tags: Link to msgid `20261001-winbond-spi-next-cadence-
rdsr-fix-v1-1-…`; SOB Miquel Raynal + Mark Brown; no Fixes/Reported-
by/Cc: stable
- **[Phase 2]** Diff: `cqspi_write_setup()` RMW → `reg =
CQSPI_REG_WR_DISABLE_AUTO_POLL` (+1/−2)
- **[Phase 3]** blame: RMW from `9cb2ff11171264` (2021); wr_completion
gate `98d948eb833104`; DTR `f453f293979fb`; all ancestors of HEAD
- **[Phase 3]** `git merge-base --is-ancestor 9cb2ff11171264 HEAD` → yes
- **[Phase 3]** `git diff v6.18.57 a1536ec7381c3^ -- drivers/spi/spi-
cadence-quadspi.c` → 0 lines (clean apply)
- **[Phase 4]** `b4 dig -c fd99864b0ac9`: lore/patch.msgid.link match;
`-a` v1 only; `-w` Broonie/TI/Winbond/linux-spi; mbox: Applied
**for-7.3**, no NAK
- **[Phase 4]** lore.kernel.org WebFetch → 403; recovered via `b4 -m` +
patchew
- **[Phase 4]** stable lore search: no hits
- **[Phase 5]** callers: `cqspi_write` → `cqspi_mem_process` →
`cqspi_exec_mem_op`; RDSR via `cqspi_command_read` STIG;
`spi_nor_read_sr()` DTR path in `drivers/mtd/spi-nor/core.c`
- **[Phase 6]** `git describe HEAD` = v6.18.57-155-gcdf239ca885b2; `make
kernelversion` = 6.18.57; v6.18.57 still has the RMW
- **[Phase 6]** octal platdata: `am654_ospi`, `versal_ospi`,
`versal2_ospi` have `CQSPI_SUPPORTS_OCTAL` and wr_completion;
SoCFPGA/Mobileye set `CQSPI_NO_SUPPORT_WR_COMPLETION`
- **[Phase 7]** `CONFIG_SPI_CADENCE_QUADSPI`; compatibles include
`ti,am654-ospi`, `xlnx,versal-ospi-1.0`
- **[Phase 8]** Failure: ODTR RDSR all 0s → false ready / failed flash
ops; severity HIGH
- **UNVERIFIED:** on-wire ODTR collision (author could not probe);
whether every Cypress Semper board already failed identically
- **semcode:** namespace not available
The patch is a small, correct hardware programming fix for a real flash-
status bug that exists in this 6.18.y tree.
**YES**
drivers/spi/spi-cadence-quadspi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 0a32e28eefd51..441426bc232ef 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1058,8 +1058,7 @@ static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
* care of polling the status register.
*/
if (cqspi->wr_completion) {
- reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
- reg |= CQSPI_REG_WR_DISABLE_AUTO_POLL;
+ reg = CQSPI_REG_WR_DISABLE_AUTO_POLL;
writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
/*
* DAC mode require auto polling as flash needs to be polled
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] spi: bcmbca-hsspi: return error from failed controller suspend
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (7 preceding siblings ...)
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.15] spi: cadence-qspi: Fix status polling with octal DTR chips Sasha Levin
@ 2026-10-19 13:52 ` Sasha Levin
8 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:52 UTC (permalink / raw)
To: patches, stable
Cc: Jiawen Liu, Mark Brown, Sasha Levin, william.zhang, kursad.oney,
jonas.gorski, anand.gore, florian.fainelli, rafal, linux-spi,
linux-arm-kernel, linux-kernel
From: Jiawen Liu <1298662399@qq.com>
[ Upstream commit faa878d4805ca375738294bc52eae310eb287b16 ]
spi_controller_suspend() can fail if pending transfers cannot stop.
bcmbca_hsspi_suspend() ignored the error and still disabled the
PLL and core clocks.
Return the error before disabling the clocks.
Signed-off-by: Jiawen Liu <1298662399@qq.com>
Link: https://patch.msgid.link/tencent_54F5634545908FBA724E758054BF03953808@qq.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.18.53** (`v6.18.53-5049-g0f0bdc2b0dffb`, branch
`autosel`). The verdict is only about this checkout: the buggy suspend
path is in `v6.18.53`, and the patch applies cleanly to it.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[spi / bcmbca-hsspi] [return] [propagate
`spi_controller_suspend()` failure instead of disabling clocks and
reporting success]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review pipeline)
- **Reported-by / Tested-by / Reviewed-by / Acked-by:** none on this
specific patch (sibling Broadcom patches in the same author’s work had
Acked-by/Reviewed-by from Broadcom)
- **Link:** https://patch.msgid.link/tencent_54F5634545908FBA724E758054B
F03953808@qq.com
- **Cc: stable:** absent (not a negative signal)
- **Signed-off-by:** Jiawen Liu (author); Mark Brown (SPI maintainer).
Ignore Sasha Levin SOB (pipeline).
- **[Upstream commit faa878d4805c…]** / pipeline markers: ignored per
instructions
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `spi_controller_suspend()` can fail when pending transfers
cannot be stopped; `bcmbca_hsspi_suspend()` ignored that and still
disabled PLL and core clocks, then returned 0.
- **Symptom:** system suspend is reported successful while SPI clocks
are gated with the queue possibly still active → MMIO to unclocked
hardware, incomplete SPI transactions.
- **Version info:** none in the message. Driver added in v6.3; bug
present from introduction.
- **Root cause:** missing return-value check before clock teardown.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not disguised. It is an explicit PM error-path fix: “Handle X
properly” / missing error check. Real bug, not style.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/spi/spi-bcmbca-hsspi.c` — **+5 / −1**
- Function: `bcmbca_hsspi_suspend()`
- Scope: **single-file surgical fix**
Upstream/context in the provided snippet also shows `remove()`/`#ifdef
CONFIG_PM_SLEEP` around it; the change that actually lands on this tree
is only the suspend return check (parent of `072b287a97bf3` matches
`v6.18.53` for this file, 0-line diff).
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `spi_controller_suspend(host)` return ignored →
`clk_disable_unprepare(pll)` + `clk_disable_unprepare(clk)` → `return
0`.
- **After:** if `spi_controller_suspend()` fails, return that error
immediately; clocks stay enabled.
- Path: **system-sleep suspend error path**, not probe/init.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** logic / PM error-path correctness (resource teardown
while transfers may still be running).
- **Mechanism:** this driver sets `host->transfer_one_message`, so
registration calls `spi_controller_initialize_queue()` and
`ctlr->queued = true`. `spi_controller_suspend()` then calls
`spi_stop_queue()`, which polls up to 500 × ~10 ms (~5 s) and returns
**`-EBUSY`** if the queue/busy bit never clears. The driver used to
power down clocks anyway.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct and matches the usual SPI driver pattern (e.g.
`rockchip_spi_suspend()` in this tree).
- Minimal; no API/data-structure changes.
- Regression risk is low. An automated review (sashiko-bot) noted that
`spi_controller_suspend()` still sets `SPI_CONTROLLER_SUSPENDED` on
failure, and a failed device suspend does not get `resume()`. That is
a **pre-existing SPI core quirk** shared by most drivers that already
check this return. The old path is worse: same flag is set **and**
clocks are gated. Mark Brown applied the series without requesting an
unwind.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Ignored `spi_controller_suspend()` + clock disable introduced in
**`a38a2233f23b5`** (“spi: bcmbca-hsspi: Add driver for newer HSSPI
controller”, William Zhang, 2023-02-09).
- First appeared: **`v6.3-rc1`**. Present for the entire life of this
driver, including 6.18.y.
**Step 3.2: FOLLOW THE FIXES TAG**
Record: No Fixes tag. Buggy code is the original driver, not a later
regression.
**Step 3.3: CHECK FILE HISTORY**
Record (this file, newest first):
- `2b580ac54f40f` — modern PM macros (later autosel candidate; **not** a
prerequisite)
- `072b287a97bf3` — **this** commit
- `148a3f03aec89` — resume-side clock cleanup (**already in v6.18.53**,
had Fixes + Cc: stable)
- Driver-original suspend path otherwise unchanged since v6.3
Standalone. Cover letter: *“The patches are independent and only touch
the local error paths in each driver.”*
**Step 3.4: CHECK THE AUTHOR’S OTHER COMMITS**
Record: Jiawen Liu submitted a 4-patch SPI PM error-handling series
(atcspi200, **bcmbca-hsspi**, fsl-dspi, nxp-fspi). Additional similar
Broadcom fixes exist for bcm63xx / bcm63xx-hsspi / bcm-qspi. Author is a
regular contributor, not the SPI maintainer. Maintainer **Mark Brown**
applied all 4.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Uses existing `spi_controller_suspend()`. Applies
standalone. Series 2/4 is independent.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c faa878d4805c`: **[PATCH 2/4] spi: bcmbca-hsspi: return
error from failed controller suspend**
- Thread: https://patch.msgid.link/tencent_54F5634545908FBA724E758054BF0
3953808@qq.com
- Cover: **[PATCH 0/4] spi: fix PM error handling in several drivers**
(2026-06-20)
- `b4 dig -a`: **v1 only**; committed version is that v1.
- Human review: no NAK. Mark Brown applied to `spi/for-7.3`
(git.kernel.org/broonie/spi/c/faa878d4805c).
- sashiko-bot: “permanently wedged in a suspended state” if error is
propagated (SPI core marks suspended even on `-EBUSY`). No maintainer
follow-up; patch still applied.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To Mark Brown; Cc William Zhang, Kursad Oney,
Jonas Gorski, bcm-kernel-feedback-list, Florian Fainelli, linux-spi,
linux-arm-kernel, etc. Right maintainers/lists. No Acked-by on this hunk
(unlike some sibling Broadcom patches).
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot, bugzilla, or user crash report. Bug is from code
inspection of the ignored return. The same driver’s **resume**
counterpart (`148a3f03aec89`) was user/author-driven with
Fixes+Cc:stable and is already in this tree.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Independent 2/4. Sibling Broadcom suspend-return patches are the
same class, not build/runtime dependencies. Resume clock-disable-on-
failure is already in 6.18.53; this is the suspend-side counterpart.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org returned 403; WebSearch of
`lore.kernel.org/stable` for this subject found **no** stable-specific
discussion. UNVERIFIED that anyone requested or rejected it on stable@;
no evidence of a known reason to skip it.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: `bcmbca_hsspi_suspend()` (modified). Callees of interest:
`spi_controller_suspend()`, `spi_stop_queue()`,
`clk_disable_unprepare()`.
**Step 5.2: TRACE CALLERS**
Record: Wired via `DEFINE_SIMPLE_DEV_PM_OPS(bcmbca_hsspi_pm_ops, …)` /
`.pm` on the platform driver. Called from the PM core on **system
sleep**, not from syscalls directly. Reachable on BCMBCA boards that
suspend with `CONFIG_SPI_BCMBCA_HSSPI` and `CONFIG_PM_SLEEP`.
**Step 5.3: TRACE CALLEES**
Record:
- `spi_controller_suspend()` → `spi_stop_queue()` if queued; always
`__spi_mark_suspended()`.
- Then (success only, after this patch) clock disable.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
`pm_suspend` → `dpm_suspend` → `bcmbca_hsspi_suspend` →
`spi_controller_suspend` → `spi_stop_queue`.
In-flight work is `bcmbca_hsspi_transfer_one` → `bcmbca_hsspi_do_txrx`
doing `__raw_readl`/`__raw_writel` on `bs->regs`. Yanking clocks while
that runs is a hang/abort/corruption path. Userspace can keep the queue
busy (SPI flash, sensors) across a suspend request.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Most SPI drivers in this tree already check
`spi_controller_suspend()` (dw, tegra, rockchip, qup, pxa2xx, atmel, …).
This driver was an outlier. Same author’s bcm63xx/bcm-qspi patches fix
the same pattern.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `v6.18.53:drivers/spi/spi-bcmbca-hsspi.c` still has:
```c
spi_controller_suspend(host);
clk_disable_unprepare(bs->pll_clk);
clk_disable_unprepare(bs->clk);
return 0;
```
`git merge-base --is-ancestor 072b287a97bf3 v6.18.53` → **not an
ancestor**. Bug introduced in v6.3, still in 6.18.53.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** `git diff v6.18.53 072b287a97bf3^ --
drivers/spi/spi-bcmbca-hsspi.c` is empty. No rework needed. Later
“modern PM macros” commit is unrelated and not required.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: Resume-side `148a3f03aec89` **is** in v6.18.53. No other
suspend-error check for this function. This is the missing counterpart,
not a duplicate.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **drivers/spi** — Broadcom BCMBCA HSSPI
(`CONFIG_SPI_BCMBCA_HSSPI`, `depends on ARCH_BCMBCA || COMPILE_TEST`).
**PERIPHERAL / platform-specific**, but SPI NOR on those SoCs is often
boot/config flash, so a bad suspend is not cosmetic.
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: SPI core is mature; this driver is small and not heavily
churned. Bug is long-standing (since v6.3), not a brand-new feature.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: **Platform-specific** — Broadcom BCMBCA (ARM/ARM64 broadband
SoCs: gateways/routers). Only if the HSSPI driver and system sleep are
enabled.
**Step 8.2: TRIGGER CONDITIONS**
Record: System suspend while the SPI queue stays busy for ~5 seconds
(`spi_stop_queue` limit). Uncommon, but exactly the dangerous case
(stuck or long transfer). Not a syzbot syscall crash; can be induced by
userspace SPI activity plus suspend. Unprivileged trigger depends on who
can open the SPI device and who can suspend (typically privileged).
**Step 8.3: FAILURE MODE SEVERITY**
Record:
- Clocks gated while the pump may still MMIO → bus hang, imprecise
abort, or torn SPI NOR write.
- PM core is told suspend succeeded, so the machine continues into sleep
with live SPI I/O unclocked.
- **Severity: HIGH** when it hits (corruption/hang); trigger is
uncommon.
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium-high for BCMBCA devices that suspend; prevents
clock-off during active SPI. Completes PM error handling already fixed
on resume in this tree.
- **Risk:** very low (5 lines, standard pattern). Residual:
`SPI_CONTROLLER_SUSPENDED` can stick if suspend fails (SPI core). That
already exists for the majority of SPI drivers here; old code still
set the flag **and** cut clocks.
- **Ratio:** benefit outweighs risk.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: EVIDENCE**
**For backporting:**
- Real PM bug: ignored `-EBUSY` from `spi_controller_suspend()`, then
clock disable + `return 0`.
- Can unclock MMIO and corrupt in-flight SPI (including flash).
- 5-line, obviously correct, maintainer-applied (Mark Brown).
- Driver and buggy code exist in **this** 6.18.53 tree since v6.3.
- Applies cleanly; no prerequisites.
- Same class as the resume fix already in 6.18.53.
- Aligns this outlier with the rest of the SPI subsystem.
**Against:**
- No user/syzbot report; error path is uncommon (~5 s busy).
- Platform-specific (not all machines).
- Incomplete vs an unwind via `spi_controller_resume()` (SPI core marks
suspended on failure).
- No Fixes/Cc:stable (expected here; not a reason to reject).
**Unresolved:**
- Could not fetch lore.kernel.org (403). Thread recovered via `b4 dig`
mbox + LKML/openwall. No stable@ discussion found.
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — applied by SPI maintainer;
pattern used across `drivers/spi`.
2. Fixes a real bug that affects users? **PASS** — ignored suspend
failure + clock teardown.
3. Important issue? **PASS** — hang/abort/data corruption on active SPI
during suspend, not a style tweak.
4. Small and contained? **PASS** — +5/−1, one function.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — clean on v6.18.53.
**Step 9.3: EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/doc exception. Ordinary small
bugfix.
**Step 9.4: DECISION**
This belongs in this 6.18.y tree. The bug is in the tree, the fix is
surgical, and leaving clocks on when the queue cannot stop is the
correct hardware behavior.
---
### Problem for stable users
On Broadcom BCMBCA HSSPI, suspend called `spi_controller_suspend()`
(which can return `-EBUSY` after ~5 s if transfers will not stop) and
then **always** disabled PLL/core clocks and returned success. That can
cut clocks under a live transfer (`bcmbca_hsspi_do_txrx` MMIO). This
tree already fixed the matching resume error path; suspend was still
wrong.
### Stable rules / risk
Meets the rules: real bug, small, no new API, clean apply to 6.18.53.
Risk is a few lines in one PM callback. Benefit is avoiding unclocked
SPI and false-successful suspend.
### Concerns
No extra commits needed. Sashiko’s “stuck SUSPENDED flag” note is a SPI-
core limitation already shared by most drivers; it does not make the old
“disable clocks anyway” path acceptable.
---
Verification:
- [Phase 1] Parsed subject/tags/body from `git show 072b287a97bf3` /
upstream `faa878d4805c`; no Fixes/Reported-by; Link to msgid; Mark
Brown SOB
- [Phase 2] Diff: +5/−1 in `bcmbca_hsspi_suspend()`; ignore →
check/return
- [Phase 2] `spi_controller_suspend()` / `spi_stop_queue()` in
`drivers/spi/spi.c`: queued controllers can return `-EBUSY`;
`__spi_mark_suspended()` always runs
- [Phase 2] Driver sets `transfer_one_message`;
`spi_register_controller()` initializes the queue (`ctlr->queued =
true`)
- [Phase 3] `git blame 072b287a97bf3^`: buggy lines from `a38a2233f23b5`
(v6.3-rc1)
- [Phase 3] `git log --oneline -- drivers/spi/spi-bcmbca-hsspi.c`:
resume fix `148a3f03aec89` already in v6.18.53; this commit not in
v6.18.53
- [Phase 3] `git merge-base --is-ancestor 072b287a97bf3 v6.18.53` → no;
`git diff v6.18.53 072b287a97bf3^ -- drivers/spi/spi-bcmbca-hsspi.c` →
0 lines
- [Phase 3] Author series via `git log --author="Jiawen Liu" --
drivers/spi/`
- [Phase 4] `b4 dig -c faa878d4805c` / `-a` / `-w` / `-m`: v1 2/4,
independent series, applied by Mark Brown 2026-06-30; Broadcom/SPI
lists on Cc
- [Phase 4] Cover letter from lkml.iu.edu: patches independent
- [Phase 4] sashiko-bot mbox msg: SUSPENDED-flag concern; no NAK from
maintainer
- [Phase 4] lore.kernel.org WebFetch 403; stable@ search found nothing
relevant — UNVERIFIED no stable@ thread
- [Phase 5] Callers: PM ops only; callees: `spi_controller_suspend`,
`clk_disable_unprepare`; transfer path uses MMIO
- [Phase 5] Similar pattern already in `spi-rockchip.c` and many other
SPI drivers
- [Phase 6] `git describe HEAD` → `v6.18.53-5049-g0f0bdc2b0dffb`; `make
kernelversion` → 6.18.53; buggy suspend in `v6.18.53` file
- [Phase 7] `CONFIG_SPI_BCMBCA_HSSPI` depends on `ARCH_BCMBCA`
- [Phase 8] Failure mode: unclocked SPI MMIO / possible flash
corruption; trigger = suspend with busy queue for ~5 s
- UNVERIFIED: live user reports of this suspend path; full
lore.kernel.org HTML (403)
**YES**
drivers/spi/spi-bcmbca-hsspi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcmbca-hsspi.c b/drivers/spi/spi-bcmbca-hsspi.c
index a1ff33f6dc095..50beb8443ea90 100644
--- a/drivers/spi/spi-bcmbca-hsspi.c
+++ b/drivers/spi/spi-bcmbca-hsspi.c
@@ -586,8 +586,12 @@ static int bcmbca_hsspi_suspend(struct device *dev)
{
struct spi_controller *host = dev_get_drvdata(dev);
struct bcmbca_hsspi *bs = spi_controller_get_devdata(host);
+ int ret;
+
+ ret = spi_controller_suspend(host);
+ if (ret)
+ return ret;
- spi_controller_suspend(host);
clk_disable_unprepare(bs->pll_clk);
clk_disable_unprepare(bs->clk);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-19 14:05 UTC | newest]
Thread overview: 9+ 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:47 ` [PATCH AUTOSEL 6.18] spi: qpic-snand: free OOB buffer with ECC context Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm-qspi: return error from failed controller suspend Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] spi: nxp-fspi: disable runtime PM on probe failures Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx: return error from failed controller suspend Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18] spi: loongson: pm: add .freeze .poweroff .thaw .restore Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] spi: bcm63xx-hsspi: return error from failed controller suspend Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] spi: fsl-dspi: clean up after failed suspend and resume Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.15] spi: cadence-qspi: Fix status polling with octal DTR chips Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.6] spi: bcmbca-hsspi: return error from failed controller suspend Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox