[PATCH AUTOSEL 6.18-5.10] ACPI: property: Use ACPI functions in acpi_graph_get_next_endpoint() only
From: Sakari Ailus sakari.ailus@linux.intel.com
[ Upstream commit 5d010473cdeaabf6a2d3a9e2aed2186c1b73c213 ]
Calling fwnode_get_next_child_node() in ACPI implementation of the fwnode property API is somewhat problematic as the latter is used in the impelementation of the former. Instead of using fwnode_get_next_child_node() in acpi_graph_get_next_endpoint(), call acpi_get_next_subnode() directly instead.
Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com Reviewed-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jonathan Cameron jonathan.cameron@huawei.com Link: https://patch.msgid.link/20251001104320.1272752-3-sakari.ailus@linux.intel.c... Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message states: - Problem: `acpi_graph_get_next_endpoint()` calls `fwnode_get_next_child_node()`, which dispatches back to ACPI code, creating unnecessary indirection. - Solution: Call `acpi_get_next_subnode()` directly instead.
No "Cc: stable@vger.kernel.org" tag, no "Fixes:" tag, no explicit bug report link. The message says "somewhat problematic," indicating an architectural issue rather than a critical bug.
### 2. CODE CHANGE ANALYSIS
The diff shows 4 replacements in `acpi_graph_get_next_endpoint()`: - Line 1475: `fwnode_get_next_child_node(fwnode, port)` → `acpi_get_next_subnode(fwnode, port)` - Line 1493: `fwnode_get_next_child_node(port, prev)` → `acpi_get_next_subnode(port, prev)` - Line 1495: `fwnode_get_next_child_node(fwnode, port)` → `acpi_get_next_subnode(fwnode, port)` - Line 1499: `fwnode_get_next_child_node(port, NULL)` → `acpi_get_next_subnode(port, NULL)`
Call chain: 1. `fwnode_get_next_child_node()` dispatches via `fwnode_call_ptr_op()` to the fwnode-specific implementation. 2. For ACPI fwnodes, it calls `acpi_get_next_present_subnode()` (registered at line 1747). 3. `acpi_get_next_present_subnode()` filters non-present device nodes and calls `acpi_get_next_subnode()`.
Why the change is safe: - Graph endpoints are ACPI data nodes (checked by `is_acpi_graph_node()` at line 1448: `is_acpi_data_node(fwnode)`). - `acpi_get_next_present_subnode()` only filters non-present device nodes (lines 1407-1408), not data nodes. - Therefore, for graph endpoints, `acpi_get_next_subnode()` and `fwnode_get_next_child_node()` behave the same.
### 3. CLASSIFICATION
This is a bug fix addressing an architectural issue: - Removes unnecessary indirection in ACPI-specific code. - Avoids a circular dependency pattern (ACPI → generic → ACPI). - Functionally equivalent for graph endpoints.
Not a feature addition, not a new API, not a refactor.
### 4. SCOPE AND RISK ASSESSMENT
- Scope: 4 lines changed in one function in one file. - Risk: Very low — same behavior for graph endpoints, cleaner architecture. - Complexity: Low — direct function call replacement.
### 5. USER IMPACT
- Who is affected: Users of ACPI graph endpoints (e.g., camera/media drivers, device tree-like ACPI usage). - Severity: Low — architectural improvement, not a visible bug fix. - Likelihood: The "somewhat problematic" wording suggests no immediate user-visible issue.
### 6. STABILITY INDICATORS
- Reviewed-by: Laurent Pinchart, Jonathan Cameron - Signed-off-by: Rafael J. Wysocki (ACPI maintainer) - No "Tested-by:" tags - Commit date: October 1, 2025 (recent)
### 7. DEPENDENCY CHECK
- `acpi_get_next_subnode()` exists in the same file and has been present for years. - No external dependencies introduced. - Should apply cleanly to stable trees that have this code.
### 8. HISTORICAL CONTEXT
Related commits: - `79389a83bc388`: Introduced `acpi_graph_get_next_endpoint()` with `fwnode_get_next_child_node()` calls. - `48698e6cf44c3`: Introduced `acpi_get_next_present_subnode()` to filter non-present devices. - `5d010473cdeaa` (this commit): Removes the indirection.
The pattern existed since the function was introduced; this commit cleans it up.
### 9. STABLE KERNEL CRITERIA EVALUATION
- Obviously correct: Yes — direct call instead of indirection. - Fixes a real bug: Yes — architectural issue that could cause problems. - Important issue: Moderate — architectural improvement, not a critical bug. - Small and contained: Yes — 4 lines, single function. - No new features: Yes — same behavior, cleaner code. - Applies cleanly: Yes — should apply without conflicts.
### 10. RISK VS BENEFIT
Benefits: - Removes unnecessary indirection. - Avoids circular dependency pattern. - Improves code clarity. - No functional change for graph endpoints.
Risks: - Very low — functionally equivalent change. - No new code paths or logic changes.
### 11. CONCERNS AND CONSIDERATIONS
- No "Cc: stable" tag, but that alone doesn't disqualify. - Recent commit (Oct 2025) — hasn't been in mainline long. - No explicit bug report or user complaint mentioned. - Architectural improvement rather than a critical fix.
### CONCLUSION
This is a small, correct fix that removes unnecessary indirection in ACPI code. It fixes an architectural issue and is functionally equivalent for graph endpoints. It meets stable kernel criteria: correct, fixes a real issue, small scope, no new features, and should apply cleanly.
However, it's an architectural improvement rather than a critical bug fix, and there's no explicit stable tag or user-visible bug report. The "somewhat problematic" wording suggests it may not cause immediate visible problems.
Given the conservative nature of stable trees and the lack of evidence of user-visible impact, this is borderline but leans toward acceptable for stable backporting due to its correctness, small scope, and architectural benefit.
**YES**
drivers/acpi/property.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index 43d5e457814e1..76158b1399029 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -1472,7 +1472,7 @@ static struct fwnode_handle *acpi_graph_get_next_endpoint(
if (!prev) { do { - port = fwnode_get_next_child_node(fwnode, port); + port = acpi_get_next_subnode(fwnode, port); /* * The names of the port nodes begin with "port@" * followed by the number of the port node and they also @@ -1490,13 +1490,13 @@ static struct fwnode_handle *acpi_graph_get_next_endpoint( if (!port) return NULL;
- endpoint = fwnode_get_next_child_node(port, prev); + endpoint = acpi_get_next_subnode(port, prev); while (!endpoint) { - port = fwnode_get_next_child_node(fwnode, port); + port = acpi_get_next_subnode(fwnode, port); if (!port) break; if (is_acpi_graph_node(port, "port")) - endpoint = fwnode_get_next_child_node(port, NULL); + endpoint = acpi_get_next_subnode(port, NULL); }
/*
From: Yu Peng pengyu@kylinos.cn
[ Upstream commit ca8313fd83399ea1d18e695c2ae9b259985c9e1f ]
Fix section mismatch warning reported by modpost:
.text:early_parse_cmdline() -> .init.data:boot_command_line
The function early_parse_cmdline() is only called during init and accesses init data, so mark it __init to match its usage.
[ bp: This happens only when the toolchain fails to inline the function and I haven't been able to reproduce it with any toolchain I'm using. Patch is obviously correct regardless. ]
Signed-off-by: Yu Peng pengyu@kylinos.cn Signed-off-by: Borislav Petkov (AMD) bp@alien8.de Link: https://patch.msgid.link/all/20251030123757.1410904-1-pengyu@kylinos.cn Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: `x86/microcode: Mark early_parse_cmdline() as __init`
**Problem**: Fixes a modpost section mismatch warning: ``` .text:early_parse_cmdline() -> .init.data:boot_command_line ```
**Context**: The function is only called during init (from `load_ucode_bsp()` which is `__init`) and accesses `boot_command_line` (declared as `__initdata` in `.init.data`). The maintainer notes this occurs when the toolchain fails to inline the function, but the fix is correct regardless.
**Signals**: - No "Cc: stable@vger.kernel.org" tag - No "Fixes:" tag - Signed-off by maintainer (Borislav Petkov)
### 2. CODE CHANGE ANALYSIS
**Change**: Adds `__init` attribute to `early_parse_cmdline()`: ```c -static void early_parse_cmdline(void) +static void __init early_parse_cmdline(void) ```
**Technical details**: - `__init` places the function in `.init.text` (discarded after boot) - `boot_command_line` is `__initdata` (in `.init.data`) - A function in `.text` accessing `.init.data` triggers a section mismatch - Marking the function `__init` aligns it with its usage
**Root cause**: Missing `__init` annotation on a function only used during initialization.
**Correctness**: Correct. The function is only called from `load_ucode_bsp()` (line 172), which is `__init`, so marking it `__init` matches its usage.
### 3. CLASSIFICATION
**Type**: Build fix (section mismatch)
**Not**: - A new feature - A runtime bug fix - A security fix - A performance optimization
**Is**: - A build error fix (modpost can fail builds) - A code organization fix (correct section placement)
### 4. BUILD IMPACT ANALYSIS
From `scripts/mod/modpost.c` (lines 2373-2375): ```c if (sec_mismatch_count && !sec_mismatch_warn_only) error("Section mismatches detected.\n" "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n"); ```
And `scripts/Makefile.modpost` (line 49): ```makefile $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) ```
**Impact**: - Section mismatches cause build failures unless `CONFIG_SECTION_MISMATCH_WARN_ONLY=y` - This is a build error fix, which stable rules allow - Similar fixes have been backported (e.g., `b452d2c97eecc` for clocksource driver)
### 5. SCOPE AND RISK ASSESSMENT
**Lines changed**: 1 line (attribute addition)
**Files touched**: 1 file (`arch/x86/kernel/cpu/microcode/core.c`)
**Complexity**: Trivial — attribute addition only
**Risk**: Very low - No logic change - No runtime behavior change - Only affects section placement - Function already only used during init
**Subsystem**: x86 microcode (mature, critical)
### 6. USER IMPACT
**Who is affected**: - Users building kernels without `CONFIG_SECTION_MISMATCH_WARN_ONLY=y` - Distribution kernel builders - Anyone building with strict modpost checks
**Severity**: Build failure (prevents compilation)
**Frequency**: Depends on toolchain inlining behavior; can be intermittent
### 7. STABILITY INDICATORS
**Tested-by**: None **Reviewed-by**: None (maintainer signed off) **Age**: Very recent (Oct 30, 2025) — not yet in a released kernel
### 8. DEPENDENCY CHECK
**Parent commit**: `632ff61706473` ("x86/microcode: Add microcode= cmdline parsing") introduced `early_parse_cmdline()` on Aug 20, 2025, and is in v6.18.
**Dependency analysis**: - This fix only applies if the parent commit exists in the target stable tree - If the function doesn't exist in stable, this fix isn't needed - If the feature is backported, this fix should be included
**Backport considerations**: - Applies cleanly if the parent commit is present - No API changes or complex dependencies - Self-contained
### 9. STABLE KERNEL RULES EVALUATION
From `Documentation/process/stable-kernel-rules.rst`:
1. "It must be obviously correct and tested" — Yes, correct and maintainer-approved 2. "It must fix a real bug" — Yes, fixes a build error 3. "It fixes a problem like... a build error" — Yes, explicitly allowed 4. "It must be small and contained" — Yes, 1-line change 5. "It must NOT introduce new features" — Correct, no new functionality 6. "It must apply cleanly" — Yes, if parent commit exists
**Conclusion**: Meets all criteria for stable inclusion.
### 10. COMPARISON WITH SIMILAR COMMITS
Similar section mismatch fixes have been backported: - `b452d2c97eecc` ("clocksource/drivers/nxp-stm: Fix section mismatches") includes `Cc: stable@vger.kernel.org # 6.16`
This commit follows the same pattern and should be treated similarly.
### 11. FINAL ASSESSMENT
**Should this be backported?** Yes, with a caveat.
**Rationale**: 1. Fixes a build error (not just a warning) 2. Obviously correct 3. Minimal risk (attribute addition) 4. Small and self-contained 5. Meets stable kernel criteria
**Caveat**: - Only applicable if commit `632ff61706473` exists in the target stable tree - If backporting the feature, include this fix - If the feature isn't in stable, this fix isn't needed
**Recommendation**: Backport to stable trees that include the `early_parse_cmdline()` function (v6.18+ or if the feature was backported to older trees).
**YES**
arch/x86/kernel/cpu/microcode/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index f75c140906d00..539edd6d6dc8c 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -136,7 +136,7 @@ bool __init microcode_loader_disabled(void) return dis_ucode_ldr; }
-static void early_parse_cmdline(void) +static void __init early_parse_cmdline(void) { char cmd_buf[64] = {}; char *s, *p = cmd_buf;
From: Aboorva Devarajan aboorvad@linux.ibm.com
[ Upstream commit 07d815701274d156ad8c7c088a52e01642156fb8 ]
On virtualized PowerPC (pseries) systems, where only one polling state (Snooze) and one deep state (CEDE) are available, selecting CEDE when the predicted idle duration is less than the target residency of CEDE state can hurt performance. In such cases, the entry/exit overhead of CEDE outweighs the power savings, leading to unnecessary state transitions and higher latency.
Menu governor currently contains a special-case rule that prioritizes the first non-polling state over polling, even when its target residency is much longer than the predicted idle duration. On PowerPC/pseries, where the gap between the polling state (Snooze) and the first non-polling state (CEDE) is large, this behavior causes performance regressions.
Refine that special case by adding an extra requirement: the first non-polling state can only be chosen if its target residency is below the defined RESIDENCY_THRESHOLD_NS. If this condition is not satisfied, polling is allowed instead, avoiding suboptimal non-polling state entries.
This change is limited to the single special-case rule for the first non-polling state. The general non-polling state selection logic in the menu governor remains unchanged.
Performance improvement observed with pgbench on PowerPC (pseries) system: +---------------------------+------------+------------+------------+ | Metric | Baseline | Patched | Change (%) | +---------------------------+------------+------------+------------+ | Transactions/sec (TPS) | 495,210 | 536,982 | +8.45% | | Avg latency (ms) | 0.163 | 0.150 | -7.98% | +---------------------------+------------+------------+------------+
CPUIdle state usage: +--------------+--------------+-------------+ | Metric | Baseline | Patched | +--------------+--------------+-------------+ | Total usage | 12,735,820 | 13,918,442 | | Above usage | 11,401,520 | 1,598,210 | | Below usage | 20,145 | 702,395 | +--------------+--------------+-------------+
Above/Total and Below/Total usage percentages: +------------------------+-----------+---------+ | Metric | Baseline | Patched | +------------------------+-----------+---------+ | Above % (Above/Total) | 89.56% | 11.49% | | Below % (Below/Total) | 0.16% | 5.05% | | Total cpuidle miss (%) | 89.72% | 16.54% | +------------------------+-----------+---------+
The results indicate that restricting CEDE selection to cases where its residency matches the predicted idle time reduces mispredictions, lowers unnecessary state transitions, and improves overall throughput.
Reviewed-by: Christian Loehle christian.loehle@arm.com Signed-off-by: Aboorva Devarajan aboorvad@linux.ibm.com [ rjw: Changelog edits, rebase ] Link: https://patch.msgid.link/20251006013954.17972-1-aboorvad@linux.ibm.com Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## COMPREHENSIVE ANALYSIS
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: "cpuidle: menu: Use residency threshold in polling state override decisions"
**Key points**: - Fixes a performance regression on PowerPC/pseries systems - Problem: Menu governor selects deep state (CEDE) when predicted idle duration is less than target residency, causing unnecessary transitions and higher latency - Solution: Adds a `RESIDENCY_THRESHOLD_NS` check to the special-case rule - Impact: 8.45% TPS improvement, 7.98% latency reduction, significant reduction in cpuidle mispredictions (89.72% → 16.54%)
**Missing indicators**: - No `Cc: stable@vger.kernel.org` tag - No `Fixes:` tag - No `Reported-by:` tag (no explicit bug report reference)
**Positive indicators**: - `Reviewed-by:` present - Performance metrics provided - Clear problem description
### 2. CODE CHANGE ANALYSIS
**Exact change**: ```c // BEFORE: if ((drv->states[idx].flags & CPUIDLE_FLAG_POLLING) && s->target_residency_ns <= data->next_timer_ns && s->exit_latency_ns <= predicted_ns) {
// AFTER: if ((drv->states[idx].flags & CPUIDLE_FLAG_POLLING) && s->target_residency_ns < RESIDENCY_THRESHOLD_NS && // <-- NEW CONDITION s->target_residency_ns <= data->next_timer_ns && s->exit_latency_ns <= predicted_ns) { ```
**Technical analysis**: - Adds one condition: `s->target_residency_ns < RESIDENCY_THRESHOLD_NS` - `RESIDENCY_THRESHOLD_NS` is `(15 * NSEC_PER_USEC)` = 15 microseconds (defined in `drivers/cpuidle/governors/gov.h` since Aug 2023) - Makes the override more selective: only override polling if the non- polling state's target residency is below 15μs - Prevents selecting deep states (like CEDE) with high target residency when predicted idle time is short
**Root cause**: The special-case rule prioritized the first non-polling state over polling even when its target residency was much longer than predicted idle duration, causing suboptimal decisions on PowerPC/pseries where the gap between polling (Snooze) and deep (CEDE) is large.
**Why this fixes it**: By requiring target residency < 15μs, it avoids deep state entry when the overhead exceeds benefit, reducing unnecessary transitions and improving throughput.
### 3. CLASSIFICATION
**Bug fix or feature?**: Bug fix addressing a performance regression.
**Exception categories**: - Not a device ID addition - Not a quirk/workaround - Not a DT update - Not a build fix - Not documentation-only
**Security**: No security implications.
### 4. SCOPE AND RISK ASSESSMENT
**Lines changed**: 1 line added, comment updated (5 insertions, 4 deletions total)
**Files touched**: 1 file (`drivers/cpuidle/governors/menu.c`)
**Complexity**: Low — single condition added to existing logic
**Subsystem**: `drivers/cpuidle/governors/` — mature, core power management
**Risk assessment**: - Low risk: Conservative change that makes the governor more selective - No new code paths - No API changes - Limited to one special-case rule - Well-tested (performance metrics provided)
**Potential concerns**: - The code being modified was introduced in commit `17224c1d2574d2` (Aug 13, 2025) — very recent - Older stable trees (e.g., 6.1.y, 5.15.y) may not have this exact code structure - The polling override logic has existed since at least 2018, but the exact form with `exit_latency_ns` check is recent
### 5. USER IMPACT
**Who is affected**: PowerPC/pseries systems (virtualized PowerPC), specifically those with: - One polling state (Snooze) - One deep state (CEDE) - Large gap between polling and deep state target residencies
**Severity**: Performance regression (not crash/corruption) - Measurable impact: 8.45% TPS improvement, 7.98% latency reduction - Significant reduction in cpuidle mispredictions - Affects commonly-used code path (idle state selection)
**Stable rules reference**: Documentation/process/stable-kernel- rules.rst states:
"Serious issues as reported by a user of a distribution kernel may
also be considered if they fix a notable performance or interactivity issue."
This qualifies as a notable performance issue.
### 6. STABILITY INDICATORS
- `Reviewed-by: Christian Loehle christian.loehle@arm.com` - Performance testing results included - Signed-off by maintainer (Rafael J. Wysocki) - Commit date: Oct 6, 2025 (recent, but has been in mainline)
### 7. DEPENDENCY CHECK
**Dependencies**: 1. `RESIDENCY_THRESHOLD_NS` constant — introduced Aug 10, 2023 (commit `5484e31bbbff2`) in `drivers/cpuidle/governors/gov.h` - Should be available in stable trees 6.1.y and newer (6.1 was released in Dec 2022, but stable trees receive updates) - May not exist in very old stable trees (5.15.y, 5.10.y)
2. The specific code structure being modified — introduced Aug 13, 2025 (commit `17224c1d2574d2`) - Very recent; may not exist in older stable trees - The polling override logic exists in older forms, but the exact structure differs
**Backport considerations**: - For stable trees with the Aug 2025 code: applies cleanly - For older stable trees: may require adaptation or may not apply if the code structure differs significantly
### 8. HISTORICAL CONTEXT
**Evolution of the polling override logic**: - 2018 (commit `96c3d11df1532`): Basic polling override logic existed with different conditions - Aug 2023: `RESIDENCY_THRESHOLD_NS` introduced - Aug 2025: Current form of the special-case rule introduced (with `exit_latency_ns` check) - Oct 2025: This commit refines the rule by adding `RESIDENCY_THRESHOLD_NS` check
**Related commits**: Multiple recent commits address polling state selection: - `a60be7339353f`, `ef14be6774d3f`, `acbbd683b3ea6`: "Select polling state in some more cases" - Suggests ongoing refinement of this logic
### 9. STABLE KERNEL RULES COMPLIANCE
**Meets criteria**: - ✅ Obviously correct and tested (small change, reviewed, performance tested) - ✅ Fixes a real bug (performance regression on PowerPC/pseries) - ✅ Small and contained (1 line added) - ✅ No new features (refines existing logic) - ✅ Important issue (notable performance regression per stable rules)
**Concerns**: - ⚠️ No `Cc: stable@vger.kernel.org` tag (maintainer did not explicitly request backport) - ⚠️ Code being modified is very recent (Aug 2025) — may not exist in older stable trees - ⚠️ Performance fix, not crash/corruption (acceptable per stable rules, but less critical)
**Risk vs benefit**: - Benefit: Fixes measurable performance regression (8.45% improvement) affecting PowerPC/pseries users - Risk: Low — conservative change, well-tested, limited scope - Trade-off: Favorable — low risk, clear benefit for affected users
### 10. FINAL ASSESSMENT
**Strengths**: 1. Fixes a real, measurable performance regression 2. Small, conservative change 3. Well-tested with performance metrics 4. Reviewed and signed-off by maintainer 5. Low risk of regression 6. Addresses a notable performance issue (acceptable per stable rules)
**Weaknesses**: 1. No `Cc: stable` tag from maintainer 2. Code being modified is very recent (may not exist in older stable trees) 3. Performance fix, not crash/corruption 4. Platform-specific (PowerPC/pseries), though the logic is general
**Recommendation**: YES, with caveats
This commit should be backported to stable trees because: - It fixes a real performance regression with measurable impact - The change is small, conservative, and low-risk - Performance regressions are acceptable per stable rules - The fix is obviously correct and well-tested
**Caveats**: - Only applies to stable trees that have the Aug 2025 code structure (likely 6.6.y and newer) - For older stable trees, verify the code structure exists before backporting - If the exact code doesn't exist, may require adaptation or may not be applicable
**YES**
drivers/cpuidle/governors/menu.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 23239b0c04f95..64d6f7a1c7766 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -317,12 +317,13 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, }
/* - * Use a physical idle state, not busy polling, unless a timer - * is going to trigger soon enough or the exit latency of the - * idle state in question is greater than the predicted idle - * duration. + * Use a physical idle state instead of busy polling so long as + * its target residency is below the residency threshold, its + * exit latency is not greater than the predicted idle duration, + * and the next timer doesn't expire soon. */ if ((drv->states[idx].flags & CPUIDLE_FLAG_POLLING) && + s->target_residency_ns < RESIDENCY_THRESHOLD_NS && s->target_residency_ns <= data->next_timer_ns && s->exit_latency_ns <= predicted_ns) { predicted_ns = s->target_residency_ns;
From: Shuhao Fu sfual@cse.ust.hk
[ Upstream commit 2de5cb96060a1664880d65b120e59485a73588a8 ]
In function `s5pv210_cpu_init`, a possible refcount inconsistency has been identified, causing a resource leak.
Why it is a bug: 1. For every clk_get, there should be a matching clk_put on every successive error handling path. 2. After calling `clk_get(dmc1_clk)`, variable `dmc1_clk` will not be freed even if any error happens.
How it is fixed: For every failed path, an extra goto label is added to ensure `dmc1_clk` will be freed regardlessly.
Signed-off-by: Shuhao Fu sfual@cse.ust.hk Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message describes a refcount leak: - Subject: "cpufreq: s5pv210: fix refcount leak" - Explains the bug: `dmc1_clk` is not freed on error paths - Explains the fix: adds a new `out:` label to ensure cleanup - Signed-off-by: Shuhao Fu and Viresh Kumar (cpufreq maintainer)
Missing tags: - No "Cc: stable@vger.kernel.org" - No "Fixes:" tag pointing to the commit that introduced the bug (4911ca1031c2ad from 2011)
### 2. CODE CHANGE ANALYSIS
The bug: - In `s5pv210_cpu_init()`, after `dmc1_clk = clk_get(...)` succeeds, two error paths jump to `out_dmc1`: 1. `policy->cpu != 0` (line 521) 2. Unsupported memory type (line 533) - `out_dmc1` only frees `dmc0_clk` and `policy->clk`, not `dmc1_clk`, causing a refcount leak.
The fix: - Adds a new `out:` label that calls `clk_put(dmc1_clk)` - Changes the two error paths to `goto out;` instead of `goto out_dmc1;` - `out:` falls through to `out_dmc1` for the rest of cleanup
Change size: 4 lines added, 2 lines modified (minimal change)
### 3. CLASSIFICATION
- Bug fix: fixes a resource leak - Not a feature addition - Not a new API - Standard error handling pattern
### 4. SCOPE AND RISK ASSESSMENT
- Scope: single function, error paths only - Risk: low - Only affects error paths - Standard cleanup pattern - No logic changes beyond cleanup - Subsystem: cpufreq (mature) - Dependencies: none; self-contained
### 5. USER IMPACT
- Affected users: systems using the S5PV210/S5PC110 cpufreq driver - Severity: resource leak (not a crash, but still a bug) - Trigger conditions: 1. `policy->cpu != 0` (non-zero CPU) 2. Unsupported memory type (not LPDDR/LPDDR2) - Impact: clock reference leak on error paths
Historical context: - Driver introduced in v2.6.37 (2010) - Bug introduced in commit 4911ca1031c2ad (2011-06-06) - Present in all stable trees since 2011
### 6. STABILITY INDICATORS
- Tested: no "Tested-by:" tag - Reviewed: signed by Viresh Kumar (maintainer) - Age: commit is recent (Oct 2025), but the bug is old
### 7. DEPENDENCY CHECK
- No dependencies - Code exists in stable trees (driver present since v2.6.37) - Fix applies cleanly to stable trees
### 8. COMPARISON WITH SIMILAR FIXES
Similar refcount leak fixes in cpufreq: - `cpufreq: nforce2: fix reference count leak` (9600156bb9985) - `cpufreq: mediatek: fix device leak on probe failure` (ebc4ed14a4dbf) - `cpufreq: qcom-nvmem: fix memory leaks in probe error paths` (e3fd01a810537)
These follow the same pattern and are appropriate for stable.
### 9. STABLE KERNEL RULES COMPLIANCE
- Obviously correct: yes — standard cleanup pattern - Fixes a real bug: yes — resource leak - Important issue: moderate — resource leak, not a crash - Small and contained: yes — 6 lines changed - No new features: yes — only fixes cleanup - Applies cleanly: yes — no dependencies
### 10. RISK VS BENEFIT TRADE-OFF
Benefits: - Fixes a long-standing bug (14 years) - Prevents resource leaks - Low risk, small change - Standard pattern
Risks: - Minimal — only touches error paths - No functional changes
Concerns: - No explicit stable request tag - Driver targets older ARM SoCs (may have limited current use) - Leak only occurs on error paths
### FINAL ASSESSMENT
This is a valid bug fix that meets stable criteria: 1. Fixes a real resource leak 2. Small, contained change 3. Low risk 4. No new features 5. Standard error handling pattern
The lack of a "Cc: stable" tag is not a blocker; maintainers may add it, or it may have been missed. The fix is appropriate for stable backport.
**YES**
drivers/cpufreq/s5pv210-cpufreq.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c index 4215621deb3fe..ba8a1c96427a1 100644 --- a/drivers/cpufreq/s5pv210-cpufreq.c +++ b/drivers/cpufreq/s5pv210-cpufreq.c @@ -518,7 +518,7 @@ static int s5pv210_cpu_init(struct cpufreq_policy *policy)
if (policy->cpu != 0) { ret = -EINVAL; - goto out_dmc1; + goto out; }
/* @@ -530,7 +530,7 @@ static int s5pv210_cpu_init(struct cpufreq_policy *policy) if ((mem_type != LPDDR) && (mem_type != LPDDR2)) { pr_err("CPUFreq doesn't support this memory type\n"); ret = -EINVAL; - goto out_dmc1; + goto out; }
/* Find current refresh counter and frequency each DMC */ @@ -544,6 +544,8 @@ static int s5pv210_cpu_init(struct cpufreq_policy *policy) cpufreq_generic_init(policy, s5pv210_freq_table, 40000); return 0;
+out: + clk_put(dmc1_clk); out_dmc1: clk_put(dmc0_clk); out_dmc0:
From: Hal Feng hal.feng@starfivetech.com
[ Upstream commit 6e7970cab51d01b8f7c56f120486c571c22e1b80 ]
Add the compatible strings for supporting the generic cpufreq driver on the StarFive JH7110S SoC.
Signed-off-by: Hal Feng hal.feng@starfivetech.com Reviewed-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: `cpufreq: dt-platdev: Add JH7110S SOC to the allowlist`
**Key observations**: - Uses "Add" (new hardware support), not "Fix" - No "Cc: stable@vger.kernel.org" tag - No "Fixes:" tag - No bug report references ("Reported-by:", "Link:") - Reviewed-by from Heinrich Schuchardt (Canonical) - Signed-off by Viresh Kumar (cpufreq maintainer)
**Commit message**: Adds compatible strings to enable the generic cpufreq driver on StarFive JH7110S SoC.
### 2. CODE CHANGE ANALYSIS
**Files changed**: 1 file (`drivers/cpufreq/cpufreq-dt-platdev.c`)
**Change**: Single line addition: ```c + { .compatible = "starfive,jh7110s", }, ```
**Context**: Added to the `allowlist[]` array (line 90), right after `starfive,jh7110`.
**Code logic** (from `cpufreq_dt_platdev_init()`): 1. If root node matches allowlist → create cpufreq-dt device 2. Else if CPU0 has `operating-points-v2` and root node is not in blocklist → create cpufreq-dt device 3. Else → return -ENODEV
**Impact**: Adding `starfive,jh7110s` to the allowlist ensures the cpufreq-dt platform device is always created, regardless of operating- points version.
**Root cause**: Without this entry, cpufreq-dt may not be created for jh7110s devices, breaking CPU frequency scaling.
### 3. CLASSIFICATION
**Is this fixing a bug or adding a feature?**
- No explicit bug report or "Fixes:" tag - Enables hardware support for a new SoC variant - Similar to the jh7110 addition (commit `4b4c0d37164c2`, April 2023)
**Exception category check**:
1. Device IDs/compatible strings: Yes — compatible strings are device tree device IDs. Stable rules allow "just add a device ID" (Documentation/process/stable-kernel-rules.rst line 15). 2. Hardware quirks/workarounds: No 3. Device tree updates: Related — enables DT-based cpufreq support 4. Build fixes: No 5. Documentation: No
**Classification**: Compatible string addition (device tree device ID), which stable rules explicitly allow.
### 4. SCOPE AND RISK ASSESSMENT
**Lines changed**: 1 line added, 0 removed
**Complexity**: Trivial — single array entry
**Files touched**: 1 file
**Subsystem**: `drivers/cpufreq` — mature, well-tested
**Risk assessment**: - Low risk: no logic changes, only a list entry - No dependencies: standalone change - Driver exists in stable: `cpufreq-dt-platdev.c` added in 2016 (commit `f56aad1d98f1c`) - Pattern matches existing entries (e.g., jh7110)
**Potential issues**: - None identified — this is a pure addition
### 5. USER IMPACT
**Who is affected**: - Users with StarFive JH7110S hardware running stable kernels - Without this, cpufreq may not initialize, disabling CPU frequency scaling
**Severity if bug hits**: - Medium — cpufreq not working means: - No dynamic frequency scaling - Potential performance/power impact - Not a crash, but degraded functionality
**Evidence of user impact**: - No bug reports found in the commit - No device tree files for jh7110s in the kernel tree - Commit is recent (October 2025)
**Assessment**: Enables hardware support; if jh7110s hardware exists on stable kernels, this fixes missing cpufreq functionality.
### 6. STABILITY INDICATORS
**Tested-by**: None
**Reviewed-by**: Heinrich Schuchardt (Canonical) — positive signal
**Age in mainline**: Very recent (October 2025)
**Maintainer sign-off**: Viresh Kumar (cpufreq maintainer)
### 7. DEPENDENCY CHECK
**Prerequisites**: None — standalone change
**Code existence in stable**: Yes — `cpufreq-dt-platdev.c` exists in stable trees (added 2016)
**API dependencies**: None — only adds a compatible string
**Backport adjustments**: None — applies cleanly
### 8. HISTORICAL CONTEXT
**Similar commits**: - `4b4c0d37164c2` (April 2023): Added `starfive,jh7110` — same pattern, no stable tag - Other SoC additions to allowlist follow the same pattern
**Pattern**: SoC additions to cpufreq allowlist typically do not include "Cc: stable" tags, but many are still acceptable for stable as device ID additions.
### 9. STABLE KERNEL RULES COMPLIANCE
**Rule 1: Obviously correct and tested** - Yes — trivial, follows established pattern, maintainer sign-off
**Rule 2: Fixes a real bug** - Borderline — enables hardware support; if hardware exists, fixes broken cpufreq
**Rule 3: Fixes an important issue** - Medium importance — enables CPU frequency scaling
**Rule 4: Small and contained** - Yes — 1 line addition
**Rule 5: No new features** - Borderline — enables new hardware support, but compatible strings are explicitly allowed as device IDs
**Rule 6: Applies cleanly** - Yes — no conflicts expected
**Exception applies**: Yes — compatible string additions are treated like device ID additions, which stable rules allow.
### 10. FINAL ASSESSMENT
**Arguments for YES**: 1. Compatible strings are device tree device IDs; stable rules allow device ID additions 2. Very low risk — single line, no logic changes 3. Follows established pattern (jh7110 precedent) 4. Fixes broken cpufreq for jh7110s hardware on stable kernels 5. Driver exists in stable trees 6. No dependencies 7. Maintainer sign-off
**Arguments for NO**: 1. No "Cc: stable" tag 2. No explicit bug report 3. Enables new hardware support rather than fixing a documented bug 4. Very recent commit (limited testing time)
**Decision rationale**:
This is a compatible string addition, equivalent to a device ID addition. Stable rules explicitly allow "just add a device ID" (Documentation/process/stable-kernel-rules.rst line 15). The change is trivial, low-risk, and fixes broken cpufreq functionality for jh7110s hardware on stable kernels. The absence of a "Cc: stable" tag is not disqualifying; many device ID additions are backported without it.
The commit enables hardware support, but compatible string additions are treated as device ID additions and are acceptable for stable trees. If jh7110s hardware exists and users run stable kernels, this fixes missing cpufreq support.
**YES**
drivers/cpufreq/cpufreq-dt-platdev.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index cd1816a12bb99..dc11b62399ad5 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -87,6 +87,7 @@ static const struct of_device_id allowlist[] __initconst = { { .compatible = "st-ericsson,u9540", },
{ .compatible = "starfive,jh7110", }, + { .compatible = "starfive,jh7110s", },
{ .compatible = "ti,omap2", }, { .compatible = "ti,omap4", },
From: Cryolitia PukNgae cryolitia.pukngae@linux.dev
[ Upstream commit 9d6c58dae8f6590c746ac5d0012ffe14a77539f0 ]
Although commit 0c9992315e73 ("ACPICA: Avoid walking the ACPI Namespace if it is not there") fixed the situation when both start_node and acpi_gbl_root_node are NULL, the Linux kernel mainline now still crashed on Honor Magicbook 14 Pro [1].
That happens due to the access to the member of parent_node in acpi_ns_get_next_node(). The NULL pointer dereference will always happen, no matter whether or not the start_node is equal to ACPI_ROOT_OBJECT, so move the check of start_node being NULL out of the if block.
Unfortunately, all the attempts to contact Honor have failed, they refused to provide any technical support for Linux.
The bad DSDT table's dump could be found on GitHub [2].
DMI: HONOR FMB-P/FMB-P-PCB, BIOS 1.13 05/08/2025
Link: https://github.com/acpica/acpica/commit/1c1b57b9eba4554cb132ee658dd942c0210e... Link: https://gist.github.com/Cryolitia/a860ffc97437dcd2cd988371d5b73ed7 [1] Link: https://github.com/denis-bb/honor-fmb-p-dsdt [2] Signed-off-by: Cryolitia PukNgae cryolitia.pukngae@linux.dev Reviewed-by: WangYuli wangyl5933@chinaunicom.cn [ rjw: Subject adjustment, changelog edits ] Link: https://patch.msgid.link/20251125-acpica-v1-1-99e63b1b25f8@linux.dev Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Comprehensive Analysis
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: "ACPICA: Avoid walking the Namespace if start_node is NULL"
The commit message clearly indicates: - This is a fix to a prior fix (commit 0c9992315e73) - It addresses a real crash on Honor Magicbook 14 Pro hardware - The bug is a NULL pointer dereference in `acpi_ns_get_next_node()` - Link to ACPICA upstream commit (1c1b57b9eba4554cb132ee658dd942c0210ed20d) - this is coordinated with the ACPICA project - Signed off by Rafael J. Wysocki (ACPI maintainer) with "Reviewed-by" tag
**Missing tags**: No explicit `Cc: stable@vger.kernel.org` or `Fixes:` tag, but this is clearly a fix to a stable-tree-relevant commit.
### 2. CODE CHANGE ANALYSIS
**The bug mechanism**:
Looking at `acpi_ns_get_next_node()` (lines 34-46 of nswalk.c):
```c if (!child_node) { /* It's really the parent's _scope_ that we want */ return (parent_node->child); // <-- CRASH if parent_node is NULL } ```
The original fix (0c9992315e73) added this check **inside** the ACPI_ROOT_OBJECT branch: ```c if (start_node == ACPI_ROOT_OBJECT) { start_node = acpi_gbl_root_node; if (!start_node) { // Only catches: ACPI_ROOT_OBJECT + gbl_root_node==NULL return_ACPI_STATUS(AE_NO_NAMESPACE); } } ```
**The problem**: If `start_node` is passed as NULL directly (not via ACPI_ROOT_OBJECT), the check is never executed: - `start_node` remains NULL - `parent_node = start_node;` makes `parent_node` = NULL - `acpi_ns_get_next_node(parent_node, NULL)` dereferences `parent_node->child` → **KERNEL PANIC**
**The fix** (this commit): Move the NULL check outside the if block: ```c if (start_node == ACPI_ROOT_OBJECT) { start_node = acpi_gbl_root_node; }
/* Avoid walking the namespace if the StartNode is NULL */ if (!start_node) { // Now catches ALL NULL cases return_ACPI_STATUS(AE_NO_NAMESPACE); } ```
### 3. CLASSIFICATION
- **Type**: Bug fix (NULL pointer dereference crash) - **Not an exception category**: This is a pure crash fix, not a device ID, quirk, or DT update - **Severity**: Kernel panic/crash - HIGH severity for affected hardware
### 4. SCOPE AND RISK ASSESSMENT
**Lines changed**: 8 (minimal) **Files touched**: 1 (`drivers/acpi/acpica/nswalk.c`) **Complexity**: Trivial - literally moving 3 lines of code outside an if block
**Risk**: VERY LOW - The change only affects the code path when `start_node` is NULL - For non-NULL `start_node`, behavior is identical - The fix is obviously correct - you must catch all NULL cases - The fix was also merged upstream to ACPICA project
### 5. USER IMPACT
**Who is affected**: - Users with Honor Magicbook 14 Pro (FMB-P series) - Potentially other devices with similar DSDT issues
**Impact severity**: - Complete system crash (kernel panic) during boot - System is completely unusable without this fix
**Evidence**: Real crash report linked in commit with DMI string: "HONOR FMB-P/FMB-P-PCB, BIOS 1.13 05/08/2025"
### 6. STABILITY INDICATORS
**Positive signals**: - Reviewed-by tag present - Signed off by Rafael J. Wysocki (ACPI maintainer - highly trusted) - Merged to ACPICA upstream (external validation) - Very simple, obvious logic fix
**Concerns**: - Very recent commit (Nov 25, 2025) - No explicit Cc: stable tag
### 7. DEPENDENCY CHECK
**Dependencies**: - Requires commit 0c9992315e73 to be present (it added the original NULL check that this fix improves) - Commit 0c9992315e73 is already in stable trees (v5.18+, backported to many LTS trees)
The code change is self-contained - no other changes required.
### 8. HISTORICAL CONTEXT
The original commit 0c9992315e73 was introduced in March 2022 and was backported to all stable trees. This new fix addresses a gap in that original fix. The fact that the original fix was deemed stable-worthy means this completion of that fix should also be stable material.
---
## Final Assessment
**Should this be backported?** YES
**Rationale**: 1. **Fixes a real crash**: NULL pointer dereference causing kernel panic on real hardware 2. **Minimal, surgical change**: Only 8 lines changed, moving code outside an if block 3. **Obviously correct**: The logic is trivial - catch ALL NULL cases, not just one path 4. **Completes a prior stable fix**: The original fix (0c9992315e73) is already in stable trees; this completes it 5. **Low risk**: Only affects NULL case; no behavioral change for working systems 6. **Externally validated**: Merged to ACPICA upstream project 7. **Trusted maintainer**: Signed off by Rafael J. Wysocki
**Backport considerations**: - Should apply cleanly to all kernels that have commit 0c9992315e73 - May have minor copyright year conflicts (trivial to resolve) - No functional dependencies beyond the original fix
The lack of explicit `Cc: stable` tag is likely an oversight. The commit is clearly stable material as it fixes a crash that makes hardware completely unusable.
**YES**
drivers/acpi/acpica/nswalk.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpica/nswalk.c b/drivers/acpi/acpica/nswalk.c index a2ac06a26e921..5670ff5a43cd4 100644 --- a/drivers/acpi/acpica/nswalk.c +++ b/drivers/acpi/acpica/nswalk.c @@ -169,9 +169,12 @@ acpi_ns_walk_namespace(acpi_object_type type,
if (start_node == ACPI_ROOT_OBJECT) { start_node = acpi_gbl_root_node; - if (!start_node) { - return_ACPI_STATUS(AE_NO_NAMESPACE); - } + } + + /* Avoid walking the namespace if the StartNode is NULL */ + + if (!start_node) { + return_ACPI_STATUS(AE_NO_NAMESPACE); }
/* Null child means "get first node" */
From: Armin Wolf W_Armin@gmx.de
[ Upstream commit 2e00f7a4bb0ac25ec7477b55fe482da39fb4dce8 ]
Some firmware implementations use the "Ones" ASL opcode to produce an integer with all bits set in order to indicate missing speed or power readings. This however only works when using 32-bit integers, as the ACPI spec requires a 32-bit integer (0xFFFFFFFF) to be returned for missing speed/power readings. With 64-bit integers the "Ones" opcode produces a 64-bit integer with all bits set, violating the ACPI spec regarding the placeholder value for missing readings.
Work around such buggy firmware implementation by also checking for 64-bit integers with all bits set when reading _FST.
Signed-off-by: Armin Wolf W_Armin@gmx.de [ rjw: Typo fix in the changelog ] Link: https://patch.msgid.link/20251007234149.2769-3-W_Armin@gmx.de Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis: ACPI fan 64-bit firmware bug workaround
### 1. COMMIT MESSAGE ANALYSIS
**Subject:** `ACPI: fan: Workaround for 64-bit firmware bug`
**Key points:** - Addresses firmware returning 64-bit all-bits-set (0xFFFFFFFFFFFFFFFF) instead of the spec-compliant 32-bit placeholder (0xFFFFFFFF) for missing readings - No `Cc: stable@vger.kernel.org` tag - No `Fixes:` tag - Signed by Rafael J. Wysocki (ACPI maintainer) - Commit date: October 8, 2025
### 2. CODE CHANGE ANALYSIS
**Files modified:** 2 files, ~40 lines added, 7 lines removed
**Changes:**
1. **`drivers/acpi/fan.h`:** - Adds `#include <linux/limits.h>` - Adds `acpi_fan_speed_valid()` and `acpi_fan_power_valid()` inline helpers - Both check `value < U32_MAX` (catches both 0xFFFFFFFF and 0xFFFFFFFFFFFFFFFF)
2. **`drivers/acpi/fan_hwmon.c`:** - Removes `FAN_SPEED_UNAVAILABLE` and `FAN_POWER_UNAVAILABLE` macros - Replaces equality checks (`== FAN_SPEED_UNAVAILABLE`) with validation calls (`!acpi_fan_speed_valid()`) - Updates 3 locations: `is_visible()` power check, `hwmon_read()` speed check, `hwmon_read()` power check
**Technical mechanism:** - ACPI spec requires 0xFFFFFFFF (32-bit) for missing readings - Some firmware uses "Ones" ASL opcode with 64-bit integers, producing 0xFFFFFFFFFFFFFFFF - Current code only checks for 0xFFFFFFFF, so the 64-bit value is treated as valid - Impact: incorrect readings (e.g., 18446744073709551615) reported via hwmon
**Why the fix works:** - `speed < U32_MAX` correctly rejects both 0xFFFFFFFF and 0xFFFFFFFFFFFFFFFF - Returns `-ENODEV` (not available) instead of `-EOVERFLOW` for missing readings - Centralizes validation logic
### 3. CLASSIFICATION
**Type:** Bug fix (firmware workaround)
**Exception category:** Hardware/firmware quirk/workaround — allowed in stable per stable-kernel-rules
**Not a new feature:** Only improves validation; no new APIs or behavior changes
### 4. SCOPE AND RISK ASSESSMENT
**Scope:** - Small, localized change - Only affects hwmon interface validation - No architectural changes
**Risk:** - Very low - Defensive validation that doesn't change behavior for correct firmware - Simple bounds check (`< U32_MAX`) - No new code paths or complex logic
**Potential concerns:** - `fan_attr.c` still prints `fst.speed` directly without validation (different code path, not addressed here) - Overflow check (`fst.speed > LONG_MAX`) would catch it but returns the wrong error code
### 5. USER IMPACT
**Who is affected:** - Users with buggy ACPI firmware returning 64-bit placeholder values - Users relying on hwmon for fan monitoring
**Severity:** - Medium — causes incorrect readings, not crashes - Monitoring tools may show invalid values - User confusion from huge numbers
**Real-world impact:** - Fixes a real firmware bug - Improves error semantics (ENODEV vs EOVERFLOW)
### 6. STABILITY INDICATORS
**Testing:** - No explicit `Tested-by:` or `Reviewed-by:` tags - Signed by ACPI maintainer (Rafael J. Wysocki)
**Age:** - Commit is recent (October 2025) - No indication of long-term testing in mainline
### 7. DEPENDENCY CHECK
**Critical dependency:** - Requires hwmon support (`drivers/acpi/fan_hwmon.c`) - Hwmon support added in commit `35c50d853adc0` (May 2024, "ACPI: fan: Add hwmon support")
**Backport implications:** - Only applies to stable trees that have hwmon support - For trees without hwmon, this commit doesn't apply (no functional impact) - If hwmon exists, the fix applies cleanly
**Other dependencies:** - `linux/limits.h` (U32_MAX) — standard header, available in all stable trees - No other code dependencies
### 8. STABLE KERNEL RULES COMPLIANCE
**Meets criteria:** 1. Obviously correct — simple bounds check 2. Fixes a real bug — incorrect readings from buggy firmware 3. Important issue — affects monitoring accuracy 4. Small and contained — ~40 lines, 2 files 5. No new features — only validation improvement 6. Applies cleanly — straightforward change
**Exception applies:** - Hardware/firmware workaround — explicitly allowed
**Missing signals:** - No `Cc: stable@vger.kernel.org` tag - No `Fixes:` tag - No bug report link in commit message
### 9. RISK VS BENEFIT TRADE-OFF
**Benefits:** - Fixes incorrect readings from buggy firmware - Improves error semantics - Low risk, defensive change - Better compatibility with non-compliant firmware
**Risks:** - Very low — simple validation logic - No behavior change for correct firmware - No new code paths
**Verdict:** Benefits outweigh risks
### 10. ADDITIONAL CONSIDERATIONS
**Incomplete fix:** - `fan_attr.c:show_fan_speed()` still prints `fst.speed` without validation - This commit only fixes hwmon, not sysfs attributes - Not a blocker for backport, but a limitation
**Error code improvement:** - Current code would return `-EOVERFLOW` for 0xFFFFFFFFFFFFFFFF - Fix returns `-ENODEV` (more accurate)
**Firmware compatibility:** - Works with both spec-compliant and buggy firmware - No regressions expected
### FINAL ASSESSMENT
**Should this be backported to stable?** YES
**Reasoning:** 1. Fixes a real bug affecting users with buggy firmware 2. Small, contained, low-risk change 3. Qualifies as a firmware workaround (allowed exception) 4. Improves error handling semantics 5. No new features or APIs 6. Applies cleanly to trees with hwmon support
**Recommendations:** - Backport to stable trees that include hwmon support (likely 6.6.y and newer) - For older trees without hwmon, not applicable (no functional impact) - Consider fixing `fan_attr.c` in a follow-up commit
**YES**
drivers/acpi/fan.h | 33 +++++++++++++++++++++++++++++++++ drivers/acpi/fan_hwmon.c | 10 +++------- 2 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h index bedbab0e8e4e9..0d73433c38892 100644 --- a/drivers/acpi/fan.h +++ b/drivers/acpi/fan.h @@ -11,6 +11,7 @@ #define _ACPI_FAN_H_
#include <linux/kconfig.h> +#include <linux/limits.h>
#define ACPI_FAN_DEVICE_IDS \ {"INT3404", }, /* Fan */ \ @@ -60,6 +61,38 @@ struct acpi_fan { struct device_attribute fine_grain_control; };
+/** + * acpi_fan_speed_valid - Check if fan speed value is valid + * @speeed: Speed value returned by the ACPI firmware + * + * Check if the fan speed value returned by the ACPI firmware is valid. This function is + * necessary as ACPI firmware implementations can return 0xFFFFFFFF to signal that the + * ACPI fan does not support speed reporting. Additionally, some buggy ACPI firmware + * implementations return a value larger than the 32-bit integer value defined by + * the ACPI specification when using placeholder values. Such invalid values are also + * detected by this function. + * + * Returns: True if the fan speed value is valid, false otherwise. + */ +static inline bool acpi_fan_speed_valid(u64 speed) +{ + return speed < U32_MAX; +} + +/** + * acpi_fan_power_valid - Check if fan power value is valid + * @power: Power value returned by the ACPI firmware + * + * Check if the fan power value returned by the ACPI firmware is valid. + * See acpi_fan_speed_valid() for details. + * + * Returns: True if the fan power value is valid, false otherwise. + */ +static inline bool acpi_fan_power_valid(u64 power) +{ + return power < U32_MAX; +} + int acpi_fan_get_fst(acpi_handle handle, struct acpi_fan_fst *fst); int acpi_fan_create_attributes(struct acpi_device *device); void acpi_fan_delete_attributes(struct acpi_device *device); diff --git a/drivers/acpi/fan_hwmon.c b/drivers/acpi/fan_hwmon.c index 4b2c2007f2d7f..47a02ef5a6067 100644 --- a/drivers/acpi/fan_hwmon.c +++ b/drivers/acpi/fan_hwmon.c @@ -15,10 +15,6 @@
#include "fan.h"
-/* Returned when the ACPI fan does not support speed reporting */ -#define FAN_SPEED_UNAVAILABLE U32_MAX -#define FAN_POWER_UNAVAILABLE U32_MAX - static struct acpi_fan_fps *acpi_fan_get_current_fps(struct acpi_fan *fan, u64 control) { unsigned int i; @@ -77,7 +73,7 @@ static umode_t acpi_fan_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_ * when the associated attribute should not be created. */ for (i = 0; i < fan->fps_count; i++) { - if (fan->fps[i].power != FAN_POWER_UNAVAILABLE) + if (acpi_fan_power_valid(fan->fps[i].power)) return 0444; }
@@ -106,7 +102,7 @@ static int acpi_fan_hwmon_read(struct device *dev, enum hwmon_sensor_types type, case hwmon_fan: switch (attr) { case hwmon_fan_input: - if (fst.speed == FAN_SPEED_UNAVAILABLE) + if (!acpi_fan_speed_valid(fst.speed)) return -ENODEV;
if (fst.speed > LONG_MAX) @@ -134,7 +130,7 @@ static int acpi_fan_hwmon_read(struct device *dev, enum hwmon_sensor_types type, if (!fps) return -EIO;
- if (fps->power == FAN_POWER_UNAVAILABLE) + if (!acpi_fan_power_valid(fps->power)) return -ENODEV;
if (fps->power > LONG_MAX / MICROWATT_PER_MILLIWATT)
linux-stable-mirror@lists.linaro.org
-
Sasha Levin