Skip to contentSkip to conversation

ASoC: mediatek: mt6797: fix wrong unwind order and error code in enable_clock

PATCH Superseded [email protected] 2 participants 7 messages Aug 19 – Aug 20

Subsystems (3)
Sound ASoC Arm MediaTek

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH v2 1/3 ASoC: mediatek: mt6797: fix wrong unwind order and error code in enable_clock

From: bui duc phuc <[email protected]>

The error paths in mt6797_afe_enable_clock() use incorrect goto labels, causing clocks that failed to enable to be disabled during cleanup.

Fix the goto labels to only unwind clocks that were successfully enabled, and return the actual error code.

Signed-off-by bui duc phuc <[email protected]>
---
 sound/soc/mediatek/mt6797/mt6797-afe-clk.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
sound/soc/mediatek/mt6797/mt6797-afe-clk.c +6 -8
index 6f3e6acfcfab..0ac04d73b777 100644
@@ -63,21 +63,21 @@ int mt6797_afe_enable_clock(struct mtk_base_afe *afe)
 	if (ret) {
 		dev_err(afe->dev, "%s(), clk_prepare_enable %s fail %d\n",
 			__func__, aud_clks[CLK_INFRA_SYS_AUD], ret);
-		goto CLK_INFRA_SYS_AUDIO_ERR;
+		return ret;
 	}
 
 	ret = clk_prepare_enable(afe_priv->clk[CLK_INFRA_SYS_AUD_26M]);
 	if (ret) {
 		dev_err(afe->dev, "%s(), clk_prepare_enable %s fail %d\n",
 			__func__, aud_clks[CLK_INFRA_SYS_AUD_26M], ret);
-		goto CLK_INFRA_SYS_AUD_26M_ERR;
+		goto CLK_INFRA_SYS_AUDIO_ERR;
 	}
 
 	ret = clk_prepare_enable(afe_priv->clk[CLK_TOP_MUX_AUD]);
 	if (ret) {
 		dev_err(afe->dev, "%s(), clk_prepare_enable %s fail %d\n",
 			__func__, aud_clks[CLK_TOP_MUX_AUD], ret);
-		goto CLK_MUX_AUDIO_ERR;
+		goto CLK_INFRA_SYS_AUD_26M_ERR;
 	}
 
 	ret = clk_set_parent(afe_priv->clk[CLK_TOP_MUX_AUD],

@@ -93,13 +93,11 @@ int mt6797_afe_enable_clock(struct mtk_base_afe *afe)
 	if (ret) {
 		dev_err(afe->dev, "%s(), clk_prepare_enable %s fail %d\n",
 			__func__, aud_clks[CLK_TOP_MUX_AUD_BUS], ret);
-		goto CLK_MUX_AUDIO_INTBUS_ERR;
+		goto CLK_MUX_AUDIO_ERR;
 	}
 
-	return ret;
+	return 0;
 
-CLK_MUX_AUDIO_INTBUS_ERR:
-	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_BUS]);
 CLK_MUX_AUDIO_ERR:
 	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD]);
 CLK_INFRA_SYS_AUD_26M_ERR:

@@ -107,7 +105,7 @@ int mt6797_afe_enable_clock(struct mtk_base_afe *afe)
 CLK_INFRA_SYS_AUDIO_ERR:
 	clk_disable_unprepare(afe_priv->clk[CLK_INFRA_SYS_AUD]);
 
-	return 0;
+	return ret;
 }
 
 int mt6797_afe_disable_clock(struct mtk_base_afe *afe)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v2 2/3 ASoC: mediatek: mt6797: Use dev_err_probe() for error handling

From: bui duc phuc <[email protected]>

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

Signed-off-by bui duc phuc <[email protected]>
---
 sound/soc/mediatek/mt6797/mt6797-afe-clk.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
sound/soc/mediatek/mt6797/mt6797-afe-clk.c +3 -6
index 0ac04d73b777..2326496f8b27 100644
@@ -43,12 +43,9 @@ int mt6797_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
author
#3 lore raw

PATCH v2 3/3 ASoC: mediatek: mt6797: 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.

Signed-off-by bui duc phuc <[email protected]>
---
 sound/soc/mediatek/mt6797/mt6797-afe-pcm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
sound/soc/mediatek/mt6797/mt6797-afe-pcm.c +2 -6
index f62a32f2f2b6..dfe67727fe47 100644
@@ -754,10 +754,8 @@ static int mt6797_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	/* initial audio related clock */
 	ret = mt6797_init_clock(afe);
-	if (ret) {
-		dev_err(dev, "init clock error\n");
+	if (ret)
 		return ret;
-	}
 
 	/* regmap init */
 	afe->base_addr = devm_platform_ioremap_resource(pdev, 0);

@@ -800,10 +798,8 @@ static int mt6797_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq_id, mt6797_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)
 		return ret;
-	}
 
 	/* init sub_dais */
 	INIT_LIST_HEAD(&afe->sub_dais);
Signature · 1 line
2.43.0
Reviewed-by on v2 3/3
#4 lore raw

Re: [PATCH v2 3/3] ASoC: mediatek: mt6797: Drop redundant probe error messages

Quoted 7 lines · On 8/19/2026 12:17 PM, [email protected] wrote:

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.

Signed-off-by bui duc phuc <[email protected]>
Reviewed-by Cezary Rojewski <[email protected]>
Reviewed-by on v2 1/3
#5 lore raw
In reply to [email protected]
Quoted 10 lines · On 8/19/2026 12:17 PM, [email protected] wrote:

From: bui duc phuc <[email protected]>

The error paths in mt6797_afe_enable_clock() use incorrect goto labels, causing clocks that failed to enable to be disabled during cleanup.

Fix the goto labels to only unwind clocks that were successfully enabled, and return the actual error code.

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

---

There is no changelog in this series so reviewers have harder job analyzing the v2 patches. If there is no cover-letter, you can always paste the update here, after '---'.

Quoted 2 lines
  sound/soc/mediatek/mt6797/mt6797-afe-clk.c | 14 ++++++--------
  1 file changed, 6 insertions(+), 8 deletions(-)

...

Quoted 17 lines
sound/soc/mediatek/mt6797/mt6797-afe-clk.c +2 -4
@@ -93,13 +93,11 @@ int mt6797_afe_enable_clock(struct mtk_base_afe *afe)
  	if (ret) {
  		dev_err(afe->dev, "%s(), clk_prepare_enable %s fail %d\n",
  			__func__, aud_clks[CLK_TOP_MUX_AUD_BUS], ret);
-		goto CLK_MUX_AUDIO_INTBUS_ERR;
+		goto CLK_MUX_AUDIO_ERR;
  	}
 
-	return ret;
+	return 0;
 
-CLK_MUX_AUDIO_INTBUS_ERR:
-	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_BUS]);
  CLK_MUX_AUDIO_ERR:
  	clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD]);
  CLK_INFRA_SYS_AUD_26M_ERR:

sound/soc/mediatek/mt6797/mt6797-afe-clk.c int mt6797_afe_enable_clock(struct mtk_base_afe *afe) · line 109

Quoted 6 lines
sound/soc/mediatek/mt6797/mt6797-afe-clk.c +1 -1
@@ -107,7 +105,7 @@ int mt6797_afe_enable_clock(struct mtk_base_afe *afe)
  CLK_INFRA_SYS_AUDIO_ERR:
  	clk_disable_unprepare(afe_priv->clk[CLK_INFRA_SYS_AUD]);
 
-	return 0;
+	return ret;
  }

In regard to the patch, the fix looks good - given the number of errors in the existing code with invalid return code on top, perhaps someone wanted the function to be permissive.

Otherwise it's just bunch of untested stuff and your change should be tagged with: Fixes: and propagated downstream.

Reviewed-by Cezary Rojewski <[email protected]>
author
#6 lore raw

Hi Cezary,

Thank you for your review.

Quoted 6 lines

There is no changelog in this series so reviewers have harder job analyzing the v2 patches. If there is no cover-letter, you can always paste the update here, after '---'.

Quoted 8 lines

In regard to the patch, the fix looks good - given the number of errors in the existing code with invalid return code on top, perhaps someone wanted the function to be permissive.

Otherwise it's just bunch of untested stuff and your change should be tagged with: Fixes: and propagated downstream.

Reviewed-by Cezary Rojewski <[email protected]>

I’ll add the Fixes: tag and include a changelog
for the changes in the next revision.

Best regards,
Phuc