[PATCH AUTOSEL 7.0-5.10] ASoC: Intel: cht_bsw_rt5672: Fix MCLK leak on platform_clock_control error
From: Sasha Levin
Date: Mon Apr 20 2026 - 09:58:06 EST
From: Aravind Anilraj <aravindanilraj0702@xxxxxxxxx>
[ Upstream commit dced5a373a96cfd9f3bd0ffcf5339a7579d1473a ]
If snd_soc_dai_set_pll() or snd_soc_dai_set_sysclk() fail inside the
EVENT_ON path, the function returns without calling
clk_disable_unprepare() on ctx->mclk, which was already enabled earlier
in the same code path. Add the missing clk_disable_unprepare() calls
before returning the error.
Signed-off-by: Aravind Anilraj <aravindanilraj0702@xxxxxxxxx>
Reviewed-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
Link: https://patch.msgid.link/20260401220507.23557-4-aravindanilraj0702@xxxxxxxxx
Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
LLM Generated explanations, may be completely bogus:
I now have all the information needed for a complete analysis. Let me
compile the full assessment.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: PARSE THE SUBJECT LINE
Record: [ASoC: Intel: cht_bsw_rt5672] [Fix] [MCLK leak on
platform_clock_control error path]
### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- **Signed-off-by:** Aravind Anilraj (author)
- **Reviewed-by:** Cezary Rojewski (Intel ASoC reviewer)
- **Link:** patch.msgid.link to the original patch
- **Signed-off-by:** Mark Brown (ASoC subsystem maintainer, committer)
- No Fixes: tag (expected for manual review candidates)
- No Reported-by: tag
- No Cc: stable tag
Record: Reviewed by Intel ASoC developer and merged by ASoC subsystem
maintainer. No syzbot or external reporter — this is a code-inspection-
found bug.
### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The commit message clearly states: if `snd_soc_dai_set_pll()` or
`snd_soc_dai_set_sysclk()` fail in the EVENT_ON path, the function
returns without calling `clk_disable_unprepare()` on `ctx->mclk`, which
was already enabled. The fix adds the missing cleanup calls.
Record: Bug: clock resource leak on error paths. Symptom: MCLK left
enabled after error, preventing proper power management. Root cause:
missing cleanup in two error return paths.
### Step 1.4: DETECT HIDDEN BUG FIXES
This is explicitly labeled as a "Fix" and describes a resource leak — no
disguise here.
Record: This is an overt bug fix for a resource leak.
## PHASE 2: DIFF ANALYSIS
### Step 2.1: INVENTORY THE CHANGES
- **File:** `sound/soc/intel/boards/cht_bsw_rt5672.c`
- **Lines added:** +4 (two `if (ctx->mclk)
clk_disable_unprepare(ctx->mclk);` blocks)
- **Lines removed:** 0
- **Functions modified:** `platform_clock_control()`
- **Scope:** Single-file, surgical fix
Record: 1 file, 4 lines added, 0 removed. Single function modified.
Extremely contained.
### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
- **Before:** In the EVENT_ON path, after
`clk_prepare_enable(ctx->mclk)` succeeds, if either
`snd_soc_dai_set_pll()` or `snd_soc_dai_set_sysclk()` fails, the
function returns the error without disabling the clock.
- **After:** Both error paths now call
`clk_disable_unprepare(ctx->mclk)` (guarded by `if (ctx->mclk)`)
before returning the error.
Record: Error paths now properly clean up the enabled clock before
returning.
### Step 2.3: IDENTIFY THE BUG MECHANISM
**Category: Error path / resource leak fix.**
The clock `ctx->mclk` is enabled via `clk_prepare_enable()` at line 67.
If subsequent calls fail (lines 78-81 or 86-89), the function returns
without the matching `clk_disable_unprepare()`. This leaves the platform
clock running, preventing proper power management. The EVENT_OFF path at
line 103-104 already properly calls `clk_disable_unprepare()`,
confirming the intended pattern.
Record: Resource leak — MCLK left in enabled/prepared state on error
paths.
### Step 2.4: ASSESS THE FIX QUALITY
- **Obviously correct:** Yes. Mirrors the existing cleanup pattern in
the EVENT_OFF path (line 103-104). The `if (ctx->mclk)` guard is
consistent with the guard at line 66.
- **Minimal/surgical:** Yes. Only 4 lines added, no unrelated changes.
- **Regression risk:** Extremely low. Only affects error paths. The
`clk_disable_unprepare()` call is the exact counterpart to
`clk_prepare_enable()` that was already called.
Record: Fix is obviously correct, minimal, and has negligible regression
risk.
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: BLAME THE CHANGED LINES
The MCLK handling was introduced by commit `c25695ae88ce26` (Pierre-
Louis Bossart, 2017-06-23) — "ASoC: Intel: cht_bsw_rt5672: 19.2MHz clock
for Baytrail platforms". The bug has existed since that commit, which
was included in v4.13-rc1.
Record: Buggy code introduced in c25695ae88ce26 (v4.13-rc1, 2017).
Present in all active stable trees.
### Step 3.2: FOLLOW THE FIXES: TAG
No Fixes: tag present. However, the logical "Fixes:" target is
`c25695ae88ce26`, which introduced the MCLK handling without proper
error path cleanup.
Record: The implicit Fixes target (c25695ae88ce26) is in all stable
trees since v4.13.
### Step 3.3: CHECK FILE HISTORY
The file has had ~20 commits over the years, mostly cleanups and
conversions. No prior attempt to fix this specific leak was found (no
commits matching "MCLK leak" in the history).
Record: Standalone fix, no prerequisites needed. No prior fix for this
issue exists.
### Step 3.4: CHECK THE AUTHOR
The author (Aravind Anilraj) appears to be a new contributor. However,
the patch was reviewed by Cezary Rojewski (Intel ASoC reviewer) and
merged by Mark Brown (ASoC subsystem maintainer).
Record: Reviewed by experienced subsystem developers despite being from
a new contributor.
### Step 3.5: CHECK FOR DEPENDENCIES
The fix only adds `clk_disable_unprepare()` calls to existing error
paths. No new functions, structures, or APIs are used. The fix is
completely self-contained.
Record: No dependencies. Can apply standalone.
## PHASE 4: MAILING LIST RESEARCH
### Step 4.1-4.5: PATCH DISCUSSION
The lore.kernel.org site was not reachable due to bot protection. b4 dig
could not find the commit by message-id (it doesn't exist in this tree
as a commit). However, the commit tags show:
- **Reviewed-by:** Cezary Rojewski (Intel) — an active ASoC reviewer
- **Signed-off-by:** Mark Brown — the ASoC subsystem maintainer who
merged it
These signatures indicate the patch went through proper review.
Record: Proper review by Intel ASoC developer and subsystem maintainer.
Could not access lore discussion directly.
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1-5.2: FUNCTION AND CALLERS
`platform_clock_control()` is registered as a DAPM supply widget
callback:
```114:116:sound/soc/intel/boards/cht_bsw_rt5672.c
SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
platform_clock_control, SND_SOC_DAPM_PRE_PMU |
SND_SOC_DAPM_POST_PMD),
```
It is called by the DAPM framework whenever the "Platform Clock" supply
widget powers up (PRE_PMU) or down (POST_PMD). This happens during every
audio playback/capture start and stop operation. All audio paths
("Headphone", "Headset Mic", "Int Mic", "Ext Spk") depend on this widget
(lines 130-133).
Record: Called on every audio stream open/close. High frequency for
audio-active systems.
### Step 5.3-5.5: SIMILAR PATTERNS
I found the same bug pattern in sibling drivers `bytcr_rt5651.c` and
`bytcr_rt5640.c`, though those have slightly different code structure.
The `cht_bsw_rt5672.c` fix is specific to this driver and doesn't
require changes to siblings.
Record: Similar pattern exists in sibling drivers but this fix is self-
contained for cht_bsw_rt5672.
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: BUGGY CODE IN STABLE
The buggy code was introduced in v4.13-rc1 (commit c25695ae88ce26). It
exists in all active stable trees (5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y,
etc.).
Record: Bug affects all active stable trees.
### Step 6.2: BACKPORT COMPLICATIONS
The file has been relatively stable. The current code at the fix site
matches what was introduced in c25695ae88ce26, with only minor changes
(the `if (ctx->mclk)` guard and the `65b2df10a1e62` commit that changed
the EVENT_OFF path). The fix should apply cleanly or with minimal
adjustment.
Record: Expected clean or near-clean apply to all stable trees.
### Step 6.3: RELATED FIXES
No prior fix for this specific issue was found in any tree.
Record: No existing fix for this leak.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: SUBSYSTEM CRITICALITY
- **Subsystem:** sound/soc/intel/boards — ASoC Intel machine driver
- **Criticality:** PERIPHERAL (specific to Cherryview/Baytrail platforms
with RT5672 codec)
- **Affected hardware:** Lenovo and other Cherryview/Baytrail-based
laptops/tablets with RT5670/RT5672 codec (reasonably common consumer
devices)
Record: ASoC Intel board driver, PERIPHERAL but for consumer devices
(laptops/tablets).
### Step 7.2: SUBSYSTEM ACTIVITY
The file sees occasional updates (cleanups and fixes). It's a mature
driver.
Record: Mature, stable driver with occasional maintenance commits.
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: AFFECTED USERS
Users of Cherryview/Baytrail platforms with RT5670/RT5672 codec. This
includes Lenovo and similar consumer laptops/tablets from the Baytrail
era.
Record: Platform-specific but for real consumer devices.
### Step 8.2: TRIGGER CONDITIONS
The bug triggers when `snd_soc_dai_set_pll()` or
`snd_soc_dai_set_sysclk()` fails during audio stream start. While these
failures are not common in normal operation, they can occur during
hardware errors, suspend/resume transitions, or codec communication
issues.
Record: Trigger requires PLL/sysclk configuration failure during audio
start. Uncommon but possible.
### Step 8.3: FAILURE MODE SEVERITY
When triggered, the platform clock remains enabled (leaked), preventing
proper power management. Repeated triggering could cause increased power
consumption. The clock framework may also track prepare/enable counts
incorrectly, potentially affecting system suspend or causing warnings.
Record: Severity: MEDIUM (resource leak affecting power management, no
crash).
### Step 8.4: RISK-BENEFIT RATIO
- **Benefit:** Fixes a real resource leak on error paths, improves power
management reliability.
- **Risk:** Very low — 4 lines added, only on error paths, mirrors
existing cleanup patterns.
- **Ratio:** Favorable. Even though the trigger is uncommon, the fix is
trivially safe.
Record: Benefit outweighs risk. Minimal fix with no regression
potential.
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: EVIDENCE COMPILATION
**FOR backporting:**
- Fixes a real clock resource leak (missing `clk_disable_unprepare()` on
error paths)
- Extremely small and surgical (4 lines, single file, single function)
- Obviously correct — mirrors existing cleanup pattern in the same
function
- Bug has existed since v4.13-rc1 (2017), affects all stable trees
- Reviewed by Intel ASoC developer, merged by subsystem maintainer
- No dependencies, self-contained
- No regression risk
**AGAINST backporting:**
- No user reports of the issue (code-inspection find)
- Trigger requires error conditions that are uncommon in normal
operation
- Driver-specific (not core kernel)
### Step 9.2: STABLE RULES CHECKLIST
1. Obviously correct and tested? **YES** — reviewed by Intel developer,
merged by maintainer
2. Fixes a real bug? **YES** — clock resource leak on error paths
3. Important issue? **MEDIUM** — resource leak, not a crash
4. Small and contained? **YES** — 4 lines, single file
5. No new features or APIs? **YES** — only adds cleanup calls
6. Can apply to stable? **YES** — code unchanged since introduction
### Step 9.3: EXCEPTION CATEGORIES
Not an exception category — this is a straightforward bug fix.
### Step 9.4: DECISION
This is a small, obvious, and safe fix for a real resource leak. The fix
adds missing `clk_disable_unprepare()` calls on two error paths where
the clock was already enabled. It's reviewed by the right people, has no
dependencies, and applies to all stable trees.
## Verification
- [Phase 1] Parsed tags: Reviewed-by Cezary Rojewski (Intel), merged by
Mark Brown (ASoC maintainer)
- [Phase 2] Diff analysis: 4 lines added, two `if (ctx->mclk)
clk_disable_unprepare(ctx->mclk)` blocks on error paths
- [Phase 3] git blame: buggy code introduced in c25695ae88ce26
(v4.13-rc1, 2017), present in all stable trees
- [Phase 3] git show c25695ae88ce26: confirmed original commit added
MCLK handling without error path cleanup
- [Phase 3] git describe --contains c25695ae88ce26:
v4.13-rc1~142^2~1^2~5^2~8 — confirmed it's been in mainline since
v4.13
- [Phase 3] git log --oneline -20 -- file: no prior MCLK leak fix found
- [Phase 4] b4 dig: could not find commit by message-id in local tree;
lore blocked by bot protection
- [Phase 5] DAPM widget registration at lines 114-116 confirms
`platform_clock_control` is called on every audio stream start/stop
- [Phase 5] DAPM routes at lines 130-133 confirm all audio paths depend
on Platform Clock
- [Phase 5] Grep for clk_disable_unprepare in sibling drivers confirms
the same cleanup pattern is expected
- [Phase 6] Code at the fix site is essentially unchanged since
c25695ae88ce26 — clean apply expected
- [Phase 8] Failure mode: clock resource leak preventing proper power
management, severity MEDIUM
- UNVERIFIED: Could not access lore.kernel.org discussion for reviewer
commentary on stable suitability
**YES**
sound/soc/intel/boards/cht_bsw_rt5672.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index 359723f2700e4..57d6997eb12ff 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -77,6 +77,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
CHT_PLAT_CLK_3_HZ, 48000 * 512);
if (ret < 0) {
dev_err(card->dev, "can't set codec pll: %d\n", ret);
+ if (ctx->mclk)
+ clk_disable_unprepare(ctx->mclk);
return ret;
}
@@ -85,6 +87,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
48000 * 512, SND_SOC_CLOCK_IN);
if (ret < 0) {
dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
+ if (ctx->mclk)
+ clk_disable_unprepare(ctx->mclk);
return ret;
}
} else {
--
2.53.0