ASoC: mediatek: mt8183: Fix clock error handling

PATCH Superseded [email protected] 3 participants 8 messages Sep 3 – Sep 26

Subsystems (3)

Sound ASoC Arm MediaTek

lore

Share & tools

lore

[email protected]

author

2026-09-03 04:52 UTC #0 lore raw

v4 cover ASoC: mediatek: mt8183: Fix clock error handling

From: bui duc phuc <[email protected]>

Hi all,

This series fixes and cleans up clock error handling in the MT8183 ASoC driver.

Compile tested only.

Link v2:
https://lore.kernel.org/all/[email protected]/

Link v3:
https://lore.kernel.org/all/[email protected]/

Changes in v2:
- Remove the redundant error log for apll1_mux_setting error handling.
- Add Fixes tags.
- Add a cover letter.
Changes in v3:
- Add a patch to fix clock handling in the mux disable path, as suggested by Cezary.
- Return directly from apllx_mux_setting() instead of using a goto label, as suggested by Cezary.
- Add a Reviewed-by tag.
Changes in v4:
- Add a Reviewed-by tag from Cezary.
- In the mux disable path, remove the goto labels so that all teardown steps are attempted and any errors are only reported.

Best regards,
Phuc

bui duc phuc (5):
ASoC: mediatek: mt8183: Fix wrong clock cleanup on clk_set_parent()
failure
ASoC: mediatek: mt8183: Fix clock handling in mux disable path
ASoC: mediatek: mt8183: Fix APLL enable error handling
ASoC: mediatek: mt8183: Use dev_err_probe() for error handling
ASoC: mediatek: mt8183: Drop redundant probe error messages

 sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 31 +++++++++++-----------
 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c |  8 ++----
 2 files changed, 17 insertions(+), 22 deletions(-)

Signature · 1 line

2.43.0

[email protected]

author

2026-09-03 04:52 UTC #1 lore raw

PATCH v4 1/5 ASoC: mediatek: mt8183: Fix wrong clock cleanup on clk_set_parent() failure

From: bui duc phuc <[email protected]>

In mt8183_afe_enable_clock(), when clk_set_parent() fails, the current error path incorrectly cleans up the previously enabled clock instead of the clock used by clk_set_parent().

Fix the error path to clean up the correct clock when clk_set_parent() fails.

Fixes a94aec035a12 ("ASoC: mediatek: mt8183: add platform driver")

Reviewed-by Cezary Rojewski <[email protected]>

Signed-off-by bui duc phuc <[email protected]>

---

Changes in v3:
- Add a Reviewed-by tag.

 sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

sound/soc/mediatek/mt8183/mt8183-afe-clk.c +2 -2

index cc4f8f4d3dab..6ab0734ad136 100644
@@ -137,7 +137,7 @@ int mt8183_afe_enable_clock(struct mtk_base_afe *afe)
 		dev_err(afe->dev, "%s(), clk_set_parent %s-%s fail %d\n",
 			__func__, aud_clks[CLK_MUX_AUDIO],
 			aud_clks[CLK_CLK26M], ret);
-		goto CLK_MUX_AUDIO_ERR;
+		goto CLK_MUX_AUDIO_INTBUS_ERR;
 	}
 
 	ret = clk_prepare_enable(afe_priv->clk[CLK_MUX_AUDIOINTBUS]);

@@ -153,7 +153,7 @@ int mt8183_afe_enable_clock(struct mtk_base_afe *afe)
 		dev_err(afe->dev, "%s(), clk_set_parent %s-%s fail %d\n",
 			__func__, aud_clks[CLK_MUX_AUDIOINTBUS],
 			aud_clks[CLK_TOP_SYSPLL_D2_D4], ret);
-		goto CLK_MUX_AUDIO_INTBUS_ERR;
+		goto CLK_AFE_ERR;
 	}
 
 	ret = clk_prepare_enable(afe_priv->clk[CLK_AFE]);

Signature · 1 line

2.43.0

[email protected]

author

2026-09-03 04:52 UTC #2 lore raw

PATCH v4 2/5 ASoC: mediatek: mt8183: Fix clock handling in mux disable path

In reply to [email protected]

From: bui duc phuc <[email protected]>

In the disable path of the appl*_mux_setting() functions, clk_disable_unprepare() is called after clk_set_parent(). If clk_set_parent() fails, clk_disable_unprepare() is skipped, potentially leaving one of the clocks enabled.

Remove the goto exit paths so that all teardown steps are attempted and any errors are only reported.

Fixes a94aec035a12 ("ASoC: mediatek: mt8183: add platform driver")

Signed-off-by bui duc phuc <[email protected]>

---

Changes in v4:
- In the mux disable path, remove the goto labels so that all teardown steps are attempted and any errors are only reported.

 sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 6 ------
 1 file changed, 6 deletions(-)

sound/soc/mediatek/mt8183/mt8183-afe-clk.c +0 -6

index 6ab0734ad136..dd1a5925af96 100644
@@ -271,7 +271,6 @@ static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
 			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
 				__func__, aud_clks[CLK_TOP_MUX_AUD_ENG1],
 				aud_clks[CLK_CLK26M], ret);
-			goto EXIT;
 		}
 		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_ENG1]);

@@ -281,7 +280,6 @@ static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
 			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
 				__func__, aud_clks[CLK_TOP_MUX_AUD_1],
 				aud_clks[CLK_CLK26M], ret);
-			goto EXIT;
 		}
 		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_1]);
 	}

@@ -298,7 +296,6 @@ static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
 		       afe_priv->clk[CLK_CLK26M]);
 	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_1]);
 ERR_ENABLE_CLK_TOP_MUX_AUD_1:
-EXIT:
 	return ret;
 }

@@ -345,7 +342,6 @@ static int apll2_mux_setting(struct mtk_base_afe *afe, bool enable)
 			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
 				__func__, aud_clks[CLK_TOP_MUX_AUD_ENG2],
 				aud_clks[CLK_CLK26M], ret);
-			goto EXIT;
 		}
 		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_ENG2]);

@@ -355,7 +351,6 @@ static int apll2_mux_setting(struct mtk_base_afe *afe, bool enable)
 			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
 				__func__, aud_clks[CLK_TOP_MUX_AUD_2],
 				aud_clks[CLK_CLK26M], ret);
-			goto EXIT;
 		}
 		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_2]);
 	}

@@ -372,7 +367,6 @@ static int apll2_mux_setting(struct mtk_base_afe *afe, bool enable)
 		       afe_priv->clk[CLK_CLK26M]);
 	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_2]);
 ERR_ENABLE_CLK_TOP_MUX_AUD_2:
-EXIT:
 	return ret;
 }

Signature · 1 line

2.43.0

Cezary Rojewski

Reviewed-by on v4 2/5

2026-09-03 11:01 UTC #3 lore raw

Re: [PATCH v4 2/5] ASoC: mediatek: mt8183: Fix clock handling in mux disable path

Quoted 24 lines · On 9/3/2026 6:52 AM, [email protected] wrote:

Changes in v4:
- In the mux disable path, remove the goto labels so that all teardown steps are attempted and any errors are only reported.

 sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 6 ------
 1 file changed, 6 deletions(-)

sound/soc/mediatek/mt8183/mt8183-afe-clk.c +0 -2

@@ -271,7 +271,6 @@ static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
 			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
 				__func__, aud_clks[CLK_TOP_MUX_AUD_ENG1],
 				aud_clks[CLK_CLK26M], ret);
-			goto EXIT;
 		}
 		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_ENG1]);

@@ -281,7 +280,6 @@ static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
 			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
 				__func__, aud_clks[CLK_TOP_MUX_AUD_1],
 				aud_clks[CLK_CLK26M], ret);
-			goto EXIT;
 		}
 		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_1]);
 	}

sound/soc/mediatek/mt8183/mt8183-afe-clk.c static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable) · line 300

Quoted 6 lines

sound/soc/mediatek/mt8183/mt8183-afe-clk.c +0 -1

@@ -298,7 +296,6 @@ static int apll1_mux_setting(struct mtk_base_afe *afe, bool enable)
 		       afe_priv->clk[CLK_CLK26M]);
 	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_1]);
 ERR_ENABLE_CLK_TOP_MUX_AUD_1:
-EXIT:
 	return ret;
 }

When looking at this again, I'd probably refactor the whole thing so we have a permissive teardown path but still propagate the error code in case there is a caller who does care about the result of the disable scenario. Alas, nothing in this file ever checks the results of apll1/2_mux_setting() so the callers needed an update too. Well, let's just say that's a tale for a separate patchset..

TLDR: the patch is still an improvement and the (possible) refactor is a outside of its scope.

Reviewed-by Cezary Rojewski <[email protected]>

[email protected]

author

2026-09-03 04:52 UTC #4 lore raw

PATCH v4 3/5 ASoC: mediatek: mt8183: Fix APLL enable error handling

In reply to [email protected]

From: bui duc phuc <[email protected]>

Currently, the mt8183_apll*_enable() functions call mux_setting(afe, true) but do not check its return value to handle failures.

In addition, the cleanup paths of mt8183_apll*_enable() do not call mux_setting(afe, false) when the enable operation fails, while the mt8183_apll*_disable() functions do.

Add error handling for apll*_mux_setting() and call mux_setting(afe, false) in the cleanup paths when mt8183_apll*_enable() fails.

Fixes a94aec035a12 ("ASoC: mediatek: mt8183: add platform driver")

Reviewed-by Cezary Rojewski <[email protected]>

Signed-off-by bui duc phuc <[email protected]>

---

Changes in v3:
- Return directly from apllx_mux_setting() instead of using a goto label, as suggested by Cezary.
Changes in v4:
- Add a Reviewed-by tag from Cezary.

 sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

sound/soc/mediatek/mt8183/mt8183-afe-clk.c +10 -2

index dd1a5925af96..06556689976d 100644
@@ -376,7 +376,9 @@ int mt8183_apll1_enable(struct mtk_base_afe *afe)
 	int ret;
 
 	/* setting for APLL */
-	apll1_mux_setting(afe, true);
+	ret = apll1_mux_setting(afe, true);
+	if (ret)
+		return ret;
 
 	ret = clk_prepare_enable(afe_priv->clk[CLK_APLL22M]);
 	if (ret) {

@@ -405,6 +407,8 @@ int mt8183_apll1_enable(struct mtk_base_afe *afe)
 ERR_CLK_APLL1_TUNER:
 	clk_disable_unprepare(afe_priv->clk[CLK_APLL22M]);
 ERR_CLK_APLL22M:
+	apll1_mux_setting(afe, false);
+
 	return ret;
 }

@@ -430,7 +434,9 @@ int mt8183_apll2_enable(struct mtk_base_afe *afe)
 	int ret;
 
 	/* setting for APLL */
-	apll2_mux_setting(afe, true);
+	ret = apll2_mux_setting(afe, true);
+	if (ret)
+		return ret;
 
 	ret = clk_prepare_enable(afe_priv->clk[CLK_APLL24M]);
 	if (ret) {

@@ -459,6 +465,8 @@ int mt8183_apll2_enable(struct mtk_base_afe *afe)
 ERR_CLK_APLL2_TUNER:
 	clk_disable_unprepare(afe_priv->clk[CLK_APLL24M]);
 ERR_CLK_APLL24M:
+	apll2_mux_setting(afe, false);
+
 	return ret;
 }

Signature · 1 line

2.43.0

[email protected]

author

2026-09-03 04:53 UTC #5 lore raw

PATCH v4 4/5 ASoC: mediatek: mt8183: Use dev_err_probe() for error handling

In reply to [email protected]

From: bui duc phuc <[email protected]>

Replace dev_err() with dev_err_probe() to prevent log spam when probe returns -EPROBE_DEFER.

Reviewed-by Cezary Rojewski <[email protected]>

Signed-off-by bui duc phuc <[email protected]>

---

Changes in v3:
- Add a Reviewed-by tag.

 sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

sound/soc/mediatek/mt8183/mt8183-afe-clk.c +3 -6

index 06556689976d..40f9afcdaa16 100644
@@ -101,12 +101,9 @@ int mt8183_init_clock(struct mtk_base_afe *afe)
 
 	for (i = 0; i < CLK_NUM; i++) {
 		afe_priv->clk[i] = devm_clk_get(afe->dev, aud_clks[i]);
-		if (IS_ERR(afe_priv->clk[i])) {
-			dev_err(afe->dev, "%s(), devm_clk_get %s fail, ret %ld\n",
-				__func__, aud_clks[i],
-				PTR_ERR(afe_priv->clk[i]));
-			return PTR_ERR(afe_priv->clk[i]);
-		}
+		if (IS_ERR(afe_priv->clk[i]))
+			return dev_err_probe(afe->dev, PTR_ERR(afe_priv->clk[i]),
+					     "failed to get clock %s\n", aud_clks[i]);
 	}
 
 	return 0;

Signature · 1 line

2.43.0

[email protected]

author

2026-09-03 04:53 UTC #6 lore raw

PATCH v4 5/5 ASoC: mediatek: mt8183: Drop redundant probe error messages

In reply to [email protected]

From: bui duc phuc <[email protected]>

The errors handled here are already reported by the called functions, either directly or deeper in the call chain. Therefore, the additional dev_err() calls are redundant and can be removed.

Reviewed-by Cezary Rojewski <[email protected]>

Signed-off-by bui duc phuc <[email protected]>

---

Changes in v3:
- Add a Reviewed-by tag.

 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

sound/soc/mediatek/mt8183/mt8183-afe-pcm.c +2 -6

index 2634699534db..46b7a2bb6aaa 100644
@@ -809,10 +809,8 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	/* initial audio related clock */
 	ret = mt8183_init_clock(afe);
-	if (ret) {
-		dev_err(dev, "init clock error\n");
+	if (ret)
 		return ret;
-	}
 
 	pm_runtime_enable(dev);

@@ -903,10 +901,8 @@ static int mt8183_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq_id, mt8183_afe_irq_handler,
 			       IRQF_TRIGGER_NONE, "asys-isr", (void *)afe);
-	if (ret) {
-		dev_err(dev, "could not request_irq for asys-isr\n");
+	if (ret)
 		goto err_pm_disable;
-	}
 
 	/* init sub_dais */
 	INIT_LIST_HEAD(&afe->sub_dais);

Signature · 1 line

2.43.0

Mark Brown

maintainer

2026-09-26 00:42 UTC #7 lore raw

In reply to [email protected]

Quoted 10 lines · On Thu, 03 Sep 2026 11:51:58 +0700, [email protected] wrote:

ASoC: mediatek: mt8183: Fix clock error handling
From: bui duc phuc <[email protected]>
Hi all,
This series fixes and cleans up clock error handling in the MT8183 ASoC driver.
[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4

Thanks!

[1/5] ASoC: mediatek: mt8183: Fix wrong clock cleanup on clk_set_parent() failure

      https://git.kernel.org/broonie/sound/c/7f05596bd404
[2/5] ASoC: mediatek: mt8183: Fix clock handling in mux disable path
      https://git.kernel.org/broonie/sound/c/4a0f9566ae0c
[3/5] ASoC: mediatek: mt8183: Fix APLL enable error handling
      https://git.kernel.org/broonie/sound/c/30cf48383006
[4/5] ASoC: mediatek: mt8183: Use dev_err_probe() for error handling
      https://git.kernel.org/broonie/sound/c/21cc0c50d395
[5/5] ASoC: mediatek: mt8183: Drop redundant probe error messages
      https://git.kernel.org/broonie/sound/c/f7bcb8c94ccb

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark