* [PATCH AUTOSEL 6.18] clk: clocking-wizard: fix clock difference detection
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vencsys: add missing MODULE_DEVICE_TABLE() Sasha Levin
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Colin Foster, Shubhrajyoti Datta, Brian Masney, Sasha Levin,
sboyd, bmasney+clk, jbrunet+clk, michal.simek, anatoliy.klymenko,
linux-clk, linux-arm-kernel, linux-kernel
From: Colin Foster <colin.foster@in-advantage.com>
[ Upstream commit 006ba909c240b832c876b7aed29c6beec9d1f5e5 ]
Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
change is a one-line restoration of `abs()` in Xilinx Clocking Wizard
divisor search: unsigned wrap made frequencies below the request look
like ~2^64 error, so they were ignored.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[clk / xilinx clocking-wizard] [fix] [clock difference
detection]** — restore correct closest-match comparison when computed
frequency is below the requested rate.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record (from `git show 006ba909c240b`):
- Reviewed-by: Shubhrajyoti Datta \<shubhrajyoti.datta@amd.com\>
(Xilinx/AMD clocking-wizard author)
- Reviewed-by: Brian Masney \<bmasney@redhat.com\> (clk co-maintainer;
also committer)
- Signed-off-by: Colin Foster \<colin.foster@in-advantage.com\> (author)
- Signed-off-by: Brian Masney \<bmasney@redhat.com\>
- No Fixes:, Cc: stable, Reported-by, Tested-by, Link, or Acked-by in
the **merged** commit
- Absence of Fixes/Cc:stable is expected for this review pipeline and is
not a negative
The merged git message has **no body**. The mailing-list submission did
(see Phase 4).
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Merged commit: subject + tags only.
Mailing-list body (from b4 mbox, not in git): unsigned “rollover” in
`diff = freq - rate`. Frequencies below the request produced diffs like
`18446744073708237796` (2^64 wrap) and were not considered. Author
included before/after traces at 133312500 Hz / parent 99999000 Hz.
Cover letter: this is patch **1/3** of “clock-wizard fixups”. The series
also optimizes exact-match search (2/3) and removes the 20 kHz
`WZRD_MIN_ERR` cutoff (3/3). Cover letter: the 20 kHz check made **out-
of-tree consumers fail `clk_set_rate`**; patch 1 is the separate
difference-calculation bug.
Record: **[Bug: unsigned underflow when freq \< rate] [Symptom: wrong
closest match and/or -EBUSY from set_rate/determine_rate] [Root cause:
abs() dropped in a later refactor] [No version tags in the commit
itself]**
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** Subject says “fix”. It is a real comparison bug,
not cleanup.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/clk/xilinx/clk-xlnx-clock-wizard.c` — **1 insertion, 1
deletion**
- Function: `clk_wzrd_get_divisors()`
- Scope: **single-file surgical one-liner**
- Versal sibling `clk_wzrd_get_divisors_ver()` is **not** changed
(already uses `abs()`)
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Before: `diff = freq - rate` with `freq`/`diff` as `u64`. If `freq <
rate`, subtraction wraps. `best_diff` starts at `-1ULL`. Wrapped diffs
lose to any over-rate candidate; if every candidate is under-rate,
`best_diff` stays huge and `return best_diff < WZRD_MIN_ERR ? 0 :
-EBUSY` returns **-EBUSY** (20 kHz threshold still present in the parent
of this commit).
After: `diff = abs(freq - rate)` so both sides of the target compete;
closest match wins.
Path: clock **determine_rate / set_rate** for non-Versal Clocking Wizard
(dynamic reconfig / fractional M and O).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Logic / correctness + unsigned wrap]** — `abs()` was dropped
when 7681f64e6404ed switched the search to `u64` and `best_diff`.
Original v6.4 code in 595c88cda65d30 used `diff = abs(freq - rate)`.
This restores that.
Kernel `abs()` on the `u64` difference works here: wrap of a small Hz
delta becomes a negative `signed long long`, then `abs()` yields the
true delta. Clock Hz values are far below 2^63. Same pattern as the
Versal function.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: **Obviously correct, minimal, matches the pre-regression and
Versal code.** Regression risk is low: set_rate may now succeed with a
slightly-below rate instead of failing or picking a worse over-rate.
That is the intended fix. `abs_diff()` would be cleaner with matching
types, but reviewers accepted `abs()`, and it matches the sibling.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Current `abs()` line: **006ba909c240b** (this commit), 2026-06-29
- Surrounding fractional search: **7681f64e6404ed** (“calculate dividers
fractional parts”), 2025-01-06, **v6.13-rc1**
- Function introduced: **595c88cda65d30** (v6.4-rc1) **with abs()
already**
- 7681f64e **removed** `abs()` and used `diff = freq - rate`
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Buggy commit is **7681f64e6404ed**, ancestor
of HEAD (`git merge-base --is-ancestor`).
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: 3-patch series, same author/date:
1. **006ba909c240b** — this fix (1/3)
2. **9329714414d1f** — early-out on exact match (optimization, 2/3)
3. **54ed94b709e03** — drop 20 kHz `WZRD_MIN_ERR` (3/3)
This patch is **standalone**. It does not need 2/3 or 3/3. With the 20
kHz check still present, `abs()` is what lets a close **under**-rate
pass `best_diff < 20000` instead of wrapping to -EBUSY.
Related: **4adf593c6fc5a** overflow fix in the same file (`Cc: stable`,
Fixes: 7681f64e). **e0a94c6bb5b48** (v6.18-rc1) already used `abs()` on
the Versal path.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Colin Foster is not the historical driver author (that is
Shubhrajyoti Datta / AMD). Datta reviewed this. Foster’s three clocking-
wizard commits are this series.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Needs `clk_wzrd_get_divisors()` with `u64` `freq`/`diff` as in
7681f64e. That is in this tree. **Applies standalone.** `abs()` is in
`include/linux/math.h`.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 006ba909c240b` →
https://patch.msgid.link/20260629205346.3228886-2-colin.foster@in-
advantage.com
- `b4 dig -a`: only **PATCH v2 1/3** (v2: rebase + signoffs). v1 not in
the matched thread.
- Saved mbox: cover + 3 patches, **no reply NAKs** in that thread
- WebFetch/curl to lore.kernel.org: **403** — discussion beyond the mbox
is UNVERIFIED
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
`b4 dig -w` To/Cc: linux-clk, linux-arm-kernel, linux-kernel,
Shubhrajyoti Datta, Michal Simek, Brian Masney, Stephen Boyd, Michael
Turquette.
Record: **Clk lists + clk maintainers CC’d; Reviewed-by from AMD driver
author and Brian Masney.**
**Step 4.3: SEARCH FOR THE BUG REPORT**
No syzbot/bugzilla. Evidence is the author’s traces (unsigned wrap
~2^64) and cover-letter `clk_set_rate` failures.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: **v2 0/3 “clock-wizard fixups”.** 1/3 is this logic fix; 2/3
optimization; 3/3 policy (allow error ≥ 20 kHz). 1/3 does not depend on
the others.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned **403**. UNVERIFIED
whether stable@ discussed it.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
semcode MCP: **not available**. Used grep + callers in this file.
**Step 5.1: KEY FUNCTIONS**
Record: **`clk_wzrd_get_divisors()`** only.
**Step 5.2: TRACE CALLERS**
- `clk_wzrd_dynamic_all_nolock()` → `clk_wzrd_dynamic_all()` →
`clk_wzrd_clk_div_all_ops.set_rate`
- `clk_wzrd_determine_rate_all()` →
`clk_wzrd_clk_div_all_ops.determine_rate`
Versal ops use `clk_wzrd_get_divisors_ver()`, already using `abs()`.
**Step 5.3: TRACE CALLEES**
`div64_u64`, `DIV_ROUND_CLOSEST_ULL`, writes into
`divider->{m,m_frac,d,o,o_frac}`. No allocation/locking inside the
search.
**Step 5.4: FOLLOW THE CALL CHAIN**
Probe registers outputs with `clk_wzrd_clk_div_all_ops` for non-Versal
dynamic-reconfig Clocking Wizard (`xlnx,clocking-wizard`, v5.2, v6.0).
`clk_set_rate` / `clk_round_rate` from consumers (display pixel clocks
were the reason fractional M/O were added).
Record: **Reachable from userspace/driver clock APIs on that IP. Not a
syscall bug by itself; standard clk consumer path.**
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Versal `clk_wzrd_get_divisors_ver()` already has `diff = abs(freq -
rate)` (e0a94c6bb5b48). Original non-Versal code had abs() until
7681f64e dropped it.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **v7.3-rc4** / Makefile **7.3.0-rc4**
- File and `clk_wzrd_get_divisors()` **exist**
- HEAD already has `diff = abs(freq - rate)` because **006ba909c240b is
an ancestor** (`git merge-base --is-ancestor` → yes). `git describe
--contains` → **v7.3-rc1**
- Bug introduced **v6.13** (7681f64e), long before 7.3
- Pre-fix code (parent of 006ba909) still had `diff = freq - rate` and
`WZRD_MIN_ERR` 20000
This is **not** “bug added after this branch.” The driver and the
regression are part of this 7.3 line.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
On **this** checkout the one-liner is already applied; a second apply
would not change anything. Against the pre-fix function the hunk is
context-identical (one line). Expected difficulty: **clean apply** on a
tree that still has `diff = freq - rate`.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This exact fix **is** in HEAD, plus 2/3 and 3/3. That is the
mainline landing, not a separate in-tree workaround. No competing
alternate fix.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **`drivers/clk/xilinx/`** — Xilinx Clocking Wizard
(`CONFIG_COMMON_CLK_XLNX_CLKWZRD`, `depends on OF`). **PERIPHERAL /
platform-specific** (FPGA/SoC), but it is the clock provider for that
IP, including pixel-clock accuracy.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Active through 2025–2026 (fractional dividers, Versal, overflow
fix, this series). Mature in-tree driver, not staging.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific / config-specific** —
`COMMON_CLK_XLNX_CLKWZRD`, non-Versal Clocking Wizard with dynamic
reconfig / fractional search. Not universal.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: **`clk_set_rate` / determine_rate** when the closest achievable
frequency is below the request, or when under-rate candidates must
compete with worse over-rate ones. Common for arbitrary pixel/device
rates. Unprivileged userspace can hit it if a consumer exposes clock
control; typically a device driver at probe/modeset.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Wrong clock chosen or `clk_wzrd_get_divisors()` returns
-EBUSY** so rate set fails. Not oops/UAF/deadlock. Author showed
wraparound and `clk_set_rate` failure in the series. **Severity:
MEDIUM** (functional breakage on that hardware, including display
clocks).
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: **Benefit: medium** for Clocking Wizard users (correct closest
rate; fewer false -EBUSY). **Risk: very low** (1 line, same as
old/Versal code). **Ratio: favorable.**
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real unsigned wrap; traces show ~2^64 diffs for freq \< rate
- Regression: 595c88cd used `abs()`; 7681f64e dropped it in v6.13
- Restores closest-match; can avoid -EBUSY under the 20 kHz check
- 1 line, obviously correct
- Reviewed by AMD driver author and clk co-maintainer
- Driver and function exist in this 7.3 tree; bug predates 7.3
- Standalone; not a new API
AGAINST:
- Merged commit message has no body (explanation only on list)
- Not crash/security/corruption
- Peripheral Xilinx FPGA/SoC driver
- Cover letter also talks about OOT consumers and patch 3’s 20 kHz
policy
- Author’s 133312500 Hz sample still picked the over-rate 19500 Hz
candidate; abs() matters when the **best** match is below the request
- lore.kernel.org HTTP 403; no stable@ thread verified
UNRESOLVED:
- Full v1 review thread
- Whether stable@ already took this (403)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — wrap math is clear; author
posted before/after; two Reviewed-by
2. Fixes a real bug that affects users? **PASS** — wrong diffs; set_rate
can fail or pick a worse rate
3. Important issue? **PASS** (medium) — not a panic, but the driver’s
job is correct rates; pixel-clock path was the reason fractional
search exists
4. Small and contained? **PASS** — 1 line, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — same function is here; HEAD
already contains this commit as of v7.3-rc1, which means 7.3’s own
code already includes the fix. That does not make the fix unsuitable;
it confirms the code matches.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **None** (not a device ID, quirk, DT, build, or docs exception).
It is a normal small correctness fix.
**Step 9.4: MAKE YOUR DECISION**
This tree is 7.3-rc4 and already contains the line, because the commit
landed in v7.3-rc1. The decision is still whether this is appropriate
**for this tree’s code**, not whether the pipeline already “selected”
it.
It is a **v6.13 regression** (accidental drop of `abs()`), one line,
reviewed, and it fixes real closest-rate / -EBUSY behavior on Xilinx
Clocking Wizard. That meets stable rules for this checkout. Later series
patches (search micro-opt, dropping 20 kHz) are separate and not
required.
===============================================================
DETAILED EXPLANATION
===============================================================
**Problem:** `clk_wzrd_get_divisors()` ranked candidates with unsigned
`freq - rate`. Any frequency below the request wrapped to ~2^64, so
those configs were ignored. Combined with `best_diff < 20000`, that
yields **-EBUSY** when the only close match is slightly low, or a
**worse higher** clock when a closer lower one exists. That is exactly
the fractional-divider accuracy the v6.13 work was supposed to provide
(pixel clocks).
**Why it matters here:** This 7.3 tree has that search (7681f64e).
Consumers of `xlnx,clocking-wizard` that call `clk_set_rate` can fail or
lock the IP to the wrong rate.
**Stable rules:** Small, obvious, real bug, no new API. Risk is a
correct rate instead of failure/wrong rate.
**Concerns:** Empty merged commit message; not a crash. Series 3/3 is a
policy change and should not be required. No backport rework is needed
on a tree that still has `diff = freq - rate`.
===============================================================
Verification:
- [Phase 1] `git show 006ba909c240b`: subject+tags only; Reviewed-by
Datta and Masney; no Fixes/stable
- [Phase 1] b4 mbox body: wraparound explanation and traces (133312500
Hz)
- [Phase 2] Diff: one line `diff = freq - rate` → `diff = abs(freq -
rate)` in `clk_wzrd_get_divisors`
- [Phase 2] Parent still has `return best_diff < WZRD_MIN_ERR ? 0 :
-EBUSY` with `WZRD_MIN_ERR` 20000
- [Phase 2] `include/linux/math.h`: `abs()` signed conversion; Hz deltas
fit
- [Phase 3] blame: abs line is 006ba909; search from 7681f64e6404ed
(v6.13-rc1)
- [Phase 3] 595c88cda65d30 originally used `diff = abs(freq - rate)`;
7681f64e removed it
- [Phase 3] `git merge-base --is-ancestor 006ba909c240b HEAD` → yes;
`--contains` → v7.3-rc1
- [Phase 3] Series 006ba909, 9329714414d1f, 54ed94b709e03; this patch is
1/3 standalone
- [Phase 4] `b4 dig -c`:
patch.msgid.link/20260629205346.3228886-2-colin.foster@in-
advantage.com
- [Phase 4] `b4 dig -a`: v2 1/3 only in match; cover v2 rebase +
signoffs
- [Phase 4] `b4 dig -w`: linux-clk, Boyd, Turquette, Datta, Simek,
Masney
- [Phase 4] WebFetch/curl lore: 403; stable@ discussion UNVERIFIED
- [Phase 5] Callers: `clk_wzrd_dynamic_all_nolock`,
`clk_wzrd_determine_rate_all` (non-Versal ops)
- [Phase 5] Versal sibling already `abs()` via e0a94c6bb5b48 (v6.18-rc1)
- [Phase 6] `git describe HEAD` v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` 7.3.0-rc4
- [Phase 6] HEAD already has `diff = abs(freq - rate)` at lines 375 and
416
- [Phase 7] Kconfig `COMMON_CLK_XLNX_CLKWZRD`; compatibles
xlnx,clocking-wizard{,-v5.2,-v6.0}
- [Phase 8] Failure mode: wrong rate or -EBUSY on set_rate; severity
MEDIUM
- UNVERIFIED: v1 thread contents; stable@ history (lore 403)
===============================================================
**YES**
drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index dbef983eb4252..4ef0595f7d993 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -406,7 +406,7 @@ static int clk_wzrd_get_divisors(struct clk_hw *hw, unsigned long rate,
if (o < omin || o > omax)
continue;
freq = DIV_ROUND_CLOSEST_ULL(vco_freq, o);
- diff = freq - rate;
+ diff = abs(freq - rate);
if (diff < best_diff) {
best_diff = diff;
divider->m = m >> 3;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vencsys: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] clk: clocking-wizard: fix clock difference detection Sasha Levin
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp: " Sasha Levin
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Brian Masney, Sasha Levin, y.oudjana, sboyd,
bmasney+clk, jbrunet+clk, matthias.bgg, angelogioacchino.delregno,
linux-clk, linux-mediatek, linux-kernel, linux-arm-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 36cf8d279ff90835f4b002dec28eedb059554842 ]
The driver has an OF match table wired to .of_match_table, but does
not export the table with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE(of, ...) entry so module alias
information is generated for OF based module autoloading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the platform driver, and the missing module alias publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Connection lost, reconnecting to https://agentn.us.api5.cursor.sh
(attempt 1)...
Retry attempt 1...
This tree is **Linux 7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
driver exists here; the change is a one-line `MODULE_DEVICE_TABLE(of,
...)` so the vencsys clock module can autoload from DT.
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
- Record: `[clk/mediatek] [add] add missing MODULE_DEVICE_TABLE() for
mt6735-vencsys`
**Step 1.2: Tags**
- Record:
- **Fixes:** none (expected for this review)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** Brian Masney `<bmasney@redhat.com>` (COMMON CLK
FRAMEWORK maintainer)
- **Acked-by:** none
- **Link:** none in the commit; mailing list found via `b4 dig`
- **Cc: stable:** none (not a negative signal)
- **Signed-off-by:** Pengpeng Hou `<pengpeng@iscas.ac.cn>` (author);
Brian Masney `<bmasney@redhat.com>` (clk maintainer who applied it)
- Notable: maintainer review + maintainer SOB; no user/syzbot report
**Step 1.3: Body**
- Record:
- **Bug:** OF match table is wired to `.of_match_table` but not
exported with `MODULE_DEVICE_TABLE()`, so `file2alias` does not emit
`of:` aliases.
- **Symptom:** when `CONFIG_COMMON_CLK_MT6735_VENCSYS=m`,
udev/modprobe cannot autoload `clk-mt6735-vencsys` from
`mediatek,mt6735-vencsys`.
- **Version info:** none in the message. Git: driver added in
**v6.13-rc1** (`0bd9b1211f741`); this fix first appears in
**v7.3-rc1**.
- **Root cause:** omitted alias publication, not a runtime logic
error.
**Step 1.4: Hidden bug fix?**
- Record: Not hidden. It is a real functional gap (module autoload), not
a crash/UAF. The “source-level fix / no hardware reproduction” wording
matches the original lore submission, not pipeline decoration.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
- Record:
- `drivers/clk/mediatek/clk-mt6735-vencsys.c`: **+1 / -0**
- Function/data: `of_match_mt6735_vencsys[]` (file scope)
- Scope: single-file, one-line surgical change
**Step 2.2: Code flow**
- Record: Before, the table is only consumed by
`platform_driver.driver.of_match_table` (bind if the module is already
loaded or built-in). After, `MODULE_DEVICE_TABLE(of,
of_match_mt6735_vencsys)` also creates a `__mod_device_table` alias so
`scripts/mod/file2alias.c` `do_of_entry()` emits
`of:N*T*Cmediatek,mt6735-vencsys`. Build-time metadata only; no probe-
path change.
**Step 2.3: Bug mechanism**
- Record: **Hardware matching / module autoload** (same class as missing
device IDs). Match table exists but is not published for modules.
**Step 2.4: Fix quality**
- Record: Obviously correct; matches sibling MT6735 files that shipped
with `MODULE_DEVICE_TABLE` (`apmixedsys`, `infracfg`, `pericfg`,
`topckgen`). No runtime lock/API/data-structure change. Regression
risk is essentially none. `imgsys` and `mfgcfg` still lack the macro;
this patch does not depend on those.
## Phase 3: Git history
**Step 3.1: Blame**
- Record: Match table and driver introduced by `0bd9b1211f741` (“clk:
mediatek: Add drivers for MT6735 syscon clock and reset controllers”,
Yassine Oudjana, 2024-11-06, applied 2024-11-14). First in
**v6.13-rc1**. `MODULE_DEVICE_TABLE` line is this commit
`36cf8d279ff90`.
**Step 3.2: Fixes: tag**
- Record: No `Fixes:` tag. The omission dates from the introducing
commit `0bd9b1211f741`, which **is** an ancestor of this tree.
**Step 3.3: File history**
- Record: Only two commits on this file: introducer + this fix. No later
conflicting refactor. Sibling same-day commit `096ab64a0e694` does the
same for `clk-mt6735-vdecsys.c`. Standalone; not patch 2/N.
**Step 3.4: Author**
- Record: Pengpeng Hou is not the MT6735 clk maintainer (that is Yassine
Oudjana). Same author also added `MODULE_DEVICE_TABLE` in `clk-
mt6735-vdecsys.c` and two i.MX clk drivers. Applied by clk co-
maintainer Brian Masney.
**Step 3.5: Dependencies**
- Record: None. Uses existing `MODULE_DEVICE_TABLE` / `of_device_id`.
Applies on top of the original driver with no prerequisites.
## Phase 4: Mailing list and external research
**Step 4.1: Original discussion**
- Record: `b4 dig -c 36cf8d279ff90` →
https://patch.msgid.link/20260704124808.19916-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: **v1 only** (2026-07-04). Applied version is that v1.
- Thread (mbox): author patch + Brian Masney `Reviewed-by`. No NAK, no
stable nomination, no risk discussion.
**Step 4.2: Recipients (`b4 dig -w`)**
- Record: Yassine Oudjana (MT6735 clk maintainer), Stephen Boyd, Brian
Masney, Michael Turquette, Matthias Brugger, AngeloGioacchino Del
Regno; `linux-clk`, `linux-mediatek`, `linux-arm-kernel`, `linux-
kernel`. Right maintainers and lists.
**Step 4.3: Bug report**
- Record: No Reported-by, no syzbot, no bugzilla. Author states there
was no hardware reproduction.
**Step 4.4: Series**
- Record: Independent single patches (vencsys + vdecsys), not a numbered
series. This commit does not need the vdecsys patch.
**Step 4.5: Stable list**
- Record: **UNVERIFIED.** `curl` to lore.kernel.org/stable returned HTTP
403. No in-tree evidence this was rejected for stable.
## Phase 5: Code semantic analysis
**Step 5.1: Functions**
- Record: No function body changed. Adds `MODULE_DEVICE_TABLE` next to
`of_match_mt6735_vencsys[]`. Driver still uses
`module_platform_driver` → `mtk_clk_simple_probe` /
`mtk_clk_simple_remove`.
**Step 5.2: Callers**
- Record: Platform core binds via OF compatible. Probe is boot/device-
instantiation, not a syscall hot path. Impact is whether the **module
is loaded at all**.
**Step 5.3: Callees**
- Record: Unchanged. `mtk_clk_simple_probe()` in `clk-mtk.c` registers
the vencsys gates from `vencsys_clks`.
**Step 5.4: Reachability**
- Record: Reachable when DT has `mediatek,mt6735-vencsys` and the driver
is a module. **No in-tree `.dts`/`.dtsi` uses that compatible** (only
`Documentation/devicetree/bindings/clock/mediatek,syscon.yaml`). Users
need out-of-tree DT (typical for older MT6735 phones). Built-in (`=y`)
is unaffected.
**Step 5.5: Similar patterns**
- Record: Same omission still present in `clk-mt6735-imgsys.c` and `clk-
mt6735-mfgcfg.c`. Four other MT6735 clk files had
`MODULE_DEVICE_TABLE` from day one (`43c04ed791892` for apmixedsys).
Pattern is systematic but this patch is complete for vencsys.
## Phase 6: Local tree cross-reference
**Step 6.1: Buggy code in this tree?**
- Record: Yes. File exists; current tree already has the fix at line 40.
Bug was the missing alias from v6.13 until v7.3-rc1.
`CONFIG_COMMON_CLK_MT6735_VENCSYS` is **tristate**.
**Step 6.2: Backport difficulty**
- Record: **Clean apply.** File is essentially unchanged since
introduction. One inserted line after the match table.
**Step 6.3: Related fixes already here**
- Record: This exact commit is already an ancestor of HEAD (`git merge-
base --is-ancestor 36cf8d279ff90 HEAD` → yes). That is mainline
history, not a reason to reject on merit. No alternate fix. Sibling
vdecsys fix is also present; imgsys/mfgcfg still missing.
## Phase 7: Subsystem context
**Step 7.1: Subsystem / criticality**
- Record: `drivers/clk/mediatek/` — SoC clock driver. **PERIPHERAL**
(MT6735 video-encoder clocks only). Not mm/fs/net core.
**Step 7.2: Activity**
- Record: Mediatek clk is actively maintained; this file is not. Two
commits total. Mature omission, not churn.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
- Record: Only MT6735 systems with `CONFIG_COMMON_CLK_MT6735_VENCSYS=m`
and a DT node `mediatek,mt6735-vencsys`. Driver- and config-specific;
small population (no in-tree board DTS).
**Step 8.2: Trigger**
- Record: Modular boot + OF device present. Not a race. Unprivileged
users do not trigger it; udev autoload fails. Built-in and manual
`modprobe` work.
**Step 8.3: Failure mode**
- Record: Module does not autoload → vencsys clocks (smi_larb3, venc,
jpgenc, jpgdec) never register → video encode/JPEG on that SoC can
fail. **Severity: MEDIUM** (functional miss, not
oops/corruption/deadlock). Fits stable-rules “oh, that’s not good” and
the device-ID-style enablement exception.
**Step 8.4: Risk-benefit**
- Record: **Benefit:** correct autoload for a supported tristate driver;
one line. **Risk:** negligible (modinfo/depmod metadata only). Ratio
favors applying it on trees that have this driver.
## Phase 9: Final synthesis
**Step 9.1: Evidence**
FOR:
- Real autoload bug: match table used, aliases not published
- One line, obviously correct, reviewed and applied by clk maintainer
Brian Masney
- Driver is tristate; modular build is an intended config
- Same class as device-ID additions (publish existing OF IDs)
- Sibling MT6735 files already had the macro; this was an omission from
`0bd9b1211f741`
- Zero runtime regression risk; applies cleanly
- Driver and bug exist in this 7.3 tree (introduced v6.13)
AGAINST:
- Not a crash, UAF, deadlock, or security issue
- No user report and no hardware reproduction
- No in-tree DT consumer for this compatible
- Niche SoC; often clocks are built-in
UNRESOLVED:
- Whether this was discussed on the stable list (lore HTTP 403)
**Step 9.2: Stable rules checklist**
1. Obviously correct and tested? **PASS** — one-line, standard macro,
maintainer Reviewed-by/SOB. No runtime test; correctness is
mechanical.
2. Real bug that affects users? **PASS** — modular OF autoload is
broken. User set is small but real if they use `=m`.
3. Important issue? **PASS** (low end of “oh, that’s not good” /
hardware-enablement), not CRITICAL.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS** — publishes existing match table.
6. Can apply to this tree? **PASS** — file and table exist; patch is a
clean one-liner.
**Step 9.3: Exception category**
- Record: Closest match is **device ID / hardware matching** (OF module
aliases), not a new driver.
**Step 9.4: Decision**
Backport this. It is a one-line, obviously correct fix for failed OF
module autoload on an existing tristate driver. That is the same kind of
enablement stable already takes for new device IDs, with no runtime
behavior change if the module is built-in or loaded by other means. This
7.3 tree has the driver (since v6.13); there are no missing
prerequisites. Limited hardware and no crash report lower urgency, not
correctness.
### What it solves for stable users
If `clk-mt6735-vencsys` is a module, userspace cannot match
`mediatek,mt6735-vencsys` and load it. Video-encoder clocks then never
appear. `MODULE_DEVICE_TABLE(of, ...)` generates the aliases
`file2alias` already knows how to emit.
### Risk vs benefit
Benefit is correct autoload on MT6735 modular configs. Risk is
negligible: no probe, locking, or ABI change.
## Verification
- [Phase 1] Parsed tags: Reviewed-by + SOB Brian Masney; no
Fixes/Reported-by/Cc:stable
- [Phase 1] `git show -s 36cf8d279ff90`: body matches the candidate;
“source-level fix” is in the real commit and lore v1
- [Phase 2] Diff: +1 `MODULE_DEVICE_TABLE(of, of_match_mt6735_vencsys)`
in `clk-mt6735-vencsys.c`
- [Phase 2] `include/linux/module.h` and `scripts/mod/file2alias.c`
`do_of_entry()`: confirmed alias generation from that macro
- [Phase 3] `git blame`: table from `0bd9b1211f741` (v6.13-rc1); macro
from `36cf8d279ff90`
- [Phase 3] `git describe --contains 0bd9b1211f741`: v6.13-rc1; fix
contained in v7.3-rc1; `git merge-base --is-ancestor` vs v7.2: not in
v7.2
- [Phase 3] File log: only introducer + this commit; sibling
`096ab64a0e694` for vdecsys
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 lore thread; Masney Reviewed-by;
correct clk/mediatek CCs; no NAK
- [Phase 4] WebFetch lore/msgid.link: HTTP 403; used mbox instead
- [Phase 5] Kconfig: `COMMON_CLK_MT6735_VENCSYS` tristate; Makefile
`obj-$(CONFIG_COMMON_CLK_MT6735_VENCSYS)`
- [Phase 5] `mtk_clk_simple_probe` in `clk-mtk.c` used unchanged
- [Phase 5] Grep: no in-tree dts/dtsi `mediatek,mt6735-vencsys`; binding
in `mediatek,syscon.yaml`
- [Phase 5] imgsys/mfgcfg still lack `MODULE_DEVICE_TABLE`
- [Phase 6] `make kernelversion` / `git describe HEAD`: 7.3.0-rc4 /
v7.3-rc4-27-gf0100363d8c37
- [Phase 6] File currently contains the macro; `git merge-base --is-
ancestor 36cf8d279ff90 HEAD` = yes
- [Phase 7] MAINTAINERS: MT6735 clk → Yassine Oudjana; COMMON CLK →
Stephen Boyd, Brian Masney, Jerome Brunet
- [Phase 8] Failure mode: missing module autoload, not oops; severity
MEDIUM
- UNVERIFIED: stable@ lore discussion (HTTP 403)
**YES**
drivers/clk/mediatek/clk-mt6735-vencsys.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/mediatek/clk-mt6735-vencsys.c b/drivers/clk/mediatek/clk-mt6735-vencsys.c
index 8dec7f98492ac..64ba9da9fd427 100644
--- a/drivers/clk/mediatek/clk-mt6735-vencsys.c
+++ b/drivers/clk/mediatek/clk-mt6735-vencsys.c
@@ -37,6 +37,7 @@ static const struct of_device_id of_match_mt6735_vencsys[] = {
{ .compatible = "mediatek,mt6735-vencsys", .data = &vencsys_clks },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, of_match_mt6735_vencsys);
static struct platform_driver clk_mt6735_vencsys = {
.probe = mtk_clk_simple_probe,
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] clk: clocking-wizard: fix clock difference detection Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vencsys: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] clk: zynq: handle kasprintf() failure in periph_clk registration Sasha Levin
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Brian Masney, Abel Vesa, Sasha Levin, abelvesa,
sboyd, bmasney+clk, jbrunet+clk, Frank.Li, linux-clk, imx,
linux-arm-kernel, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit cd21fdcbef3fb32136f72075ab668b075423b3f6 ]
The driver has a match table for the of bus wired into its driver
structure, but the table is not exported with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE() entry so module alias information
is generated for automatic module loading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the driver registration structure, and the missing module alias
publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Link: https://patch.msgid.link/20260705001705.70400-1-pengpeng@iscas.ac.cn
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
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: **[clk/imx]** **[add]** missing `MODULE_DEVICE_TABLE()` so the
i.MX8QXP SCU clock driver’s OF match table is exported as module
aliases.
**Step 1.2 — Tags**
Record:
- `Reviewed-by: Brian Masney <bmasney@redhat.com>`
- `Link:
https://patch.msgid.link/20260705001705.70400-1-pengpeng@iscas.ac.cn`
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>` (clk/imx
maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Acked-by:`, or `Cc:
stable` (absence of the last two is expected for this review)
**Step 1.3 — Body**
Record: The driver already has `imx8qxp_match[]` wired as
`of_match_table`, but it never called `MODULE_DEVICE_TABLE(of, …)`.
Without that, modpost/`file2alias` does not emit `of:N*T*C…` aliases, so
udev/`modprobe`/initramfs tools cannot autoload `clk-imx-scu` from DT
compatible strings. Failure mode: modular `CONFIG_CLK_IMX8QXP=m` does
not bind on `fsl,scu-clk` / `fsl,imx8qxp-clk` / `fsl,imx8qm-clk` /
`fsl,imx8dxl-clk`. The author presents this as a source-level
correctness fix, not a live hardware repro. No version range is given.
**Step 1.4 — Hidden bug fix?**
Record: Not disguised. It is an explicit completeness fix for module
alias publication. That is a real functional bug for `=m` builds, not
style cleanup.
---
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
Record: One file, `drivers/clk/imx/clk-imx8qxp.c`, **+1 / -0**. One
insertion after `imx8qxp_match[]`. Surgical single-file change.
**Step 2.2 — Code flow**
Record: **Before:** `imx8qxp_match[]` is only referenced by
`imx8qxp_clk_driver.driver.of_match_table` (bind-time matching if the
module is already loaded). **After:** the same table is also aliased for
modpost (`__mod_device_table__kmod_*__of__imx8qxp_match`), which
produces OF module aliases. No probe, init, or error-path logic changes.
**Step 2.3 — Bug mechanism**
Record: **Hardware/module-autoload omission** (same class as missing
device IDs). The OF table existed; aliases were not published.
`MODULE_DEVICE_TABLE` is the required export so
`scripts/mod/file2alias.c` `do_of_entry()` can emit aliases.
**Step 2.4 — Fix quality**
Record: Obviously correct: same pattern as `clk-imx8mm.c`
(`MODULE_DEVICE_TABLE(of, imx8mm_clk_of_match)`) and the sibling LPCG
driver. Minimal, no API/data-structure change. Regression risk is
essentially none: for `=y` the extra alias symbol is unused at runtime;
for `=m` it only adds `modules.alias` entries.
---
## Phase 3: Git history
**Step 3.1 — Blame**
Record: Match table introduced in `c2cccb6d0b33b` (2018, original
driver). Modular build introduced in `e0d0d4d86c766` (“Support building
i.MX8QXP clock driver as module”, 2020-08-22, first in **v5.9**). That
is when `MODULE_DEVICE_TABLE` became necessary and was omitted. Bug is
long-standing, not recent.
**Step 3.2 — Fixes: tag**
Record: None. The introducing commit is `e0d0d4d86c766`; it is an
ancestor of this HEAD.
**Step 3.3 — File history**
Record: Immediate neighbor `e89485455ac49` (`clk: imx: imx8qxp-lpcg: add
missing MODULE_DEVICE_TABLE()`) is the same author/pattern on the LPCG
sibling. Independent 1-line fix; this patch does not depend on it. Other
recent file history (`aadb7d1f2ca0f` softdep on `scu_pd`) is unrelated.
**Step 3.4 — Author**
Record: Pengpeng Hou submitted a cluster of clk `MODULE_DEVICE_TABLE`
additions (imx8qxp, imx8qxp-lpcg, mediatek mt6735). Not the clk/imx
maintainer. Maintainer Abel Vesa applied it; Brian Masney reviewed.
**Step 3.5 — Dependencies**
Record: None. Needs only `imx8qxp_match[]` and `MODULE_DEVICE_TABLE`
from `<linux/module.h>`, both present. Standalone.
---
## Phase 4: Mailing list / external research
**Step 4.1 — Original discussion**
Record: `b4 dig -c cd21fdcbef3fb` →
https://patch.msgid.link/20260705001705.70400-1-pengpeng@iscas.ac.cn
`b4 dig -a`: single **v1**, no later revisions.
Thread (gnuweeb mirror; lore.kernel.org returned 403): Brian Masney
`Reviewed-by`; Abel Vesa “Applied, thanks!” as `cd21fdcbef3fb`. No NAK,
no stable nomination, no risk flags.
**Step 4.2 — Recipients (`b4 dig -w`)**
Record: Abel Vesa, Peng Fan, Stephen Boyd, Michael Turquette, Frank Li,
Sascha Hauer, Fabio Estevam, linux-clk, imx@lists.linux.dev, linux-arm-
kernel. Correct maintainers and lists.
**Step 4.3 — Bug report**
Record: No `Reported-by` / syzbot / bugzilla. Evidence is the match
table vs missing export. Pull text for clk-imx 7.3 lists “Add missing
MODULE_DEVICE_TABLE() declarations for i.MX8QXP clock drivers.”
**Step 4.4 — Series**
Record: Standalone `[PATCH]`, not `N/M`. Sibling LPCG patch is parallel
and not a prerequisite.
**Step 4.5 — Stable list**
Record: gnuweeb `/stable/?q=imx8qxp+MODULE_DEVICE_TABLE` did not show a
dedicated discussion of this patch (hits were unrelated imx8 noise).
lore.kernel.org/stable returned 403.
---
## Phase 5: Code semantics
**Step 5.1 — Functions**
Record: No function body changed. Data: `imx8qxp_match[]`. Related:
`imx8qxp_clk_driver`, `imx8qxp_clk_init()`, `imx8qxp_clk_probe()`.
**Step 5.2 — Callers**
Record: `imx8qxp_clk_init()` (`module_init`) →
`platform_driver_register(&imx8qxp_clk_driver)`. Probe runs when OF
compatible matches. Autoload is what was broken: without aliases the
module may never load, so probe never runs.
**Step 5.3 — Callees**
Record: Probe calls `imx_clk_scu_init()`, many
`imx_clk_scu`/`imx_clk_scu2`, `of_clk_add_hw_provider()`. Unchanged.
**Step 5.4 — Reachability**
Record: Userspace-reachable only in the sense that missing aliases
prevent the module from being requested. Affects i.MX8QXP/QM/DXL boards
with `CONFIG_CLK_IMX8QXP=m`. DT compatibles: `fsl,scu-clk`,
`fsl,imx8dxl-clk`, `fsl,imx8qxp-clk`, `fsl,imx8qm-clk`.
**Step 5.5 — Similar patterns**
Record: Other imx clk files already had `MODULE_DEVICE_TABLE(of, …)`
(`clk-imx8mm.c`, `clk-imx8mn.c`, `clk-imx8mp.c`, `clk-imx8mq.c`, `clk-
imx8-acm.c`, etc.). This file was the omission.
---
## Phase 6: Local tree
**Step 6.1 — Does the buggy code exist here?**
Record: Local tree is **v7.3-rc4** (`git describe` =
`v7.3-rc4-27-gf0100363d8c37`, `VERSION=7 PATCHLEVEL=3 SUBLEVEL=0
EXTRAVERSION=-rc4`). `CLK_IMX8QXP` is **tristate**. File has
`imx8qxp_match[]`, `module_init`/`module_exit`, and (after this commit)
`MODULE_DEVICE_TABLE`. The omission existed from v5.9 until this 7.3-era
commit. `git merge-base --is-ancestor cd21fdcbef3fb HEAD` is true — the
fix is already in this checkout; the underlying driver and match table
are present.
**Step 6.2 — Backport difficulty**
Record: Clean apply: one line after the sentinel of `imx8qxp_match[]`.
No refactor of that region.
**Step 6.3 — Related fixes already here?**
Record: Sibling LPCG table already has `MODULE_DEVICE_TABLE`. No earlier
equivalent for `clk-imx8qxp.c`. No competing fix.
---
## Phase 7: Subsystem / maintainer
**Step 7.1**
Record: **clk / i.MX SoC clocks**. **IMPORTANT** for i.MX8QXP/QM/DXL
(clock provider for the SoC), not core mm/vfs. Peripheral relative to
the whole kernel, critical on those boards.
**Step 7.2**
Record: Actively maintained (`clk-imx-7.3` pull by Abel Vesa). The
missing alias is old; the rest of the file is mature.
---
## Phase 8: Impact and risk
**Step 8.1 — Who is affected**
Record: **Platform/config-specific**: NXP i.MX8QXP/QM/DXL with
`CONFIG_CLK_IMX8QXP=m`. Built-in (`=y`) is unchanged.
**Step 8.2 — Trigger**
Record: Modular kernel + DT node with those compatibles + autoload via
aliases (udev, `request_module`, dracut/mkinitramfs). Common for distro
`=m` configs. Unprivileged users do not trigger it; boot/device-probe
does.
**Step 8.3 — Failure mode**
Record: Module never autoloads → SCU clocks never register → consumers
fail to probe (UART, net, display, etc.). **MEDIUM–HIGH** on affected
boards; not a panic/UAF. For `=y`, **no effect**.
**Step 8.4 — Risk vs benefit**
Record: **Benefit:** restores documented autoload for a supported
tristate clock driver. **Risk:** very low (1 line, no runtime path).
Ratio is strongly favorable.
---
## Phase 9: Synthesis
**Step 9.1 — Evidence**
FOR:
- Real bug: OF match table not exported; aliases missing for a
`tristate` driver with `module_init`.
- Same pattern already used throughout `drivers/clk/imx/`.
- 1-line, obviously correct; maintainer applied; Reviewed-by Brian
Masney.
- Zero runtime change when built-in.
- Analogous to device-ID additions (exception category: make existing
hardware/driver work).
- Bug present since v5.9 modular support (`e0d0d4d86c766`).
- Applies cleanly to this tree’s `imx8qxp_match[]`.
AGAINST:
- No crash/syzbot/user report; source-level only.
- Impact limited to `CLK_IMX8QXP=m` (many images use `=y`).
- Not a security/UAF/deadlock class bug.
UNRESOLVED:
- Could not fetch lore.kernel.org (403); used b4 + gnuweeb/LKML mirrors
instead.
- No dedicated stable-list thread found.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — 1-line standard macro;
reviewed and applied by clk/imx.
2. Real bug affecting users? **PASS** — `=m` autoload/initramfs alias
generation is broken without it.
3. Important? **PASS** — SoC clock provider failing to load breaks the
platform; severity is functional, not panic, but that is enough for
this class of fix.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features/APIs? **PASS** — publishes existing table.
6. Can apply to this tree? **PASS** — match table, `linux/module.h`, and
modular Kconfig are all here.
**Step 9.3 — Exception category**
Record: Closest match is **device-ID / module-alias enablement** for an
existing driver (not a new driver).
**Step 9.4 — Decision**
This tree is mainline **7.3-rc4**. The driver, OF table, and tristate
module support are here; the fix is a one-line alias export with no
runtime risk. That meets stable criteria for this checkout.
---
### Problem and why it matters
`clk-imx8qxp.c` can be `=m` since v5.9 and already matches DT via
`imx8qxp_match[]`, but never published that table with
`MODULE_DEVICE_TABLE()`. Users with modular i.MX8QXP/QM/DXL kernels do
not get `of:` aliases, so the clock module may not autoload and
consumers never get clocks.
### Stable rules
Small, obviously correct, no new API, real autoload bug, applies to this
tree.
### Risk vs benefit
Benefit is correct module loading on a supported config; risk is
negligible.
### Concerns
No hardware repro and no `Fixes:` tag; neither blocks this pattern.
Sibling LPCG patch is independent.
---
Verification:
- [Phase 1] Parsed tags: Reviewed-by Brian Masney; Link
patch.msgid.link/20260705001705.70400-1-pengpeng@iscas.ac.cn; SOBs Hou
and Abel Vesa; no Fixes/Reported-by/Cc:stable
- [Phase 2] Diff: +1 line `MODULE_DEVICE_TABLE(of, imx8qxp_match)` in
`drivers/clk/imx/clk-imx8qxp.c`; no function body change
- [Phase 3] `git blame`: table from `c2cccb6d0b33b` (2018); modular
support `e0d0d4d86c766` (v5.9) introduced the need for aliases
- [Phase 3] `git merge-base --is-ancestor e0d0d4d86c766 HEAD`: yes; `git
merge-base --is-ancestor cd21fdcbef3fb HEAD`: yes
- [Phase 3] Neighbor `e89485455ac49` is independent LPCG
MODULE_DEVICE_TABLE add
- [Phase 4] `b4 dig -c cd21fdcbef3fb`: matched v1 at that msgid; `-a`
only v1; `-w` listed clk/imx and linux-clk recipients
- [Phase 4] gnuweeb thread: Masney Reviewed-by; Vesa Applied as
cd21fdcbef3fb; no NAK
- [Phase 4] lore.kernel.org fetch: 403; used mirrors
- [Phase 5] `CLK_IMX8QXP` is tristate in `drivers/clk/imx/Kconfig`;
built as `clk-imx-scu` via Makefile; `module_init(imx8qxp_clk_init)`
registers the platform driver
- [Phase 5] `scripts/mod/file2alias.c` `do_of_entry()` emits
`of:N*T*C<compatible>` from MODULE_DEVICE_TABLE
- [Phase 5] Sibling `clk-imx8mm.c` already uses the same macro
- [Phase 6] `git describe HEAD`: v7.3-rc4-27-gf0100363d8c37; `make
kernelversion`: 7.3.0-rc4; file currently contains the added line
- [Phase 6] Apply difficulty: one line after existing `imx8qxp_match[]`
- [Phase 8] Failure mode: modular clock provider not autoloaded;
severity MEDIUM–HIGH on those SoCs, none if `=y`
- UNVERIFIED: live udev/initramfs failure on hardware (author did not
claim a runtime repro); stable-list discussion beyond the gnuweeb
search
**YES**
drivers/clk/imx/clk-imx8qxp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 3ae162625bb1a..15b08fc02f920 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -337,6 +337,7 @@ static const struct of_device_id imx8qxp_match[] = {
{ .compatible = "fsl,imx8qm-clk", &imx_clk_scu_rsrc_imx8qm, },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, imx8qxp_match);
static struct platform_driver imx8qxp_clk_driver = {
.driver = {
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] clk: zynq: handle kasprintf() failure in periph_clk registration
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp: " Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] clk: tegra: support 48MHz clock for pll_p_out1 Sasha Levin
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: William Theesfeld, Brian Masney, Michal Simek, Sasha Levin, sboyd,
bmasney+clk, jbrunet+clk, linux-clk, linux-arm-kernel,
linux-kernel
From: William Theesfeld <william@theesfeld.net>
[ Upstream commit d4d0d335e8d37c6c7e1c5664c7664288eca5a09b ]
zynq_clk_register_periph_clk() ignores the return value of the two
kasprintf() calls used to build the mux and divider clock names, and
passes the resulting (possibly NULL) pointers straight into
clk_register_mux(), clk_register_divider() and clk_register_gate() as
the clock '"'name'"' argument. On allocation failure that name later
gets dereferenced by the clock framework (e.g. in debugfs name
formatting), causing a NULL-pointer dereference.
Check both kasprintf() returns. On failure unwind any allocated name
buffer and the spinlock, then fall through to the existing err label
which sets clks[] to ERR_PTR(-ENOMEM). Freeing the spinlock on the
error path is correct here because no clk_register_*() call has had
a chance to take ownership of it; the success path intentionally
hands it off to the registered clocks.
The neighbouring zynq_clk_register_fclk() in the same file already
uses this per-allocation goto-label cleanup pattern; this change
brings periph_clk into line with it.
Signed-off-by: William Theesfeld <william@theesfeld.net>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Reviewed-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.18.53**. The buggy
`zynq_clk_register_periph_clk()` path is present here, and the patch is
an 8-line error-path fix. Full nine-phase analysis follows.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
- Subsystem: `clk: zynq`
- Action verb: **handle** (bug-fix verb: missing failure handling)
- Intent: check `kasprintf()` in peripheral clock registration so a NULL
name is not passed into the clock framework
- Record: `[clk/zynq] [handle] [check kasprintf failure when registering
periph clocks]`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
- `Reviewed-by: Brian Masney <bmasney@redhat.com>` — clk co-maintainer;
also final `Signed-off-by` into clk tree
- `Reviewed-by: Michal Simek <michal.simek@amd.com>` — ARM/Zynq
architecture maintainer
- `Signed-off-by: William Theesfeld <william@theesfeld.net>` — author
- `Signed-off-by: Brian Masney <bmasney@redhat.com>` — maintainer merge
- No `Fixes:` tag (expected for this review pipeline)
- No `Reported-by:`, `Tested-by:`, `Link:`, or `Cc: stable`
- Pipeline-added markers ignored per instructions: `[Upstream commit
d4d0d335…]`, `Signed-off-by: Sasha Levin`
- Record: maintainer-reviewed, no syzbot/user report, no Fixes tag
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
- Bug: `zynq_clk_register_periph_clk()` ignores two `kasprintf()`
results used as mux/divider names and passes them into
`clk_register_mux/divider/gate`
- Claimed symptom: NULL name later dereferenced in CCF (commit cites
debugfs name formatting) → NULL-pointer dereference
- Fix: check both returns; on failure free the other name buffer and the
spinlock, then the existing `err` label sets `clks[] =
ERR_PTR(-ENOMEM)`
- Author notes the sibling `zynq_clk_register_fclk()` already uses this
goto-cleanup pattern
- Record: `[Bug: unchecked kasprintf] [Symptom: NPD / broken clock init]
[No version info] [Root cause: missing NULL checks before
clk_register_*]`
**Step 1.4: DETECT HIDDEN BUG FIXES**
- Not disguised. Explicit error-path fix. Also closes a spinlock leak on
the failure path (lock is allocated before `kasprintf`; success hands
it to registered clocks, so failure must `kfree(lock)`).
- Record: `[Not hidden; allocation-failure / leak / NPD-class error-path
fix]`
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
- File: `drivers/clk/zynq/clkc.c` — **+8 / -0**
- Function: `zynq_clk_register_periph_clk()`
- Scope: single-file surgical error-path fix
- Record: `[clkc.c +8] [zynq_clk_register_periph_clk] [surgical]`
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Verified against parent `e3b8b15ec6f0a^`:
Before:
```
mux_name = kasprintf(...); /* unchecked */
div_name = kasprintf(...); /* unchecked */
clk_register_mux(..., mux_name, ...);
clk_register_divider(..., div_name, mux_name, ...);
clks[clk0] = clk_register_gate(..., clk_name0, div_name, ...);
```
`err` was only used for `kmalloc(lock)` failure.
After: NULL `mux_name` → `err_mux_name` (`kfree(lock)`); NULL `div_name`
→ `err_div_name` (`kfree(mux_name)` then lock). Then `err` sets
`clks[clk0/clk1] = ERR_PTR(-ENOMEM)`. Success path unchanged.
- Record: `[error path only: skip clk_register_* on kasprintf failure
and unwind]`
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Category: **error path / resource leak** plus **memory safety** (NULL
name into CCF).
Verified CCF behavior in this tree (`drivers/clk/clk.c`):
- `kstrdup_const(NULL)` → `kstrdup(NULL)` returns NULL (`mm/util.c:84`)
- `__clk_register` then fails with `-ENOMEM` if `core->name` is NULL
(`clk.c:4435-4438`)
- NULL **parent** name hits `WARN(!parent_names[i], ...)` then
`clk_cpy_name(..., true)` returns `-EINVAL` (`clk.c:4347-4351,
4307-4309`)
- `zynq_clk_setup()` then does `if (IS_ERR(clks[i])) BUG();`
(`clkc.c:570-575`)
So a fully failed registration already `BUG()`s. The remaining hole is
mixed failure (mux `kasprintf` fails, div succeeds): mux/div
registration fails, gate may still register with a valid DT name and a
never-created parent → no `BUG()`, broken UART/SDIO/SPI/LQSPI/SMC/PCAP
clocks, and the spinlock can leak because `err` is not taken.
- Record: `[error-path leak + WARN + inconsistent clks[] / possible
silent broken periph clocks]`
**Step 2.4: ASSESS THE FIX QUALITY**
- Matches the existing `zynq_clk_register_fclk()` pattern (present since
`f8fe36f6083a70`, v3.13).
- `kfree(lock)` on this path is correct: no `clk_register_*` has run
yet.
- Does not change the success path.
- Regression risk: very low. On failure, `clks[]` becomes
`ERR_PTR(-ENOMEM)` and `zynq_clk_setup()` `BUG()`s — same policy this
driver already uses for lock allocation / registration failure.
- Record: `[obviously correct, 8 lines, error-path only; risk very low]`
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
- Function introduced in `0ee52b157b8ed8` (“clk: zynq: Add clock
controller driver”), 2013, contained in **v3.11-rc1**.
- `kasprintf` calls were unchecked from introduction until this fix.
- Record: `[0ee52b157b8ed8 / v3.11] [bug present for the life of this
driver]`
**Step 3.2: FOLLOW THE FIXES: TAG**
- No `Fixes:` tag. Buggy commit is the original driver add, which **is**
in this tree.
- Record: `[no Fixes tag; original driver commit is in this tree]`
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
- `f8fe36f6083a70` (v3.13): same goto-cleanup for
`zynq_clk_register_fclk()` kasprintf/kmalloc
- `7938e9ce39d67` (2024, in this tree): “Prevent null pointer
dereference caused by kmalloc failure” in `zynq_clk_setup()`
- `a6aa462c3efc1`: parameter update to this function — not a
prerequisite for these checks
- Standalone; not patch N/M
- Record: `[standalone; sibling already had the pattern; related kmalloc
NPD fix already here]`
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
- Only this clk/zynq commit from William Theesfeld in this tree
- Weight comes from reviewers: Michal Simek (Zynq) and Brian Masney
(clk)
- Record: `[author not subsystem maintainer; both relevant maintainers
reviewed]`
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
- Uses existing `kasprintf`, `kfree`, `goto err` — all present
- Mainline context uses `kmalloc_obj`; this tree still uses
`kmalloc(sizeof(*lock), GFP_KERNEL)`. The hunk does not touch that
line, so no `kmalloc_obj` dependency.
- Record: `[no prerequisites; applies standalone against this tree’s
kmalloc() form]`
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
- `b4 dig -c d4d0d335e8d37c6c7e1c5664c7664288eca5a09b` →
https://patch.msgid.link/20260601203500.658135-1-william@theesfeld.net
- `b4 dig -a`: **v1 only** (2026-06-01)
- Thread saved to mbox: Michal Simek Reviewed-by (2026-06-02); Brian
Masney Reviewed-by (2026-06-15). No NAK, no requested respin, no Cc:
stable in review.
- Lore WebFetch returned 403; discussion read from `b4 dig -m` instead
- Record: `[lore msgid 20260601203500.658135-1-william@theesfeld.net]
[v1 applied] [no NAKs] [no stable nomination in thread]`
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
- `b4 dig -w` recipients: Turquette, Boyd, Masney, Simek, linux-clk,
linux-arm-kernel, linux-kernel
- Record: `[clk maintainers + Zynq maintainer were on To/Cc; both
Reviewed-by]`
**Step 4.3: SEARCH FOR THE BUG REPORT**
- No Reported-by / Link / syzbot
- Record: `[no external bug report; found by code inspection]`
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
- Standalone v1 patch, not a series
- Record: `[standalone]`
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
- `lore.kernel.org/stable/` fetch returned 403
- Record: `[UNVERIFIED: stable-list discussion; not used for the
verdict]`
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS**
- Record: `[zynq_clk_register_periph_clk (modified); zynq_clk_setup
(caller)]`
**Step 5.2: TRACE CALLERS**
Call sites in `zynq_clk_setup()` (CLK_OF_DECLARE `"xlnx,ps7-clkc"`,
every Zynq boot):
- lqspi, smc, pcap (single gate)
- sdio0/sdio1, uart0/uart1, spi0/spi1 (two gates)
- Record: `[boot-time OF clock init for Zynq PS peripherals]`
**Step 5.3: TRACE CALLEES**
- `kasprintf`, `kmalloc`/`kfree`, `clk_register_mux/divider/gate`
- Record: `[name alloc + CCF register; gate clocks stored in clks[]]`
**Step 5.4: FOLLOW THE CALL CHAIN**
- `CLK_OF_DECLARE` → `zynq_clk_setup` → `zynq_clk_register_periph_clk`
during early boot on `CONFIG_ARCH_ZYNQ`
- `CONFIG_ARCH_ZYNQ=y` in `arch/arm/configs/multi_v7_defconfig`
- Not a syscall path; unprivileged users cannot trigger it except by
booting the platform (possibly under memory pressure)
- Record: `[reachable on every Zynq boot; not userspace-triggerable
later]`
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
- `zynq_clk_register_fclk()` already checks every `kasprintf`
- After this patch, no remaining unchecked `kasprintf` in
`drivers/clk/zynq/clkc.c`
- Record: `[sibling already fixed; this was the remaining hole]`
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
- `git describe HEAD` → **v6.18.53-5049-g0f0bdc2b0dffb**
- `make kernelversion` → **6.18.53**
- Parent of the candidate still has unchecked `kasprintf` in
`zynq_clk_register_periph_clk`
- Driver built as `obj-$(CONFIG_ARCH_ZYNQ) += zynq/`
- Record: `[yes, buggy code exists in 6.18.y zynq clkc; bug since
v3.11]`
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
- Surrounding function matches; only the 8 check/label lines are needed
- `kmalloc_obj` in the mainline diff context is **not** used in this
file here
- Record: `[clean apply / trivial context]`
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
- `7938e9ce39d67` (kmalloc NPD in `zynq_clk_setup`) is already in this
tree — different allocation
- fclk kasprintf checks already present
- No earlier equivalent of this periph_clk check besides the candidate
itself
- Record: `[no substitute fix for this function’s kasprintf]`
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
- `drivers/clk/zynq/` — platform clock provider for Xilinx Zynq-7000
- Criticality: **IMPORTANT** for Zynq (UART/SDIO/SPI clocks);
**PERIPHERAL** vs core kernel
- Record: `[clk/zynq] [IMPORTANT for affected SoC, not universal]`
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
- File is mature (2013); recent commits are small correctness fixes, not
refactors
- Record: `[mature/stable platform driver]`
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
- Record: `[platform-specific: CONFIG_ARCH_ZYNQ / xlnx,ps7-clkc]`
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
- `kasprintf(GFP_KERNEL, "%s_mux"/"%s_div", clk_name0)` during `__init`
(~tens of bytes)
- Trigger: allocation failure at boot (rare, but same class this file
already treats as fatal via `BUG()`)
- Not triggerable later from unprivileged userspace
- Record: `[rare boot OOM; not a syscall race]`
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Verified, not just the commit text:
- NULL clock **name**: CCF fails `-ENOMEM` (no debugfs NPD on that path
in 6.18)
- NULL **parent** name: `WARN()` then `-EINVAL`
- Some combinations: `clks[i]` is `ERR_PTR` → existing `BUG()` in
`zynq_clk_setup`
- Mixed success: possible boot with broken peripheral clocks + spinlock
leak
- Author’s debugfs-NPD example is **overstated for this CCF**, but WARN
/ leak / inconsistent init are real
- Record: `[WARN + leak + possible BUG() or silently broken
UART/SDIO/SPI] [severity HIGH on trigger, trigger rare]`
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
- Benefit: correct OOM handling for clocks used on every Zynq boot;
closes leak; avoids WARN and half-registered trees
- Risk: 8 lines, error path only, same pattern already in-tree
- Record: `[benefit medium-high if it fires; risk very low; ratio favors
backport]`
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real missing error handling on `kasprintf` in a boot clock path that
already `BUG()`s on `IS_ERR(clks[i])`
- Also a spinlock leak and a `WARN()` on NULL parent names
- 8-line, obviously correct, mirrors in-tree sibling
- Code and bug exist in this 6.18 tree since v3.11
- Applies without `kmalloc_obj` or other prereqs
- Reviewed by Zynq maintainer and clk maintainer
- Same file already received a 2024 allocation-failure NPD fix
AGAINST:
- Trigger is a tiny boot-time allocation failure (rare)
- No user/syzbot report
- Debugfs NPD claim is not how 6.18 CCF handles a NULL `init.name`
(fails `-ENOMEM` first)
- Platform-specific, not core
UNRESOLVED:
- Stable mailing list search (lore 403)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — matches existing fclk
pattern; two maintainer Reviewed-by
2. Fixes a real bug that affects users? **PASS** — unchecked alloc,
leak, WARN, inconsistent `clks[]` on Zynq boot
3. Important issue? **PASS** — boot `WARN`/`BUG()`/broken UART-SDIO-SPI
clocks; same class as the in-tree kmalloc NPD fix
4. Small and contained? **PASS** — +8 lines, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — function and bug present;
context is `kmalloc(sizeof)`, not `kmalloc_obj`
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
- Not a device ID / quirk / DT / build / docs exception
- Record: `[none; qualifies as a normal small bugfix]`
**Step 9.4: MAKE YOUR DECISION**
The patch is a small, maintainer-reviewed error-path fix for code this
6.18 tree has had since v3.11. It does not add features. Risk is
confined to the allocation-failure path. That is appropriate for this
stable tree.
===============================================================
What problem it solves for this tree
===============================================================
`zynq_clk_register_periph_clk()` builds mux/divider names with
`kasprintf()` and, on failure, still calls into the clock framework.
That can `WARN` on a NULL parent, leak the per-clock spinlock, and
either `BUG()` in `zynq_clk_setup()` or leave UART/SDIO/SPI/LQSPI clocks
half-registered. The fix stops before `clk_register_*`, frees the
lock/name, and reports `-ENOMEM` the way lock allocation already does.
===============================================================
Verification
===============================================================
- [Phase 1] Parsed tags: Reviewed-by Masney + Simek; no Fixes/Reported-
by/Cc stable; ignored pipeline Upstream/Sasha SOB
- [Phase 1] Subject/body: handle kasprintf failure; claimed NPD; sibling
fclk pattern
- [Phase 2] Diff: +8 in `zynq_clk_register_periph_clk`; parent still
unchecked (git show e3b8b15^)
- [Phase 2] CCF: `kstrdup(NULL)` returns NULL (`mm/util.c:84`);
`__clk_register` `-ENOMEM` on NULL name (`clk.c:4435-4438`); NULL
parent `WARN` + `-EINVAL` (`clk.c:4347-4351`)
- [Phase 2] `zynq_clk_setup` `BUG()`s on `IS_ERR(clks[i])`
(`clkc.c:570-575`)
- [Phase 3] blame: function from `0ee52b157b8ed8` (v3.11-rc1); fclk
checks from `f8fe36f6083a70` (v3.13)
- [Phase 3] related `7938e9ce39d67` kmalloc NPD fix is ancestor of HEAD
- [Phase 3] no Fixes tag; original driver commit is in this tree
- [Phase 4] `b4 dig -c d4d0d335…`: lore msgid
20260601203500.658135-1-william@theesfeld.net
- [Phase 4] `b4 dig -a`: v1 only
- [Phase 4] `b4 dig -w`: clk + Zynq maintainers on Cc
- [Phase 4] mbox: Simek and Masney Reviewed-by; no NAK
- [Phase 4] lore WebFetch 403; stable-list search UNVERIFIED
- [Phase 5] callers: lqspi/smc/pcap/sdio/uart/spi from `zynq_clk_setup`;
`CLK_OF_DECLARE(zynq_clkc, "xlnx,ps7-clkc")`
- [Phase 5] `CONFIG_ARCH_ZYNQ=y` in multi_v7_defconfig;
`obj-$(CONFIG_ARCH_ZYNQ) += zynq/`
- [Phase 6] `git describe HEAD` = v6.18.53-5049-g0f0bdc2b0dffb;
kernelversion 6.18.53
- [Phase 6] buggy kasprintf present without this patch; no kmalloc_obj
in this file
- [Phase 8] failure mode verified against CCF (debugfs NPD overstated);
WARN/leak/BUG/broken periph clocks verified
- UNVERIFIED: whether this hash was discussed on stable@ (lore 403)
The fix is small, surgical, present in the affected 6.18 code, and meets
the stable criteria.
**YES**
drivers/clk/zynq/clkc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index c28d3dacf0fb2..db40a9e7dd3d9 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -186,7 +186,11 @@ static void __init zynq_clk_register_periph_clk(enum zynq_clk clk0,
spin_lock_init(lock);
mux_name = kasprintf(GFP_KERNEL, "%s_mux", clk_name0);
+ if (!mux_name)
+ goto err_mux_name;
div_name = kasprintf(GFP_KERNEL, "%s_div", clk_name0);
+ if (!div_name)
+ goto err_div_name;
clk_register_mux(NULL, mux_name, parents, 4,
CLK_SET_RATE_NO_REPARENT, clk_ctrl, 4, 2, 0, lock);
@@ -205,6 +209,10 @@ static void __init zynq_clk_register_periph_clk(enum zynq_clk clk0,
return;
+err_div_name:
+ kfree(mux_name);
+err_mux_name:
+ kfree(lock);
err:
clks[clk0] = ERR_PTR(-ENOMEM);
if (two_gates)
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] clk: tegra: support 48MHz clock for pll_p_out1
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] clk: zynq: handle kasprintf() failure in periph_clk registration Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 15:02 ` sashiko-bot
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vdecsys: add missing MODULE_DEVICE_TABLE() Sasha Levin
` (2 subsequent siblings)
7 siblings, 1 reply; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Dmitry Osipenko, Thierry Reding, Jonas Schwöbel,
Svyatoslav Ryhel, Brian Masney, Sasha Levin, pgaikwad, sboyd,
bmasney+clk, jbrunet+clk, thierry.reding, jonathanh, linux-clk,
linux-tegra, linux-kernel
From: Dmitry Osipenko <digetx@gmail.com>
[ Upstream commit 46e31d69a547530fbef499bde888b6bb45a1deac ]
UEFI on Surface2 sets pll_p_out1 to 48MHz which is not supported by kernel
and causes BUG() early on. Add 48MHz clock support for pll_p_out1.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Jonas Schwöbel <jonasschwoebel@yahoo.de>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **v7.3-rc4** (mainline). The change is a one-line
parent-rate case that stops a deterministic early-boot `BUG()` when UEFI
leaves `pll_p_out1` at 48 MHz. Surrounding `_calc_rate` logic has been
in-tree since 2013, so this is a self-contained crash fix, not a new
API.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
Record: **[clk: tegra:] [support] [allow pll_p_out1 parent rate 48 MHz
in `_calc_rate` so firmware-programmed 48 MHz does not hit `BUG()`]**
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- **Acked-by:** Thierry Reding `<treding@nvidia.com>` (Tegra maintainer)
- **Signed-off-by:** Dmitry Osipenko (author; long-time Tegra clk
contributor)
- **Signed-off-by:** Jonas Schwöbel
- **Signed-off-by:** Svyatoslav Ryhel (submitter)
- **Signed-off-by:** Brian Masney (clk maintainer who applied it)
- **No Fixes:** tag (expected)
- **No Cc: stable** (expected)
- **No Reported-by / Tested-by / Link:**
- Ignore any Sasha Levin SOB if present in a candidate wrapper; none on
the real commit `46e31d69a5475`
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- **Bug:** UEFI on Microsoft Surface 2 programs `pll_p_out1` to 48 MHz.
`_calc_rate()` only accepted 9.6 MHz and 28.8 MHz in that switch arm;
48 MHz fell into `default:` which prints an error and calls `BUG()`.
- **Symptom:** kernel `BUG()` during early clock init (boot crash).
- **Root cause:** missing parent-rate case in the out-of-table PLL
calculator used when PLLA’s freq table has no matching input rate.
- **Version info:** none in the message. Cover letter (v2 0/1) says the
case is aligned with NVIDIA downstream 3.4 tegra114 logic.
Step 1.4: DETECT HIDDEN BUG FIXES
Record: **Not a hidden fix.** Subject says “support” but the body is
explicit: this prevents `BUG()`. It is a firmware/hardware compatibility
quirk, not a new clock API.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
Record:
- **File:** `drivers/clk/tegra/clk-pll.c` (+1 / −0)
- **Function:** `_calc_rate()`
- **Scope:** single-file, one `case 48000000:` added next to existing
9.6/28.8 MHz cases
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record:
- **Before:** `parent_rate == 48000000` → `default` → `pr_err` +
`BUG()`.
- **After:** same arm as 9.6/28.8 MHz: `cfreq = parent_rate /
(parent_rate / 1000000)` (1 MHz for 48 MHz), then existing M/N/P
calculation and range checks.
- **Path:** clock-init `clk_set_rate()` on PLLA (and any other PLL using
default `_calc_rate`). Error path only in the sense that the old path
was a fatal default; this is the out-of-table rate calculator, not a
new userspace API.
Verified boot path in this tree:
- Tegra114 `init_table` sets `TEGRA114_CLK_PLL_A` to 564480000 (`clk-
tegra114.c`).
- `tegra_init_from_table()` calls `clk_set_rate()`.
- PLLA parent is `"pll_p_out1"` (`tegra114_audio_plls[]`).
- `pll_a_freq_table[]` only lists 9600000 and 28800000 input rates, so
`_get_table_rate()` fails and `_calc_rate()` runs.
- That is the `BUG()` the commit describes.
Step 2.3: IDENTIFY THE BUG MECHANISM
Record:
- **Category:** (h) hardware/firmware workaround + (g) logic: missing
switch case that caused (kernel `BUG()`).
- **Mechanism:** firmware leaves a valid PLLP divider output of 48 MHz;
the calculator treated it as an impossible reference and crashed.
Step 2.4: ASSESS THE FIX QUALITY
Record:
- **Quality:** obviously correct. Same formula already used for the
other PLLP_OUT1 rates. Tegra maintainer acked it.
- **Minimal:** one line, no API/data-structure change.
- **Regression risk:** very low. Extra parent rate only changes the 48
MHz path; other rates unchanged.
- **Nuance (verified):** tegra114 PLLA uses `pllp_nmp.divm_width = 5`,
so `divm_max` is 31. For 48 MHz, `cfg->m = 48`, which can fail the
existing `cfg->m > divm_max` check and return `-EINVAL` instead of
programming PLLA. That is still far better than `BUG()`:
`clk_pll_set_rate()` then `WARN_ON`s and returns. Completeness of PLLA
programming is a separate issue; the crash is fixed.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
Record:
- Switch and `BUG()` introduced in **`8f8f484bf355e5`** (“clk: tegra:
add Tegra specific clocks”, Prashant Gaikwad, 2013-01-11; in v3.9
era).
- 9.6/28.8 MHz formula adjusted in **`e52d7c04bb3911`** (2015).
- 48 MHz case added by **`46e31d69a5475`** (this commit).
- Buggy missing-rate behavior has been present for ~13 years.
Step 3.2: FOLLOW THE FIXES: TAG
Record: **No Fixes: tag.** Not applicable. Introduced by the original
2013 Tegra clk driver, not a recent regression.
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record: Recent `clk-pll.c` commits are unrelated (GFP helpers,
`determine_rate`, RPM). This commit is **standalone**, not patch X/Y of
a clk series. Author/submitter Tegra DT work around the same time
(TF600T, grouper, tegra114 EMC) does not need to land with this 1-line
clk case.
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record: Dmitry Osipenko is a regular Tegra clk contributor (`Don't allow
zero clock rate`, PLLU config, gate refcounting, etc.). Submitter Ryhel
resubmitted with maintainer Ack. Applied by Brian Masney.
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record: **None.** Diff only adds a `case` label. `_calc_rate`, the
9.6/28.8 arm, and `BUG()` default all exist in this file independently
of later refactors. Can apply standalone.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- **b4 dig:**
https://patch.msgid.link/20260427132447.118272-2-clamor95@gmail.com
- **v1:** 2026-04-17 `20260417073452.23342-2-clamor95@gmail.com`
- **v2 / resend:** 2026-04-27 (cover + patch); Thierry Ack 2026-05-28
- Cover letter: “Fix this by adding 48MHz clock support… aligned with
downstream 3.4 kernel for tegra114 logic”
- **No NAK.** Only reviewer reply was Thierry’s **Acked-by**.
- **No Cc: stable** in review (absence is not a negative).
Step 4.2: CHECK WHO REVIEWED THE PATCH
Record: Recipients included Stephen Boyd, Michael Turquette, Thierry
Reding, Jonathan Hunter, linux-clk, linux-tegra. **Thierry Reding
acked.** Right maintainers were on the thread.
Step 4.3: SEARCH FOR THE BUG REPORT
Record: No bugzilla/syzbot link. Description is a concrete
firmware+hardware trigger (Surface 2 UEFI), not theoretical. Surface 2
has **no in-tree DT** (tegra114 boards present: dalmore, roth, tn7,
asus-tf701t only). Users still hit this with UEFI + out-of-tree/work-in-
progress DT; the clk driver is in-tree for all Tegra114.
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Series is **v2 0/1 + 1/1**. Self-contained. No sibling patch
required for the `BUG()` fix.
Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: **UNVERIFIED** — lore.kernel.org returned 403 (Anubis). Original
linux-tegra/linux-clk thread was retrieved via `b4 dig -m`; no stable-
list discussion in that mbox.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS
Record: **`_calc_rate()`** only.
Step 5.2: TRACE CALLERS
Record:
- Default `pll->params->calc_rate` in `_tegra_clk_register_pll()` for
non-PLLM PLLs.
- Called from **`clk_pll_set_rate()`** and
**`clk_pll_determine_rate()`** when `_get_table_rate()` misses.
- Tegra210 PLLs use `tegra210_pll_fixed_mdiv_cfg` instead; this path is
**tegra20/30/114/124**.
- Context: boot clock init and later audio PLL rate changes. Not a
syscall-hot path, but **every boot** on affected firmware.
Step 5.3: TRACE CALLEES
Record: Integer divider math only (`cfreq`, `m`/`n`/`p`). No allocation,
no locks inside `_calc_rate`. `clk_pll_set_rate` takes `pll->lock` after
calc.
Step 5.4: FOLLOW THE CALL CHAIN
Record:
`tegra114_clock_init` → `tegra_clk_apply_init_table` →
`tegra_init_from_table` → `clk_set_rate(pll_a, 564480000)` →
`clk_pll_set_rate` → `_calc_rate`.
**Reachable from boot on Tegra114 when parent is 48 MHz.** Unprivileged
userspace is not required; firmware state is enough.
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: Same `_calc_rate` switch is the single shared calculator. No
duplicate 48 MHz holes in sibling functions. Freq tables still lack 48
MHz PLLA rows (out-of-table path is intentional, per comment).
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record:
- Tree: **`git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`**,
`VERSION=7 PATCHLEVEL=3 SUBLEVEL=0 EXTRAVERSION=-rc4`.
- `git merge-base --is-ancestor 46e31d69a5475 HEAD` → **already in this
checkout** (landed in v7.3-rc1).
- The **vulnerable pattern** (`_calc_rate` switch + `BUG()` on unknown
parent rate) is here and dates to 2013. The 48 MHz hole is what this
commit fills. This is not post-7.3 code; older stables that still lack
the case have the bug.
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: **Clean apply** on any tree that still has the 9.6/28.8 arm
without `case 48000000:`. Context in this file is stable. No backport
rewrite needed.
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: This **is** the fix (`46e31d69a5475`). No earlier alternate 48
MHz handling in `clk-pll.c`.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: **drivers/clk/tegra** — SoC clock driver. **IMPORTANT** for
NVIDIA Tegra 20/30/114/124, not core-mm/VFS. Boot-blocking on those
SoCs.
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: Mature driver (2013), still receiving fixes. 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: **Platform/firmware-specific:** Tegra boards whose firmware sets
`pll_p_out1` to 48 MHz. Documented trigger is **Surface 2 UEFI**. Any
tegra20/30/114/124 PLL using default `_calc_rate` with that parent rate.
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record: **Every boot** with that firmware, during CAR init table
application. Not a race. Not unprivileged-syscall; **unusable machine**
if you boot Linux on that hardware.
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record: **`BUG()` on boot → CRITICAL** for affected devices (oops/panic,
system does not come up). After the fix, worst case on tegra114 is
`-EINVAL` + `WARN_ON` if M overflows, not a hard `BUG()`.
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- **Benefit:** high for those devices (boot vs die); moderate overall
(narrow hardware).
- **Risk:** very low (one `case` label, existing formula).
- **Ratio:** strongly favors taking it. Matches the **hardware quirk**
stable exception (USB/PCI quirk analogue: one-line table addition so
existing driver survives real firmware).
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
**FOR:**
- Deterministic early-boot `BUG()`, verified via init_table →
`clk_set_rate(pll_a)` → `_calc_rate`.
- One-line, obviously correct, same math as 9.6/28.8 MHz.
- Acked by Tegra maintainer Thierry Reding; applied by clk maintainer.
- Code path present since 2013; no prerequisites.
- Hardware/firmware quirk, not a new UAPI.
**AGAINST:**
- Subject looks like a feature (“support 48MHz”).
- Surface 2 DT is not in mainline; user population is small.
- PLLA 5-bit M may still reject the computed `m=48` (WARN instead of
BUG). Incomplete programming, but the crash is gone.
- No syzbot/CVE/Fixes tag (expected for this review queue).
**UNRESOLVED:**
- Stable-list discussion (lore 403).
- Whether every tegra114 board with 48 MHz pll_p_out1 can actually lock
PLLA after this; downstream 3.4 used this same case.
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — one case, maintainer Ack, in
mainline since v7.3-rc1.
2. Fixes a real bug that affects users? **PASS** — UEFI Surface 2 (and
same firmware pattern) `BUG()` at boot.
3. Important issue? **PASS** — boot `BUG()`, severity CRITICAL for that
hardware.
4. Small and contained? **PASS** — +1 line, one function.
5. No new features or APIs? **PASS** — quirk: accept a parent rate the
hardware/firmware already programs. No new sysfs/uapi/driver.
6. Can apply to the local tree? **PASS** — surrounding `_calc_rate`
exists; patch is a single case add (already present on this 7.3-rc4
HEAD because the candidate came from this lineage).
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: **Hardware quirk/workaround** (firmware-programmed clock the
driver must not `BUG()` on). Analogous to USB/PCI quirk table entries.
Step 9.4: MAKE YOUR DECISION
This is a surgical boot-crash fix for a real firmware configuration. It
meets stable rules and the quirk exception. Limited hardware does not
disqualify it: obscure-device quirks are routinely taken. Incomplete
PLLA table entries do not restore the `BUG()`, so they are not a reason
to reject.
What problem it solves: Tegra `_calc_rate()` treated 48 MHz `pll_p_out1`
as illegal and called `BUG()` while setting PLLA during clock init.
Surface 2 UEFI programs that rate.
Risk vs benefit: one `case` vs a boot panic. Benefit dominates.
Concerns: no in-tree Surface 2 DT; possible later `-EINVAL` if M
overflows. Neither makes the `BUG()` acceptable to leave in stable.
Verification:
- [Phase 1] Tags: Acked-by Thierry Reding; SOBs Osipenko, Schwöbel,
Ryhel, Masney; no Fixes/Cc-stable/Reported-by
- [Phase 2] Diff: +1 line `case 48000000:` in `_calc_rate()` in
`drivers/clk/tegra/clk-pll.c`
- [Phase 2] tegra114 `init_table` sets PLLA 564480000; parent
`pll_p_out1`; freq table has no 48 MHz input → `_calc_rate` on boot
- [Phase 2] `divm_width=5` → m=48 may `-EINVAL` after the fix; `BUG()`
still avoided
- [Phase 3] git blame: switch/`BUG()` from `8f8f484bf355e5` (2013); this
commit `46e31d69a547530fbef499bde888b6bb45a1deac`
- [Phase 3] `git merge-base --is-ancestor 46e31d69a5475 HEAD` → already
in this 7.3-rc4 tree
- [Phase 3] No Fixes tag; standalone; no prerequisite commits
- [Phase 4] `b4 dig -c 46e31d69a5475` → lore patch-id match; v1
2026-04-17, v2 2026-04-27
- [Phase 4] `b4 dig -a`: v1 then v2 (“aligned with downstream 3.4”)
- [Phase 4] `b4 dig -w`: linux-clk, linux-tegra, Thierry, Boyd, Hunter
- [Phase 4] mbox: Thierry Acked-by; no NAK
- [Phase 4] lore WebFetch 403; stable list UNVERIFIED
- [Phase 5] callers: `clk_pll_set_rate`, `clk_pll_determine_rate`;
default calc_rate for non-PLLM
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; Makefile
7.3.0-rc4; file and BUG() default present
- [Phase 6] No other 48 MHz pll_p_out1 fix in this file
- [Phase 7] clk/tegra, important for Tegra SoCs
- [Phase 8] Failure mode: `BUG()` at boot, CRITICAL for affected
firmware
- UNVERIFIED: stable@ discussion; whether PLLA actually locks at 564.48
MHz with 48 MHz parent on Surface 2 hardware
**YES**
drivers/clk/tegra/clk-pll.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 591b9f0c155a0..e48ca3f4b5f86 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -575,6 +575,7 @@ static int _calc_rate(struct clk_hw *hw, struct tegra_clk_pll_freq_table *cfg,
break;
case 9600000:
case 28800000:
+ case 48000000:
/*
* PLL_P_OUT1 rate is not listed in PLLA table
*/
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vdecsys: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] clk: tegra: support 48MHz clock for pll_p_out1 Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp-lpcg: " Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] clk: sunxi-ng: fix ccu probe clock unregister on error Sasha Levin
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Brian Masney, Sasha Levin, y.oudjana, sboyd,
bmasney+clk, jbrunet+clk, matthias.bgg, angelogioacchino.delregno,
linux-clk, linux-mediatek, linux-kernel, linux-arm-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 096ab64a0e69460bfb3557d09e17d2933280a1c6 ]
The driver has an OF match table wired to .of_match_table, but does
not export the table with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE(of, ...) entry so module alias
information is generated for OF based module autoloading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the platform driver, and the missing module alias publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
vdecsys driver is present here; the change is a one-line
`MODULE_DEVICE_TABLE(of, ...)` so OF module aliases are generated.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: [clk: mediatek] [add] [missing MODULE_DEVICE_TABLE() for
mt6735-vdecsys]
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Signed-off-by: Pengpeng Hou \<pengpeng@iscas.ac.cn\> (author)
- Reviewed-by: Brian Masney \<bmasney@redhat.com\> (clk co-maintainer;
also committed the patch)
- Signed-off-by: Brian Masney \<bmasney@redhat.com\>
- No Fixes:, Reported-by:, Tested-by:, Link:, or Cc: stable
- Ignore any pipeline-added Sasha Levin SOB (none on this commit)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: OF match table is wired to `.of_match_table` but not exported
with `MODULE_DEVICE_TABLE()`, so modpost does not emit `of:` aliases.
- Symptom: when `CONFIG_COMMON_CLK_MT6735_VDECSYS=m`, udev/kmod cannot
autoload `clk-mt6735-vdecsys.ko` from the DT compatible string;
vdecsys clocks/resets never register unless the module is loaded by
hand.
- Version info: none in the message. Driver introduced in
`0bd9b1211f741` (contained in v6.13-rc1); this fix is contained in
v7.3-rc1.
- Root cause: omission at driver introduction, not a later regression.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. It is an explicit autoload/alias publication fix.
Author calls it a “source-level fix” with no hardware reproduction; that
is accurate (the missing macro is visible in the file) and does not make
the bug theoretical.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/clk/mediatek/clk-mt6735-vdecsys.c` (+1 / -0)
- Function/data: file-scope after `of_match_mt6735_vdecsys[]`
- Scope: single-file, one-line surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: Before: match table exists and is used for in-kernel platform
matching, but no module alias is published. After:
`MODULE_DEVICE_TABLE(of, of_match_mt6735_vdecsys)` aliases the table for
`file2alias`/modpost. Affects the modular autoload path only;
probe/remove logic is unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: [Hardware enablement / module autoload] `MODULE_DEVICE_TABLE` is
the OF analog of exporting PCI/USB IDs. Without it, `modules.alias`
lacks `of:N*T*Cmediatek,mt6735-vdecsys`. Platform matching still works
if the module is already loaded or built-in.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct; matches the established MediaTek clk pattern
(`65c9ad77cbc0e` “Add MODULE_DEVICE_TABLE() where appropriate”).
Compile-time alias generation only; no locking, no API, no runtime path
change. Regression risk is negligible. Sibling `clk-mt6735-imgsys.c` /
`clk-mt6735-mfgcfg.c` still lack the macro in this tree; that does not
make this hunk wrong.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record: Match table from `0bd9b1211f741` (Yassine Oudjana, 2024-11-06;
contained in v6.13-rc1). `MODULE_DEVICE_TABLE` line is this commit
`096ab64a0e694`. Bug present from first introduction of the driver.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag on this commit. A later unrelated series (`[PATCH
07/32] clk: mediatek: mt6735: Add missing MODULE_DEVICE_TABLE()`) cited
`Fixes: 0bd9b1211f74` for the same omission across four files. Intro
commit is an ancestor of HEAD.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Only two commits on this file: intro + this fix. Sibling same-
day commit `36cf8d279ff90` does the same for vencsys. Standalone for
vdecsys; no prerequisite refactor.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Pengpeng Hou is not the MT6735 clk maintainer (that is Yassine
Oudjana). Same-day vencsys twin plus several other `MODULE_DEVICE_TABLE`
additions (rtc, vdpa, watchdog). Brian Masney, clk co-maintainer,
reviewed and applied it.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Macro has long existed in `include/linux/module.h`.
Applies standalone.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- Lore/msgid:
https://patch.msgid.link/20260704124450.11035-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: v1 only, applied as submitted
- Thread (mbox): author post + Brian Masney `Reviewed-by:` on
2026-07-06. No NAK, no stable nomination, no requested revisions.
- WebFetch of lore/spinics: 403/500; thread recovered via `b4 dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): linux-clk, linux-mediatek, Stephen Boyd, Brian
Masney, AngeloGioacchino Del Regno, Matthias Brugger, Yassine Oudjana.
Review came from clk co-maintainer Brian Masney.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No Reported-by, no bugzilla/syzbot. Author states source-level
evidence only. No in-tree DTS uses `mediatek,mt6735-vdecsys` (binding
only in `Documentation/devicetree/bindings/clock/mediatek,syscon.yaml`).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Twin vencsys patch applied in the same pile. Later 07/32 series
(Akari Tsuyukusa, Aug 2026) would have covered
imgsys+mfgcfg+vdecsys+vencsys together; vdecsys/vencsys were already
fixed here. This commit does not depend on that series.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No dedicated stable thread found for this subject. UNVERIFIED
whether an AUTOSEL pick already exists; that must not drive the verdict.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: No function body changed. Added `MODULE_DEVICE_TABLE` next to
`of_match_mt6735_vdecsys`. Driver uses `mtk_clk_simple_probe` /
`mtk_clk_simple_remove` via `module_platform_driver`.
**Step 5.2: TRACE CALLERS**
Record: Probe is invoked by the platform core when a device matches
`mediatek,mt6735-vdecsys`. Autoload (udev/kmod) is the path that was
broken for `=m`. Built-in (`=y`) matching already worked via
`.of_match_table`.
**Step 5.3: TRACE CALLEES**
Record: `mtk_clk_simple_probe()` (`drivers/clk/mediatek/clk-mtk.c`)
registers the vdecsys gates and resets from `vdecsys_clks`. Unchanged by
this patch.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: DT node `compatible = "mediatek,mt6735-vdecsys"` → (modular)
udev alias lookup → `clk-mt6735-vdecsys.ko` → platform probe → video-
decoder clocks. Reachable on MT6735 boards that describe that syscon; no
in-tree board DTS. Unprivileged userspace cannot trigger it except by
loading the module.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Most MediaTek clk drivers already have `MODULE_DEVICE_TABLE`.
MT6735 core clocks (apmixedsys/infracfg/pericfg/topckgen) have it;
imgsys and mfgcfg in this tree still do not. Same class of omission as
the 2023 sweep `65c9ad77cbc0e`.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes. File exists; driver from `0bd9b1211f741` (v6.13-rc1).
Kconfig `COMMON_CLK_MT6735_VDECSYS` is **tristate**, Makefile maps it to
`clk-mt6735-vdecsys.o`. HEAD already contains `096ab64a0e694`
(v7.3-rc1); the pre-fix omission is the code this candidate addresses.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply. One added line at a stable match-table site; no
refactor around it.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This exact line is present (commit `096ab64a0e694`). No
alternate fix. imgsys/mfgcfg still missing aliases — separate omissions.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: [drivers/clk/mediatek] [PERIPHERAL] — SoC-specific video-decoder
clock/reset driver for MT6735 (Helio T1 era). Not core mm/net/fs.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: MediaTek clk is actively developed; this file itself is quiet
(two commits). MT6735 clk is listed Maintained (Yassine Oudjana).
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Users of MT6735 with `CONFIG_COMMON_CLK_MT6735_VDECSYS=m`. No
in-tree defconfig enables `COMMON_CLK_MT6735`; no in-tree DTS
instantiates the node. Built-in builds are unaffected.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Boot/enumeration with the DT node while the driver is a module.
Not a race. Not unprivileged-exploitable. Workaround: `modprobe clk-
mt6735-vdecsys` or build `=y`.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Video-decoder clocks/resets never appear unless the module is
loaded manually → decode/VPU consumers fail clock lookup. Not
panic/UAF/corruption. Severity: MEDIUM for modular MT6735 vdec; LOW
overall population. Failure is “device feature does not come up,” same
class as missing device IDs.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: Benefit: modular autoload for this clock controller. Risk: very
low (one compile-time alias line). Ratio: favorable because risk is
essentially zero.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real autoload bug on a tristate, `module_platform_driver` clock driver
- One-line, obviously correct, clk co-maintainer Reviewed-by + applied
- Same class as device-ID / quirk enablement (OF alias publication)
- Established MediaTek clk precedent (`65c9ad77cbc0e`: “allow auto-load
at boot”)
- Zero runtime/API/locking risk; applies cleanly
- Buggy driver exists in this 7.3 tree (since v6.13)
AGAINST:
- Not crash/security/corruption/deadlock
- No user report or hardware test
- Niche SoC; no in-tree DTS/defconfig
- Built-in configs already work
- Author framed it as source-level only
UNRESOLVED:
- Whether AUTOSEL/stable already discussed this hash (lore.kernel.org
stable search not useful)
- Direct fetch of lore HTML (403)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one-line alias export;
reviewed by clk co-maintainer. No HW test, but correctness does not
depend on runtime.
2. Fixes a real bug that affects users? **PASS** — modular OF autoload
is broken by construction when the table is not exported.
3. Important issue? **PASS** (MEDIUM) — hardware clocks do not come up
on the intended modular path; same practical class as missing device
IDs, not a panic.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS** — publishes an existing match
table.
6. Can apply to the local tree? **PASS** — file and match table exist;
hunk is identical.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Device-ID / module-alias publication for an existing driver (OF
`MODULE_DEVICE_TABLE`). That exception applies.
**Step 9.4: MAKE YOUR DECISION**
The bug is real for `=m` builds, the fix is trivial and already the
subsystem’s standard pattern, and this tree has the driver. Conservative
stable practice still takes this class because the downside is nil and
the upside is the driver actually binding.
===============================================================
What the commit solves, and why it matters here
===============================================================
Without `MODULE_DEVICE_TABLE(of, of_match_mt6735_vdecsys)`, modpost
never emits the `of:N*T*Cmediatek,mt6735-vdecsys` alias. On this tree
the Kconfig is tristate, so a modular vdecsys driver will not autoload;
video-decoder clocks and resets never register until someone loads the
module by name. Built-in matching via `.of_match_table` was already
fine.
It meets stable rules: obviously correct, one line, no new API, no
dependencies, and it is the OF equivalent of adding a device ID to an
existing driver. Risk is compile-time alias generation only.
Concerns: no in-tree board uses this compatible, no reporter, not a
crash. Those lower breadth of impact; they do not make the hunk unsafe
or non-bugfix.
Verification:
- [Phase 1] Parsed commit `096ab64a0e694`: SOB Pengpeng Hou; Reviewed-
by/SOB Brian Masney; no Fixes/Reported-by/Cc: stable
- [Phase 2] Diff: +1 `MODULE_DEVICE_TABLE(of, of_match_mt6735_vdecsys)`
in `clk-mt6735-vdecsys.c`
- [Phase 3] `git blame`: match table from `0bd9b1211f741`; `git describe
--contains` intro v6.13-rc1, fix v7.3-rc1
- [Phase 3] `git merge-base --is-ancestor 0bd9b1211f741 HEAD` and
`096ab64a0e694 HEAD`: both true
- [Phase 3] `git log --oneline -- drivers/clk/mediatek/clk-
mt6735-vdecsys.c`: only intro + this fix
- [Phase 3] Sibling `36cf8d279ff90` vencsys same change; earlier sweep
`65c9ad77cbc0e`
- [Phase 4] `b4 dig -c 096ab64a0e694`: msgid
`20260704124450.11035-1-pengpeng@iscas.ac.cn`; v1 only
- [Phase 4] `b4 dig -m`: Brian Masney Reviewed-by; no NAK/stable
discussion
- [Phase 4] WebFetch lore/spinics: 403/500; used mbox instead
- [Phase 5] Kconfig `COMMON_CLK_MT6735_VDECSYS` tristate; Makefile
builds `clk-mt6735-vdecsys.o`; probe is `mtk_clk_simple_probe`
- [Phase 5] `MODULE_DEVICE_TABLE` in `include/linux/module.h` creates
file2alias symbol
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` = 7.3.0-rc4
- [Phase 6] No in-tree dts/dtsi for mt6735-vdecsys; binding listed in
`mediatek,syscon.yaml`
- [Phase 6] No defconfig sets `COMMON_CLK_MT6735`
- [Phase 6] imgsys/mfgcfg still lack MODULE_DEVICE_TABLE (verified by
grep)
- [Phase 7] MAINTAINERS: COMMON CLK FRAMEWORK includes Brian Masney;
MT6735 CLOCK maintained by Yassine Oudjana
- [Phase 8] Failure mode: modular autoload miss → vdec clocks absent;
severity MEDIUM/narrow
- UNVERIFIED: whether stable@ already queued this hash; lore stable
search did not return this commit
**YES**
drivers/clk/mediatek/clk-mt6735-vdecsys.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/mediatek/clk-mt6735-vdecsys.c b/drivers/clk/mediatek/clk-mt6735-vdecsys.c
index 8817085fc1db4..d53ebf92e878d 100644
--- a/drivers/clk/mediatek/clk-mt6735-vdecsys.c
+++ b/drivers/clk/mediatek/clk-mt6735-vdecsys.c
@@ -63,6 +63,7 @@ static const struct of_device_id of_match_mt6735_vdecsys[] = {
{ .compatible = "mediatek,mt6735-vdecsys", .data = &vdecsys_clks },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, of_match_mt6735_vdecsys);
static struct platform_driver clk_mt6735_vdecsys = {
.probe = mtk_clk_simple_probe,
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp-lpcg: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vdecsys: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 13:52 ` Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] clk: sunxi-ng: fix ccu probe clock unregister on error Sasha Levin
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:52 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Frank Li, Brian Masney, Peng Fan, Abel Vesa,
Sasha Levin, abelvesa, sboyd, bmasney+clk, jbrunet+clk, linux-clk,
imx, linux-arm-kernel, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit e89485455ac49925565061733c0d73d568333fc0 ]
The driver has a match table for the of bus wired into its driver
structure, but the table is not exported with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE() entry so module alias information
is generated for automatic module loading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the driver registration structure, and the missing module alias
publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Link: https://patch.msgid.link/20260704150344.59563-1-pengpeng@iscas.ac.cn
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Local tree
This checkout is **v7.3-rc4-27-gf0100363d8c37** (`VERSION=7 PATCHLEVEL=3
SUBLEVEL=0 EXTRAVERSION=-rc4`). The candidate is `e89485455ac49` (`clk:
imx: imx8qxp-lpcg: add missing MODULE_DEVICE_TABLE()`). The file and
match table exist here; the one-line fix applies as written.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 — Subject**
- Record: `[clk: imx: imx8qxp-lpcg] [add] [export OF match table via
MODULE_DEVICE_TABLE so module aliases are generated]`
**Step 1.2 — Tags**
- Record:
- `Reviewed-by: Frank Li <Frank.Li@nxp.com>` (i.MX)
- `Reviewed-by: Brian Masney <bmasney@redhat.com>` (Red Hat / clk)
- `Reviewed-by: Peng Fan <peng.fan@nxp.com>` (i.MX clk)
- `Link: https://patch.msgid.link/20260704150344.59563-1-
pengpeng@iscas.ac.cn`
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>` (clk-imx
maintainer, applied it)
- No `Fixes:`, no `Reported-by:`, no `Cc: stable` (absence expected)
- Ignore any Sasha Levin SOB if present (none on the real commit)
**Step 1.3 — Body**
- Record: The OF match table is wired into `platform_driver` but not
published with `MODULE_DEVICE_TABLE()`. Without that, modpost does not
emit `of:` aliases, so kmod/udev cannot autoload the module from DT
compatible strings.
- Symptom: `CONFIG_CLK_IMX8QXP=m` → LPCG clock module does not load from
DT; consumers never get those clocks unless the module is loaded by
hand.
- Version info: none in the message.
- Root cause: missing alias publication, not a runtime logic bug. Author
states it is a source-level finding, not a live hardware repro.
**Step 1.4 — Hidden bug fix?**
- Record: Not disguised. It is a real autoload/modalias bug stated
plainly. The “source-level” wording means it was found by inspection,
not that it is theoretical: the table is used and not exported.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1 — Inventory**
- Record: `drivers/clk/imx/clk-imx8qxp-lpcg.c` — **+1 / −0**. One line
after `imx8qxp_lpcg_match[]`. Surgical, single-file.
**Step 2.2 — Flow**
- Record: Before: `imx8qxp_lpcg_match` is only `of_match_table` (used
after the module is already loaded). After: same table is also a
`MODULE_DEVICE_TABLE(of, …)` so file2alias/modpost emits aliases at
build time. No probe/runtime path change.
**Step 2.3 — Bug mechanism**
- Record: **Hardware/module autoload (OF alias publication)**. Match
table exists (`fsl,imx8qxp-lpcg-adma/conn/lsio` and `fsl,imx8qxp-
lpcg`) but is not exported. Same class as adding device IDs: userspace
cannot match hardware to the module.
**Step 2.4 — Fix quality**
- Record: Obviously correct; matches every other i.MX clk driver that
already has `MODULE_DEVICE_TABLE(of, …)` (`clk-imx8qxp.c`, `clk-
imx8mm.c`, etc.). No API, locking, or structural change. Regression
risk is negligible: `=y` ignores aliases; `=m` only adds autoload.
---
## PHASE 3: GIT HISTORY
**Step 3.1 — Blame**
- Record: Match table from `1e3121bfe51a3` (“clk: imx: add imx8qxp lpcg
driver”, 2018, in **v5.0-rc1**). Module metadata from `e0d0d4d86c766`
(“Support building i.MX8QXP clock driver as module”, **v5.10-rc1**).
`builtin_platform_driver` → `module_platform_driver` in
`eee377b8f44e7` (2021, Fixes: that module-support commit). The hole
has been there for the whole modular lifetime of the driver.
**Step 3.2 — Fixes: tag**
- Record: None. Buggy omission is the module-support work
(`e0d0d4d86c766` / `eee377b8f44e7`). Both are in this tree.
**Step 3.3 — File history**
- Record: Recent file commits are unrelated (pm_runtime, ioremap).
Sibling `cd21fdcbef3fb` does the same for `clk-imx8qxp.c`. Standalone;
not “patch X/Y”.
**Step 3.4 — Author**
- Record: Pengpeng Hou also added the same fix for `clk-imx8qxp.c` and
MediaTek mt6735 clocks. Not the clk-imx maintainer; Abel Vesa applied
it with three subsystem/vendor reviews.
**Step 3.5 — Dependencies**
- Record: Needs `imx8qxp_lpcg_match`, `module_platform_driver`, and
`<linux/module.h>`. All present here. Applies as a single line.
---
## PHASE 4: MAILING LIST / EXTERNAL
**Step 4.1 — Original thread**
- Record: `b4 dig -c e89485455ac49` →
https://patch.msgid.link/20260704150344.59563-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: **v1 only**, 2026-07-04.
- Review: Peng Fan, Brian Masney, Frank Li: Reviewed-by only. Abel Vesa:
“Applied, thanks!” No NAK, no stable tag, no risk discussion.
**Step 4.2 — Recipients (`b4 dig -w`)**
- Record: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Frank
Li, Sascha Hauer, Fabio Estevam; `linux-clk`, `imx@lists.linux.dev`,
`linux-arm-kernel`, `linux-kernel`. Right maintainers and lists.
**Step 4.3 — Bug report**
- Record: No Reported-by / bugzilla / syzbot. Inspection-only, as the
commit says.
**Step 4.4 — Series**
- Record: Standalone 1/1. Related but independent sibling on `clk-
imx8qxp.c`.
**Step 4.5 — Stable list**
- Record: Thread has no stable nomination. Direct lore.kernel.org
fetches returned **403**; no separate stable-list discussion verified.
---
## PHASE 5: CODE SEMANTICS
**Step 5.1 — Functions**
- Record: No C function changed. Macro `MODULE_DEVICE_TABLE(of,
imx8qxp_lpcg_match)` next to the existing `of_device_id` table. Driver
still uses `module_platform_driver(imx8qxp_lpcg_clk_driver)`.
**Step 5.2 — “Callers”**
- Record: Build-time: `scripts/mod/file2alias.c` looks up
`__mod_device_table__` and emits `of:` aliases. Runtime: kmod/udev on
DT `compatible`. Probe path unchanged. Makefile: `clk-imx-lpcg-
scu-$(CONFIG_CLK_IMX8QXP)` → aliases belong to `clk-imx-lpcg-scu.ko`.
**Step 5.3 — Callees**
- Record: None at runtime. Macro only creates an aliased copy of the
table for modpost.
**Step 5.4 — Reachability**
- Record: Yes, from userspace/DT: i.MX8QXP/8QM DTS uses `fsl,imx8qxp-
lpcg` (e.g. `arch/arm64/boot/dts/freescale/imx8qm-ss-*.dtsi`). With
`=m`, udev should load the module from that compatible. `arm64
defconfig` has `CONFIG_CLK_IMX8QXP=y`, so the default distro/defconfig
path is unaffected; custom/distro `=m` is the affected path.
**Step 5.5 — Similar patterns**
- Record: Same missing-table pattern fixed in `clk-imx8qxp.c` and
several other `drivers/clk/imx/*` files already have
`MODULE_DEVICE_TABLE`. This was the LPCG outlier.
---
## PHASE 6: THIS TREE
**Step 6.1 — Buggy code here?**
- Record: Yes. Pre-commit parent still has the table without
`MODULE_DEVICE_TABLE`. Module support is in this tree. Bug dates to
v5.10 module support (autoload hole after v5.16
`module_platform_driver`).
**Step 6.2 — Backport difficulty**
- Record: **Clean apply**. Context around the match table matches the
hunk.
**Step 6.3 — Already fixed another way?**
- Record: No other autoload fix on this file. This commit *is* the fix
(already upstream in 7.3-rc). No substitute.
---
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1**
- Record: `[drivers/clk/imx — i.MX8QXP LPCG] [IMPORTANT for
i.MX8QXP/8QM; PERIPHERAL vs core kernel]`. Clock provider: if the
module never loads, LPCG-gated peripherals fail probe.
**Step 7.2**
- Record: clk-imx is active (7.3 pulls). This driver itself is mature
(2018).
---
## PHASE 8: IMPACT AND RISK
**Step 8.1 — Who**
- Record: Config- and platform-specific: `CONFIG_CLK_IMX8QXP=m` on
i.MX8QXP/8QM/8DXL SCU clocks. Not universal. Defconfig `=y` does not
need aliases.
**Step 8.2 — Trigger**
- Record: Boot/enumeration with LPCG DT nodes and the driver as a
module. Unprivileged users cannot trigger it; it is bring-up/udev.
Common on modular distro kernels, uncommon on typical vendor `=y`
images.
**Step 8.3 — Failure mode**
- Record: Module not autoloaded → LPCG clocks missing → dependent
devices fail probe (UART/DMA/etc. depending on board). Not an
oops/UAF. Severity **MEDIUM** (functional miss for `=m`), **HIGH** on
those boards if clocks are modular.
**Step 8.4 — Risk vs benefit**
- Record: Benefit: modular i.MX8QXP clocks actually bind. Risk: **very
low** (one line of metadata). Ratio favors taking it; same rationale
as the “add a device ID” stable exception.
---
## PHASE 9: SYNTHESIS
**Step 9.1 — Evidence**
For:
- Real autoload bug: match table used, aliases not published.
- One line, obviously correct, reviewed by NXP i.MX and Red Hat.
- DT hardware uses these compatibles.
- Driver is tristate (`CLK_IMX8QXP`); `module_platform_driver` is
intentional.
- Same pattern as device-ID publication, which stable explicitly allows.
- Applies cleanly; no new API; no runtime behavior change for `=y`.
Against:
- No user/syzbot report; source-level only.
- Not crash/corruption/security.
- `arm64 defconfig` is `=y`.
- No `Cc: stable` (not a negative by itself).
Unresolved:
- Could not search lore.kernel.org/stable (HTTP 403). Original thread
had no stable discussion.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — one-line, three reviews,
maintainer applied.
2. Real bug that affects users? **PASS** — `=m` does not autoload; DTS
uses the IDs.
3. Important? **PASS** (as device-ID/autoload class; not a panic).
Missing clocks on modular i.MX8 is user-visible, not trivia.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features/APIs? **PASS** — metadata only.
6. Can apply to this tree? **PASS** —
file/table/`module_platform_driver` all present.
**Step 9.3 — Exceptions**
- Record: Device-ID / module-alias publication for an existing driver
(stable exception #1 in spirit).
**Step 9.4 — Decision**
This is a one-line, reviewed autoload fix for a long-standing modular
i.MX8QXP LPCG driver. It matches what stable already takes for device
IDs: hardware described in DT can bind without a manual `modprobe`. Risk
is negligible on this tree.
---
## What the commit solves for this tree
Without `MODULE_DEVICE_TABLE()`, `clk-imx-lpcg-scu.ko` has no
`of:N*T*Cfsl,imx8qxp-lpcg*` aliases. kmod will not load it from DT. LPCG
is a clock provider, so that shows up as missing clocks and failed
consumer probe, not as a crash. The fix only publishes the table the
driver already uses.
It meets the stable bar: obviously correct, tiny, no new API, real
modular-load bug, applies here.
Risk vs benefit: benefit is correct autoload for `CONFIG_CLK_IMX8QXP=m`;
risk is essentially zero.
Concerns: no live repro, default `=y`. That does not outweigh a one-line
alias export that other i.MX clk drivers already have.
---
## Verification
- [Phase 1] `git log -1 e89485455ac49`: tags as above; no
Fixes/Reported-by/Cc:stable.
- [Phase 2] `git show e89485455ac49`: +1 `MODULE_DEVICE_TABLE(of,
imx8qxp_lpcg_match)`.
- [Phase 2] Read `clk-imx8qxp-lpcg.c`: table used as `.of_match_table`;
`module_platform_driver`.
- [Phase 2] `include/linux/module.h`: macro creates `__mod_device_table`
alias for modpost.
- [Phase 3] `git blame` 350–370: table `1e3121bfe51a3` (v5.0); line
`e89485455ac49`.
- [Phase 3] `e0d0d4d86c766` in v5.10-rc1 (tristate); `eee377b8f44e7`
switches to `module_platform_driver`.
- [Phase 3] `git merge-base --is-ancestor e89485455ac49 HEAD` succeeded.
- [Phase 3] Parent file confirmed missing `MODULE_DEVICE_TABLE`.
- [Phase 4] `b4 dig -c/-a/-w` and `b4 dig -m`: v1 thread, three R-b,
Abel applied; To/Cc = clk + i.MX lists.
- [Phase 4] lore HTTP 403; stable-list search **UNVERIFIED**.
- [Phase 5] `Kconfig`: `CLK_IMX8QXP` tristate; Makefile builds `clk-imx-
lpcg-scu`.
- [Phase 5] DTS: `fsl,imx8qxp-lpcg` in imx8qm-ss-*.dtsi.
- [Phase 5] defconfig: `CONFIG_CLK_IMX8QXP=y`.
- [Phase 6] `git describe HEAD` / `make kernelversion`: 7.3.0-rc4; buggy
table present; clean apply.
- [Phase 6] Sibling `cd21fdcbef3fb` is a different file; not required
for this hunk.
- [Phase 8] Failure mode: no autoload when `=m` — verified from code and
MODULE_DEVICE_TABLE semantics, not from a hardware log.
- UNVERIFIED: live `=m` boot on i.MX8QXP; lore.kernel.org/stable
contents.
**YES**
drivers/clk/imx/clk-imx8qxp-lpcg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index 1dae3410ee99a..f0cf6cf918992 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -354,6 +354,7 @@ static const struct of_device_id imx8qxp_lpcg_match[] = {
{ .compatible = "fsl,imx8qxp-lpcg", NULL },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, imx8qxp_lpcg_match);
static struct platform_driver imx8qxp_lpcg_clk_driver = {
.driver = {
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] clk: sunxi-ng: fix ccu probe clock unregister on error
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (6 preceding siblings ...)
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp-lpcg: " Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
7 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Jerome Brunet, Sashiko, Chen-Yu Tsai, Sasha Levin, sboyd,
bmasney+clk, jbrunet+clk, jernej.skrabec, samuel, linux-clk,
linux-arm-kernel, linux-sunxi, linux-kernel
From: Jerome Brunet <jbrunet@baylibre.com>
[ Upstream commit 81f4ddc9f7e0c11d3e9d24d0f30023845a02f7ee ]
When registering clocks with sunxi_ccu_probe(), the number of ccu_clocks
and the number of hw clocks might be different, eventhough they usually are
the same.
If they are different, it could lead to out-of-bound access or registered
clock left behind on error.
Use a different variable when iterating on hw clocks so every registered
clock, and only those, gets unregistered on error.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260629131254.7E34C1F00A3A@smtp.kernel.org
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://patch.msgid.link/20260706-a733-rtc-v4-3-f330728db3d3@baylibre.com
Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: **[clk: sunxi-ng]** **[fix]** unregister the right set of CCU
`clk_hw` objects when `sunxi_ccu_probe()` fails.
**Step 1.2: Tags**
Record:
- `Reported-by: Sashiko <sashiko-bot@kernel.org>` — static-analysis bot,
not syzbot
- `Closes:
https://lore.kernel.org/r/20260629131254.7E34C1F00A3A@smtp.kernel.org`
- `Reviewed-by: Chen-Yu Tsai <wens@kernel.org>` — sunxi/clk maintainer
- `Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>` (author)
- `Link: https://patch.msgid.link/20260706-a733-rtc-v4-3-
f330728db3d3@baylibre.com` — **v4 3/9** of the A733 RTC series
- `Signed-off-by: Chen-Yu Tsai <wens@kernel.org>` (applied to sunxi clk
tree)
- No `Fixes:` tag (expected)
- No `Cc: stable` (expected; not treated as a negative)
**Step 1.3: Body**
Record:
[Bug] `sunxi_ccu_probe()` uses one index for both `desc->ccu_clks`
(`num_ccu_clks`) and `desc->hw_clks` (`hw_clks->num`). Those lengths are
not always equal.
[Symptom] On probe error: **out-of-bounds** `hws[i]` access, or
**registered clocks left behind**.
[Version] None stated.
[Root cause] Error-path unregister walks the leftover loop index instead
of the hw-clock index.
**Step 1.4: Hidden bug fix?**
Record: **No disguise.** This is an explicit error-path OOB/leak fix.
Sashiko rated the pre-existing issue **High**.
---
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record: `drivers/clk/sunxi-ng/ccu_common.c` only; **+6 / −6**. Function:
`sunxi_ccu_probe()`. Surgical, single-file.
**Step 2.2: Code-flow change**
Before: one `i` for (1) `ccu_clks` setup, (2) `hw_clks` register, (3)
rate-range on `ccu_clks`, then `err_clk_unreg` does `while (--i)` over
`hw_clks->hws[i]`. After loop (3), `i == num_ccu_clks`, so
provider/reset failures use the wrong bound.
After: hw-clock register/unregister uses `j`; `i` stays for `ccu_clks`.
**Step 2.3: Bug mechanism**
Record: **[memory safety / error-path leak]**
- `num_ccu_clks > hw_clks->num` → OOB read of `hws[]`
- `num_ccu_clks < hw_clks->num` → incomplete `clk_hw_unregister()`
(leak)
`clk_hw_register()` failure already used the hw index; the clobber is
the **rate-range loop** added later, then `of_clk_add_hw_provider()` /
`reset_controller_register()` failures.
**Step 2.4: Fix quality**
Record: Obviously correct (separate index). Minimal. Regression risk
**very low** (`j` is always set by `for (j = 0; …)` before any `goto
err_clk_unreg`). No API/locking changes.
---
## Phase 3: Git history
**Step 3.1: Blame**
Record: Probe helper from `1d80c14248d608` (2016). Error unregister from
`5d806f9fc8e63d` / `9bec2b9c61340` (v5.16 era). The **clobber** is
`ea977d742507e` (“Don't call hw_to_ccu_common on hw without common”,
**v6.10-rc7**), which inserted a `num_ccu_clks` loop after hw
registration and reused `i`.
**Step 3.2: Fixes: tag**
Record: **None.** Introducer is `ea977d742507e` (ancestor of HEAD). That
commit was itself `Cc: stable` / `Fixes: b914ec33b391e`.
**Step 3.3: File history**
Record: Candidate `81f4ddc9f7e0c`. Nearby Jerome commits are A733 RTC
**features** after this fix. This patch is **standalone**; later series
patches are not prerequisites.
**Step 3.4: Author**
Record: Jerome Brunet (BayLibre, regular clk/sunxi contributor).
Maintainer Chen-Yu Tsai reviewed and applied it.
**Step 3.5: Dependencies**
Record: Needs the three-loop `sunxi_ccu_probe()` from `ea977d742507e`.
That is in this tree. No new helpers. **Can apply standalone.**
---
## Phase 4: Mailing list / external
**Step 4.1: Original discussion**
Record: `b4 dig -c 81f4ddc9f7e0c` → https://patch.msgid.link/20260706-
a733-rtc-v4-3-f330728db3d3@baylibre.com
Series: v1 (Junhui Liu, Jan 2026) → v2/v3/v4 (Jerome). This fix first
appears in **v3** (“Fix ccu probe helper to properly unregister clocks
on error”), applied as **v4 3/9**.
No NAK. Chen-Yu applied it to `sunxi/clk-for-7.3`. No reviewer `Cc:
stable`.
**Step 4.2: Recipients (`b4 dig -w`)**
Record: Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Stephen Boyd,
Maxime Ripard, linux-clk, linux-sunxi. Right maintainers.
**Step 4.3: Bug report**
Record: Sashiko on **v2 5/6** (msgid
`20260629131254.7E34C1F00A3A@smtp.kernel.org`), severity **High**: error
path uses `i == num_ccu_clks` as the `hws[]` bound → leak or OOB
depending on driver sizes. Pre-existing, not introduced by the A733
patch.
**Step 4.4: Series**
Record: 9-patch A733 RTC enablement. This hunk only touches
`ccu_common.c` and does not need A733. Later patches make RTC
`num_ccu_clks` vs `hw_clks->num` diverge more; the common-probe bug
already exists.
**Step 4.5: Stable list**
Record: lore.kernel.org/stable search for this subject returned **no**
dedicated stable-thread discussion. WebFetch to lore.html returned 403;
raw mbox via curl/`b4 dig -m` succeeded.
---
## Phase 5: Semantic / impact
**Step 5.1: Functions**
Record: `sunxi_ccu_probe()` only (plus log index).
**Step 5.2: Callers**
Record: `devm_sunxi_ccu_probe()` and `of_sunxi_ccu_probe()` in the same
file. Called from **all** sunxi-ng CCU drivers (H6, A64, H3, D1, A10,
RTC, …) at SoC clock probe/boot.
**Step 5.3: Callees**
Record: `clk_hw_register` / `of_clk_hw_register`,
`of_clk_add_hw_provider`, `reset_controller_register`,
`clk_hw_unregister`. Provider add can fail (`-ENOMEM`,
`of_clk_set_defaults()`).
**Step 5.4: Reachability**
Record: Boot-time CCU probe on Allwinner. Failure is error-path (OOM,
assigned-clock defaults). On failure, `of_sunxi_ccu_probe()` /
`devm_sunxi_ccu_probe()` **kfree** the `struct sunxi_ccu` whose `lock`
remaining clocks still point at (`cclk->lock = &ccu->lock`) if
unregister is incomplete.
**Step 5.5: Similar patterns**
Record: `devm_sunxi_ccu_release()` already walks `hw_clks->num`
correctly. Only the probe error path was wrong. Counts really differ,
e.g. H6: **127** `ccu_clks` vs **CLK_NUMBER 138**; A64: **104** vs
**115**. RTC in this tree: 12 `ccu_clks` vs `CLK_NUMBER_NO_GATES` **8**
on H616/R329 → **OOB** if probe fails after the rate-range loop.
---
## Phase 6: This checkout (v7.3-rc4)
**Step 6.1: Buggy code here?**
Record: **Yes.** `git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`;
`VERSION=7 PATCHLEVEL=3`. `ea977d742507e` and `81f4ddc9f7e0c` are both
ancestors of HEAD. Pre-fix `sunxi_ccu_probe()` at `81f4ddc9f7e0c^` still
clobbers `i`. Bug since **v6.10**, long before 7.3.
**Step 6.2: Backport difficulty**
Record: **Clean apply** on this code (6-line index split). No refactor
between the buggy helper and this patch.
**Step 6.3: Other fixes already here?**
Record: This **is** the fix (`81f4ddc9f7e0c`). No earlier equivalent.
That does not mean “already selected for stable”; it is the mainline
commit being evaluated.
---
## Phase 7: Subsystem
**Step 7.1:** Record: **clk / Allwinner sunxi-ng CCU**. **IMPORTANT**
(every Allwinner platform’s clock controller), not core mm/net, not
staging.
**Step 7.2:** Record: Mature CCU core (2016) with ongoing SoC additions.
Helper is shared and stable; this is not churny experimental code.
---
## Phase 8: Impact and risk
**Step 8.1:** Record: **platform-specific** — `CONFIG_SUNXI_CCU` /
Allwinner boards (phones, SBCs, TVs). Many LTS users.
**Step 8.2:** Record: Trigger = CCU **probe failure** after clocks are
registered (`of_clk_add_hw_provider` / reset register). Not every boot.
Not a userspace syscall. `of_clk_set_defaults()` is a realistic DT
failure. Unprivileged users do not trigger it directly.
**Step 8.3:** Record: Incomplete unregister → leak; wrong bound →
**OOB**; leftover `clk_hw` plus `kfree(ccu)` → **UAF** on `cclk->lock`.
Severity **HIGH** on the error path (crash / memory safety), not a
normal-boot oops.
**Step 8.4:** Record: **Benefit high** for Allwinner error-path safety;
**risk very low** (6-line index fix). Ratio strongly favors backport.
---
## Phase 9: Synthesis
**Step 9.1: Evidence**
FOR:
- Real OOB/leak in shared CCU probe cleanup; counts differ on H6/A64/RTC
in this tree
- Sashiko High; maintainer Reviewed-by + applied
- Tiny, obviously correct, no new API
- Bug from v6.10 (`ea977d742507e`) is in this tree
- No extra series patches required
AGAINST:
- Error-path only (not every boot)
- Nested in an A733 **feature** series (this patch is still self-
contained)
- No live user oops; found by static review
- No `Fixes:` / `Cc: stable` (not a negative here)
UNRESOLVED:
- lore HTML 403 (mbox/raw used instead)
- Exact production hit rate of `of_clk_set_defaults()` failure on CCU
nodes
**Step 9.2: Stable rules**
1. Obviously correct and tested — **PASS** (maintainer review; logic is
local)
2. Real bug affecting users — **PASS** (Allwinner CCU probe fail →
leak/OOB)
3. Important (crash/corruption/deadlock/security) — **PASS** (OOB;
incomplete unwind + free of lock)
4. Small and contained — **PASS** (12 lines, one function)
5. No new features/APIs — **PASS**
6. Can apply to this tree — **PASS**
**Step 9.3: Exception category**
Record: None (not an ID/quirk/DT/build/doc exception). It is a normal
small bugfix.
**Step 9.4: Decision**
This tree is **7.3-rc4**. The helper and the v6.10 index-clobber exist
here. The patch is a surgical error-path memory-safety fix for every
sunxi-ng CCU, including OOB on RTC when `num_ccu_clks` (12) exceeds
`hw_clks->num` (8). It meets stable rules with very low regression risk.
---
### Problem and why it matters
`sunxi_ccu_probe()` registered `hw_clks` with index `i`, then reused `i`
to walk `ccu_clks` for rate limits. On later failure it unregistered
with that leftover `i`. Those array lengths differ on real SoCs, so
cleanup either walks off `hws[]` or leaves clocks registered (and can
free the `ccu` lock those clocks still use). Stable Allwinner devices
share this helper.
### Stable rules / risk
Small, reviewed, no features, applies cleanly. Benefit is avoiding
OOB/UAF on CCU probe failure; risk is a loop-index change only.
### Concerns
Needs `ea977d742507e` (present here). Do not pull the rest of the A733
series. Trigger is probe-error, not hot-path, but the failure mode is
memory safety.
Verification:
- [Phase 1] Tags: Sashiko Reported-by/Closes, Chen-Yu Reviewed-by, msgid
v4 3/9; no Fixes/Cc stable
- [Phase 2] Diff: `i` vs `j` in `sunxi_ccu_probe()`; +6/−6 in
`ccu_common.c`
- [Phase 3] `git describe HEAD` → `v7.3-rc4-27-gf0100363d8c37`; blame:
bug from `ea977d742507e` (v6.10-rc7); `git merge-base --is-ancestor
ea977d742507e HEAD` → yes
- [Phase 3] `git log --oneline -- drivers/clk/sunxi-ng/ccu_common.c`:
candidate `81f4ddc9f7e0c`; later commits are A733 features, not
prerequisites
- [Phase 4] `b4 dig -c 81f4ddc9f7e0c` → lore msgid; `-a` v1–v4; `-w`
sunxi/clk maintainers; `-m` 25-message mbox; Chen-Yu applied 3/9; no
NAK
- [Phase 4] Original Sashiko report (curl raw): High, wrong loop limit,
leak or OOB
- [Phase 4] Stable lore search: no prior stable-thread hit
- [Phase 5] Callers: `devm_sunxi_ccu_probe` / `of_sunxi_ccu_probe` → all
sunxi-ng CCU drivers
- [Phase 5] H6 127 vs 138; A64 104 vs 115; RTC 12 vs 8
(`CLK_NUMBER_NO_GATES`) verified in tree
- [Phase 5] `of_clk_add_hw_provider` can return `-ENOMEM` or
`of_clk_set_defaults` error
- [Phase 6] Pre-fix source at `81f4ddc9f7e0c^` still uses one `i`;
current file already has `j`
- [Phase 7] MAINTAINERS: Chen-Yu Tsai maintains `drivers/clk/sunxi-ng/`
- [Phase 8] Failure mode: OOB / leak / possible lock UAF on probe error
— HIGH, error-path
- UNVERIFIED: live user crash report beyond Sashiko; lore HTML pages
(403)
**YES**
drivers/clk/sunxi-ng/ccu_common.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
index c7e00f0c29a55..bbda75c2bc461 100644
--- a/drivers/clk/sunxi-ng/ccu_common.c
+++ b/drivers/clk/sunxi-ng/ccu_common.c
@@ -114,7 +114,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
const struct sunxi_ccu_desc *desc)
{
struct ccu_reset *reset;
- int i, ret;
+ int i, j, ret;
ccu->desc = desc;
@@ -130,8 +130,8 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
cclk->lock = &ccu->lock;
}
- for (i = 0; i < desc->hw_clks->num ; i++) {
- struct clk_hw *hw = desc->hw_clks->hws[i];
+ for (j = 0; j < desc->hw_clks->num ; j++) {
+ struct clk_hw *hw = desc->hw_clks->hws[j];
const char *name;
if (!hw)
@@ -143,7 +143,7 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
else
ret = of_clk_hw_register(node, hw);
if (ret) {
- pr_err("Couldn't register clock %d - %s\n", i, name);
+ pr_err("Couldn't register clock %d - %s\n", j, name);
goto err_clk_unreg;
}
}
@@ -186,8 +186,8 @@ static int sunxi_ccu_probe(struct sunxi_ccu *ccu, struct device *dev,
err_del_provider:
of_clk_del_provider(node);
err_clk_unreg:
- while (--i >= 0) {
- struct clk_hw *hw = desc->hw_clks->hws[i];
+ while (--j >= 0) {
+ struct clk_hw *hw = desc->hw_clks->hws[j];
if (!hw)
continue;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH AUTOSEL 6.18-5.10] clk: tegra: support 48MHz clock for pll_p_out1
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] clk: tegra: support 48MHz clock for pll_p_out1 Sasha Levin
@ 2026-10-19 15:02 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-19 15:02 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-clk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: tegra: _calc_rate() always fails with -EINVAL for 48MHz parent_rate
--
commit 5b85244997fd59cc85d760a4c82ed76c458e3f43
Author: Dmitry Osipenko <digetx@gmail.com>
clk: tegra: support 48MHz clock for pll_p_out1
This patch adds 48 MHz parent rate handling in _calc_rate() to avoid hitting
a BUG() when UEFI configures pll_p_out1 to 48 MHz on Surface 2.
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index 591b9f0c155a0..e48ca3f4b5f86 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -575,6 +575,7 @@ static int _calc_rate(struct clk_hw *hw, struct tegra_clk_pll_freq_table *cfg,
> break;
> case 9600000:
> case 28800000:
> + case 48000000:
> /*
> * PLL_P_OUT1 rate is not listed in PLLA table
> */
[Severity: Medium]
When parent_rate is 48000000, this switch arm in _calc_rate() in
drivers/clk/tegra/clk-pll.c computes:
cfreq = parent_rate / (parent_rate / 1000000);
which evaluates to 1000000 (1 MHz). Later in _calc_rate(), cfg->m is
calculated and validated against divm_max(pll):
cfg->m = parent_rate / cfreq;
cfg->n = cfg->output_rate / cfreq;
cfg->cpcon = OUT_OF_TABLE_CPCON;
if (cfg->m == 0 || cfg->m > divm_max(pll) ||
cfg->n > divn_max(pll) || (1 << p_div) > divp_max(pll) ||
cfg->output_rate > pll->params->vco_max) {
return -EINVAL;
}
For parent_rate == 48000000 and cfreq == 1000000, cfg->m is 48. However,
pll_a uses a 5-bit input divider across Tegra114/124 (via pllp_nmp in
drivers/clk/tegra/clk-tegra114.c) and Tegra20/30 (via default_nmp in
drivers/clk/tegra/clk-pll.c):
static struct div_nmp pllp_nmp = {
.divm_shift = 0,
.divm_width = 5,
...
};
Because divm_max(pll) is 31, cfg->m > divm_max(pll) is always true when
cfg->m is 48.
When UEFI sets pll_p_out1 to 48 MHz, won't _calc_rate() unconditionally
return -EINVAL whenever clk_pll_set_rate() or clk_pll_determine_rate() is
called on pll_a, causing tegra114_clock_init() -> tegra_clk_apply_init_table()
-> tegra_init_from_table() in drivers/clk/tegra/clk.c to fail to configure
TEGRA114_CLK_PLL_A and hit WARN_ON(1)?
if (tbl->rate)
if (clk_set_rate(clk, tbl->rate)) {
pr_err("%s: Failed to set rate %lu of %s\n",
__func__, tbl->rate,
__clk_get_name(clk));
WARN_ON(1);
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-333-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-19 15:02 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:45 ` [PATCH AUTOSEL 6.18] clk: clocking-wizard: fix clock difference detection Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vencsys: add missing MODULE_DEVICE_TABLE() Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp: " Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] clk: zynq: handle kasprintf() failure in periph_clk registration Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] clk: tegra: support 48MHz clock for pll_p_out1 Sasha Levin
2026-10-19 15:02 ` sashiko-bot
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] clk: mediatek: mt6735-vdecsys: add missing MODULE_DEVICE_TABLE() Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.10] clk: imx: imx8qxp-lpcg: " Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] clk: sunxi-ng: fix ccu probe clock unregister on error Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox