public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>,
	Mark Brown <broonie@kernel.org>, Sasha Levin <sashal@kernel.org>,
	lgirdwood@gmail.com, perex@perex.cz, tiwai@suse.com,
	Frank.Li@nxp.com, linux-sound@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 7.0-6.1] ASoC: mxs-sgtl5000: disable MCLK on error paths of mxs_sgtl5000_probe()
Date: Mon, 20 Apr 2026 09:18:33 -0400	[thread overview]
Message-ID: <20260420132314.1023554-119-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>

From: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>

[ Upstream commit c8ef13d692f19cdbbf195fb845421a5b71801704 ]

Call mxs_saif_put_mclk() to disable MCLK on error
paths of mxs_sgtl5000_probe().

Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
Link: https://patch.msgid.link/20260401053051.586290-1-lihaoxiang@isrc.iscas.ac.cn
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a complete picture. Let me compile the full analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: Subject Line**
- Subsystem: `ASoC: mxs-sgtl5000`
- Action verb: "disable" (implies fixing a missing cleanup)
- Summary: Disable MCLK on error paths of `mxs_sgtl5000_probe()`
- Record: [ASoC/mxs-sgtl5000] [disable/fix] [Add missing
  mxs_saif_put_mclk() on probe error paths]

**Step 1.2: Tags**
- Signed-off-by: Haoxiang Li (author) - a contributor focused on error-
  path resource leak fixes
- Link: `https://patch.msgid.link/20260401053051.586290-1-
  lihaoxiang@isrc.iscas.ac.cn`
- Signed-off-by: Mark Brown (ASoC maintainer applied the patch)
- No Fixes: tag, no Reported-by, no Cc: stable (expected for review
  candidates)

**Step 1.3: Commit Body**
The message is concise: call `mxs_saif_put_mclk()` to disable MCLK on
error paths. The bug is a resource leak - `mxs_saif_get_mclk()` enables
a hardware clock, and if probe fails after that point, the clock remains
enabled.

**Step 1.4: Hidden Bug Fix Detection**
This IS a resource leak fix. The wording "disable MCLK on error paths"
is a classic resource-leak-on-error-path fix pattern.

## PHASE 2: DIFF ANALYSIS

**Step 2.1: Inventory**
- 1 file changed: `sound/soc/mxs/mxs-sgtl5000.c`
- 2 lines added: two `mxs_saif_put_mclk(0)` calls + braces adjustment
- Functions modified: `mxs_sgtl5000_probe()`
- Scope: single-file surgical fix

**Step 2.2: Code Flow Change**
Two error paths are fixed:

1. **`snd_soc_of_parse_audio_routing()` failure** (line 160): BEFORE:
   returned directly without disabling MCLK. AFTER: calls
   `mxs_saif_put_mclk(0)` before returning.

2. **`devm_snd_soc_register_card()` failure** (line 165): BEFORE:
   returned directly without disabling MCLK. AFTER: calls
   `mxs_saif_put_mclk(0)` before returning.

**Step 2.3: Bug Mechanism**
Category: **Error path / resource leak fix**.

`mxs_saif_get_mclk()` at line 144:
- Calls `__mxs_saif_get_mclk()` which sets `saif->mclk_in_use = 1`
- Calls `clk_prepare_enable(saif->clk)` to enable the hardware clock
- Writes to SAIF_CTRL to enable MCLK output

When probe fails after this, `mxs_saif_put_mclk()` (which disables the
clock, clears MCLK output, and sets `mclk_in_use = 0`) is never called.
The `remove()` callback only runs if `probe()` succeeded.

**Step 2.4: Fix Quality**
- Obviously correct: mirrors the cleanup done in `mxs_sgtl5000_remove()`
- Minimal/surgical: only 2 meaningful lines added
- Regression risk: essentially zero - only affects error paths
- The fix follows the exact same pattern as the existing `remove()`
  function

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: Blame Analysis**
- The `mxs_saif_get_mclk()` call was introduced in the original driver
  commit `fcb5e47eff29a1` (2011, v3.2). So the register_card error-path
  leak has existed since 2011.
- The audio-routing error path was introduced by `949293d45d6b09` (2018,
  v4.16) which added `snd_soc_of_parse_audio_routing()` without cleanup
  on failure.
- Both error paths predate all active stable trees.

**Step 3.2: No Fixes: tag** (expected for review candidates)

**Step 3.3: Related Changes**
- `6ae0a4d8fec55` (2022): Fixed a different resource leak (of_node_put)
  in the same probe function - shows this function has a history of
  incomplete error handling.
- `7a17f6a95a613` (2021): Switched to `dev_err_probe()` for
  register_card failure.

**Step 3.4: Author Context**
Haoxiang Li is a prolific error-path/resource-leak fix contributor.
Their commit history shows many similar fixes across kernel subsystems
(PCI, SCSI, media, DRM, clock, bus drivers).

**Step 3.5: Dependencies**
No dependencies. The fix only adds `mxs_saif_put_mclk(0)` calls, which
has existed since the driver was created. Should apply cleanly to all
stable trees.

## PHASE 4: MAILING LIST RESEARCH

Lore was not accessible due to bot protection. However:
- The patch was accepted by Mark Brown (ASoC subsystem maintainer)
  directly
- The Link tag points to the original submission
- Single-version submission (no v2/v3), suggesting it was
  straightforward

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: Functions Modified**
Only `mxs_sgtl5000_probe()` is modified.

**Step 5.2: Resource Chain**
- `mxs_saif_get_mclk()` → `__mxs_saif_get_mclk()` → sets
  `mclk_in_use=1`, clears CLKGATE, configures clock rate →
  `clk_prepare_enable()` → writes SAIF_CTRL to enable MCLK RUN
- `mxs_saif_put_mclk()` → `clk_disable_unprepare()` → sets CLKGATE →
  clears RUN → `mclk_in_use=0`

Without the fix, on error: clock stays enabled, hardware MCLK output
stays active, `mclk_in_use` remains 1 (preventing future attempts to get
MCLK).

## PHASE 6: STABLE TREE ANALYSIS

**Step 6.1: Code Presence**
The buggy code exists in ALL active stable trees:
- The register_card error leak exists since v3.2 (2011)
- The audio-routing error leak exists since v4.16 (2018)
- All stable trees (5.4.y, 5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y) contain
  both bugs

**Step 6.2: Backport Difficulty**
The patch should apply cleanly or with trivial fuzz. Stable trees older
than 6.1 use `of_find_property()` instead of `of_property_present()`,
but the error path code is unchanged. The `devm_snd_soc_register_card`
error path in trees before 5.15 uses slightly different error printing
(not `dev_err_probe`), but the fix location is the same.

## PHASE 7: SUBSYSTEM CONTEXT

- Subsystem: ASoC (sound/soc) - audio driver infrastructure
- Criticality: PERIPHERAL (MXS/i.MX28 embedded audio, specific platform)
- The MXS SAIF + SGTL5000 combination is used on Freescale/NXP i.MX28
  boards

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: Affected Users**
Platform-specific: users of i.MX28 boards with SGTL5000 audio codec
(embedded systems).

**Step 8.2: Trigger**
Probe failure on the audio device. This can be triggered by:
- Invalid/malformed audio-routing DT property
- `devm_snd_soc_register_card()` failure (e.g., codec not ready, probe
  deferral errors)

**Step 8.3: Failure Mode**
- Clock resource leak (hardware clock left enabled, consuming power)
- `mclk_in_use` flag remains set, potentially blocking future MCLK
  acquisition
- Severity: MEDIUM (resource leak, not crash)

**Step 8.4: Risk-Benefit**
- BENEFIT: Fixes resource leak on error paths. Clean cleanup of hardware
  state.
- RISK: Very low. Only 2 lines, only error paths, mirrors existing
  remove() logic.
- Ratio: Favorable. Very low risk fix for a real resource leak.

## PHASE 9: FINAL SYNTHESIS

**Evidence FOR backporting:**
- Fixes a real resource leak (hardware clock left enabled on probe
  failure)
- Fixes `mclk_in_use` state leak that can prevent subsequent attempts
- Tiny, surgical fix (2 lines of meaningful code)
- Obviously correct (mirrors the cleanup in `remove()`)
- Accepted by ASoC subsystem maintainer (Mark Brown)
- Bug exists in all stable trees (since v3.2/v4.16)
- Zero regression risk (only error paths affected)
- The same function had a prior similar fix (`6ae0a4d8fec55` for
  of_node_put)

**Evidence AGAINST:**
- Platform-specific driver (limited user base, i.MX28)
- No Reported-by (found by code review, not user complaint)
- Resource leak, not crash/security/corruption
- No Fixes: tag (but expected for review candidates)

**Stable Rules Checklist:**
1. Obviously correct and tested? **YES** - mirrors remove() pattern,
   accepted by maintainer
2. Fixes a real bug? **YES** - clock resource leak on error
3. Important issue? **MEDIUM** - resource leak, not critical severity
4. Small and contained? **YES** - 2 lines, 1 file
5. No new features? **YES** - pure bugfix
6. Can apply to stable? **YES** - should apply cleanly to all active
   trees

## Verification

- [Phase 1] Parsed tags: Link to patch.msgid.link, SOB by Mark Brown
  (maintainer), author Haoxiang Li
- [Phase 2] Diff analysis: 2 lines of `mxs_saif_put_mclk(0)` added to
  two error paths in `mxs_sgtl5000_probe()`
- [Phase 3] git blame: `mxs_saif_get_mclk()` introduced in
  fcb5e47eff29a1 (v3.2, 2011); audio-routing error path introduced in
  949293d45d6b09 (v4.16, 2018)
- [Phase 3] git show 949293d45d6b09: confirmed it added
  `snd_soc_of_parse_audio_routing()` error path without cleanup
- [Phase 3] git show 6ae0a4d8fec55: confirmed prior error-handling fix
  in same function (of_node_put leak)
- [Phase 3] Author commits: Haoxiang Li is a serial error-path leak
  fixer
- [Phase 4] Lore inaccessible due to bot protection; patch accepted by
  Mark Brown directly
- [Phase 5] Read mxs_saif_get_mclk: confirms it enables clock + sets
  mclk_in_use=1; put_mclk disables clock + clears flag
- [Phase 6] Code exists in all stable trees from 5.4.y through 6.12.y
- [Phase 6] git log v6.6..HEAD: only 4 commits to this file since v6.6,
  none conflicting
- [Phase 8] Failure mode: clock resource leak + mclk_in_use state leak
  on probe error, severity MEDIUM

The fix is small, surgical, obviously correct, and fixes a real resource
leak that has existed since 2011/2018. It meets all stable kernel
criteria.

**YES**

 sound/soc/mxs/mxs-sgtl5000.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
index 245f174116384..f1c0e612313dd 100644
--- a/sound/soc/mxs/mxs-sgtl5000.c
+++ b/sound/soc/mxs/mxs-sgtl5000.c
@@ -157,13 +157,16 @@ static int mxs_sgtl5000_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(&pdev->dev, "failed to parse audio-routing (%d)\n",
 				ret);
+			mxs_saif_put_mclk(0);
 			return ret;
 		}
 	}
 
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
-	if (ret)
+	if (ret) {
+		mxs_saif_put_mclk(0);
 		return dev_err_probe(&pdev->dev, ret, "snd_soc_register_card failed\n");
+	}
 
 	return 0;
 }
-- 
2.53.0



  parent reply	other threads:[~2026-04-20 13:26 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-5.10] clk: spear: fix resource leak in clk_register_vco_pll() Sasha Levin
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-5.10] ARM: xen: validate hypervisor compatible before parsing its version Sasha Levin
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.19] wifi: mt76: avoid to set ACK for MCU command if wait_resp is not set Sasha Levin
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.18] phy: phy-mtk-tphy: Update names and format of kernel-doc comments Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 6.18] ARM: dts: microchip: sam9x7: fix gpio-lines count for pioB Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.18] drm/imx: parallel-display: add DRM_DISPLAY_HELPER for DRM_IMX_PARALLEL_DISPLAY Sasha Levin
2026-04-20 13:18 ` Sasha Levin [this message]
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.12] Bluetooth: btmtk: add MT7902 MCU support Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: flush pending TX before channel switch Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.6] wifi: mt76: fix list corruption in mt76_wcid_cleanup Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: add missing lock protection in mt76_sta_state for sta_event callback Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-6.1] Bluetooth: btmtk: improve mt79xx firmware setup retry flow Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: Disable Rx hdr_trans in monitor mode Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.12] drivers/virt: pkvm: Add Kconfig dependency on DMA_RESTRICTED_POOL Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: mt7925: Skip scan process during suspend Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-5.10] wifi: mt76: mt76x02: wake queues after reconfig Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.12] wifi: mt76: mt7925: resolve link after acquiring mt76 mutex Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 6.18] ASoC: stm32_sai: fix incorrect BCLK polarity for DSP_A/B, LEFT_J Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-6.12] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628 Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 6.18] media: rkvdec: reduce stack usage in rkvdec_init_v4l2_vp9_count_tbl() Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 6.18] soc: aspeed: socinfo: Mask table entries for accurate SoC ID matching Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 6.18] arm64: dts: imx93-9x9-qsb: change usdhc tuning step for eMMC and SD Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-6.6] wifi: mt76: mt7996: reset device after MCU message timeout Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0] spi: stm32: fix rx DMA request error handling Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 7.0-5.10] drm/mediatek: mtk_dsi: enable hs clock during pre-enable Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 7.0-6.18] wifi: mt76: mt7996: fix frequency separation for station STR mode Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 6.18] arm64: dts: imx8mq: Set the correct gpu_ahb clock frequency Sasha Levin
2026-04-20 13:22 ` [PATCH AUTOSEL 6.18] net: airoha: Fix memory leak in airoha_qdma_rx_process() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260420132314.1023554-119-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=broonie@kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=lgirdwood@gmail.com \
    --cc=lihaoxiang@isrc.iscas.ac.cn \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=perex@perex.cz \
    --cc=stable@vger.kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox