Skip to contentSkip to conversation

ASoC: mediatek: mt8189: Improve error handling

PATCH In review [email protected] 3 participants 15 messages Sep 16 – Sep 18

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v3 cover ASoC: mediatek: mt8189: Improve error handling

From: bui duc phuc <[email protected]>

Hi all,

This series addresses part of the error handling in the MT8189 AFE driver.

It propagates errors from clock operations and runtime resume, validates clock parameters, and removes redundant error messages.

In v2, there was discussion about checking the return value of regmap_update_bits(). While opinions differed, the existing driver already checks it in mt8189_afe_enable_top_cg(), which is used by mt8189_apll1_enable() and mt8189_apll2_enable() touched by this series.

Therefore, I keep these checks and add the corresponding error paths for consistency.

Compile-tested only.

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

Changes in v2:
- Update the names of the goto labels.
- Check for zero frequency since freq is unsigned.
Changes in v3:
- Add Angelo's Reviewed-by tag.
- Restore the existing afe->regmap checks,
making the separate patch for missing regmap unnecessary.
- Do not pre-initialize ret; set ret = 0 in the default case throughout the series.

Best regards,
Phuc

bui duc phuc (12):
ASoC: mediatek: mt8189: Propagate APLL enable errors
ASoC: mediatek: mt8189: Propagate MCK enable errors
ASoC: mediatek: mt8189: Validate MCK ID
ASoC: mediatek: mt8189: Propagate reg_rw clock errors
ASoC: mediatek: mt8189: Use dev_err_probe() for clock errors
ASoC: mediatek: mt8189: Propagate runtime resume errors
ASoC: mediatek: mt8189: Remove redundant error message
ASoC: mediatek: mt8189: Propagate APLL errors
ASoC: mediatek: mt8189: Propagate MCLK errors
ASoC: mediatek: mt8189: Validate sysclk frequency
ASoC: mediatek: mt8189: Propagate TDM clock errors
ASoC: mediatek: mt8189: Validate TDM MCLK frequency

 sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 134 +++++++++++++++------
 sound/soc/mediatek/mt8189/mt8189-afe-pcm.c |  45 ++++---
 sound/soc/mediatek/mt8189/mt8189-dai-i2s.c |  18 ++-
 sound/soc/mediatek/mt8189/mt8189-dai-tdm.c |  19 ++-
 4 files changed, 154 insertions(+), 62 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v3 1/12 ASoC: mediatek: mt8189: Propagate APLL enable errors

From: bui duc phuc <[email protected]>

mt8189_apll1_enable() and mt8189_apll2_enable() currently ignore errors from regmap_update_bits() and do not clean up resources when clock enable operations fail.

Propagate these errors and roll back the clocks and tuner state on errors.

Fixes dc637ffeed6c ("ASoC: mediatek: mt8189: support audio clock control")

---

Changes in v2:
- Update the names of the goto labels.

 sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 78 ++++++++++++++++------
 1 file changed, 56 insertions(+), 22 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +56 -22
index fc7a7a73b0cf..f088ccf80448 100644
@@ -454,30 +454,47 @@ int mt8189_apll1_enable(struct mtk_base_afe *afe)
 
 	ret = mt8189_afe_enable_top_cg(afe, MT8189_CG_APLL1_CK);
 	if (ret)
-		return ret;
+		goto err_clear_mux_setting;
 
 	ret = mt8189_afe_enable_top_cg(afe, MT8189_PDN_APLL_TUNER1);
 	if (ret)
-		return ret;
+		goto err_disable_apll1_ck;
 
 	/* sel 44.1kHz:1, apll_div:7, upper bound:3 */
-	regmap_update_bits(afe->regmap, AFE_APLL1_TUNER_CFG,
-			   XTAL_EN_128FS_SEL_MASK_SFT | APLL_DIV_MASK_SFT |
-			   UPPER_BOUND_MASK_SFT,
-			   (0x1 << XTAL_EN_128FS_SEL_SFT) | (7 << APLL_DIV_SFT) |
-			   (3 << UPPER_BOUND_SFT));
+	ret = regmap_update_bits(afe->regmap, AFE_APLL1_TUNER_CFG,
+				 XTAL_EN_128FS_SEL_MASK_SFT | APLL_DIV_MASK_SFT |
+				 UPPER_BOUND_MASK_SFT,
+				 (0x1 << XTAL_EN_128FS_SEL_SFT) | (7 << APLL_DIV_SFT) |
+				 (3 << UPPER_BOUND_SFT));
+	if (ret)
+		goto err_disable_apll_tuner1;
 
 	/* apll1 freq tuner enable */
-	regmap_update_bits(afe->regmap, AFE_APLL1_TUNER_CFG,
-			   FREQ_TUNER_EN_MASK_SFT,
-			   0x1 << FREQ_TUNER_EN_SFT);
+	ret = regmap_update_bits(afe->regmap, AFE_APLL1_TUNER_CFG,
+				 FREQ_TUNER_EN_MASK_SFT,
+				 0x1 << FREQ_TUNER_EN_SFT);
+	if (ret)
+		goto err_disable_apll_tuner1;
 
 	/* audio apll1 on */
 	ret = mt8189_afe_enable_top_cg(afe, MT8189_AUDIO_APLL1_EN_ON);
 	if (ret)
-		return ret;
+		goto err_clear_freq_tuner_en;
 
 	return 0;
+
+err_clear_freq_tuner_en:
+	regmap_update_bits(afe->regmap, AFE_APLL1_TUNER_CFG,
+			   FREQ_TUNER_EN_MASK_SFT,
+			   0x0);
+err_disable_apll_tuner1:
+	mt8189_afe_disable_top_cg(afe, MT8189_PDN_APLL_TUNER1);
+err_disable_apll1_ck:
+	mt8189_afe_disable_top_cg(afe, MT8189_CG_APLL1_CK);
+err_clear_mux_setting:
+	apll1_mux_setting(afe, false);
+
+	return ret;
 }
 
 void mt8189_apll1_disable(struct mtk_base_afe *afe)
@@ -506,30 +523,47 @@ int mt8189_apll2_enable(struct mtk_base_afe *afe)
 
 	ret = mt8189_afe_enable_top_cg(afe, MT8189_CG_APLL2_CK);
 	if (ret)
-		return ret;
+		goto err_clear_mux_setting;
 
 	ret = mt8189_afe_enable_top_cg(afe, MT8189_PDN_APLL_TUNER2);
 	if (ret)
-		return ret;
+		goto err_disable_apll2_ck;
 
 	/* sel 48kHz: 2, apll_div: 7, upper bound: 3*/
-	regmap_update_bits(afe->regmap, AFE_APLL2_TUNER_CFG,
-			   XTAL_EN_128FS_SEL_MASK_SFT | APLL_DIV_MASK_SFT |
-			   UPPER_BOUND_MASK_SFT,
-			   (0x2 << XTAL_EN_128FS_SEL_SFT) | (7 << APLL_DIV_SFT) |
-			   (3 << UPPER_BOUND_SFT));
+	ret = regmap_update_bits(afe->regmap, AFE_APLL2_TUNER_CFG,
+				 XTAL_EN_128FS_SEL_MASK_SFT | APLL_DIV_MASK_SFT |
+				 UPPER_BOUND_MASK_SFT,
+				 (0x2 << XTAL_EN_128FS_SEL_SFT) | (7 << APLL_DIV_SFT) |
+				 (3 << UPPER_BOUND_SFT));
+	if (ret)
+		goto err_disable_apll_tuner2;
 
 	/* apll2 freq tuner enable */
-	regmap_update_bits(afe->regmap, AFE_APLL2_TUNER_CFG,
-			   FREQ_TUNER_EN_MASK_SFT,
-			   0x1 << FREQ_TUNER_EN_SFT);
+	ret = regmap_update_bits(afe->regmap, AFE_APLL2_TUNER_CFG,
+				 FREQ_TUNER_EN_MASK_SFT,
+				 0x1 << FREQ_TUNER_EN_SFT);
+	if (ret)
+		goto err_disable_apll_tuner2;
 
 	/* audio apll2 on */
 	ret = mt8189_afe_enable_top_cg(afe, MT8189_AUDIO_APLL2_EN_ON);
 	if (ret)
-		return ret;
+		goto err_clear_freq_tuner_en;
 
 	return 0;
+
+err_clear_freq_tuner_en:
+	regmap_update_bits(afe->regmap, AFE_APLL2_TUNER_CFG,
+			   FREQ_TUNER_EN_MASK_SFT,
+			   0x0);
+err_disable_apll_tuner2:
+	mt8189_afe_disable_top_cg(afe, MT8189_PDN_APLL_TUNER2);
+err_disable_apll2_ck:
+	mt8189_afe_disable_top_cg(afe, MT8189_CG_APLL2_CK);
+err_clear_mux_setting:
+	apll2_mux_setting(afe, false);
+
+	return ret;
 }
 
 void mt8189_apll2_disable(struct mtk_base_afe *afe)
Signature · 1 line
2.43.0
#2 lore raw

PATCH v3 2/12 ASoC: mediatek: mt8189: Propagate MCK enable errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

mt8189_mck_enable() currently returns without restoring the clock state when setting the clock parent, enabling the divider, or setting the divider rate fails.

Propagate the error and disable clocks enabled by the function.

Fixes dc637ffeed6c ("ASoC: mediatek: mt8189: support audio clock control")

---

Changes in v2:
- Update the names of the goto labels.

 sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +11 -3
index f088ccf80448..56362c4756c9 100644
@@ -639,7 +639,7 @@ int mt8189_mck_enable(struct mtk_base_afe *afe, int mck_id, int rate)
 		ret = mt8189_afe_set_clk_parent(afe, afe_priv->clk[m_sel_id],
 						afe_priv->clk[apll_clk_id]);
 		if (ret)
-			return ret;
+			goto err_disable_m_sel_clk;
 	}
 
 	/* enable div, set rate */
@@ -650,13 +650,21 @@ int mt8189_mck_enable(struct mtk_base_afe *afe, int mck_id, int rate)
 
 	ret = mt8189_afe_enable_clk(afe, afe_priv->clk[div_clk_id]);
 	if (ret)
-		return ret;
+		goto err_disable_m_sel_clk;
 
 	ret = mt8189_afe_set_clk_rate(afe, afe_priv->clk[div_clk_id], rate);
 	if (ret)
-		return ret;
+		goto err_disable_div_clk;
 
 	return 0;
+
+err_disable_div_clk:
+	mt8189_afe_disable_clk(afe, afe_priv->clk[div_clk_id]);
+err_disable_m_sel_clk:
+	if (m_sel_id >= 0)
+		mt8189_afe_disable_clk(afe, afe_priv->clk[m_sel_id]);
+
+	return ret;
 }
 
 int mt8189_mck_disable(struct mtk_base_afe *afe, int mck_id)
Signature · 1 line
2.43.0
#3 lore raw

PATCH v3 3/12 ASoC: mediatek: mt8189: Validate MCK ID

In reply to [email protected]

From: bui duc phuc <[email protected]>

mt8189_mck_disable() only checks for negative MCK IDs. Reject IDs outside the valid MCK range as well.

Fixes dc637ffeed6c ("ASoC: mediatek: mt8189: support audio clock control")

---

Changes in v3:
- Add Angelo's Reviewed-by tag.

 sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +2 -2
index 56362c4756c9..7fe9d01006d5 100644
@@ -675,8 +675,8 @@ int mt8189_mck_disable(struct mtk_base_afe *afe, int mck_id)
 
 	dev_dbg(afe->dev, "mck_id: %d.\n", mck_id);
 
-	if (mck_id < 0) {
-		dev_err(afe->dev, "mck_id = %d < 0\n", mck_id);
+	if (mck_id >= MT8189_MCK_NUM || mck_id < 0) {
+		dev_err(afe->dev, "mck_id = %d\n", mck_id);
 		return -EINVAL;
 	}
 
Signature · 1 line
2.43.0
#4 lore raw

PATCH v3 4/12 ASoC: mediatek: mt8189: Propagate reg_rw clock errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

mt8189_afe_enable_reg_rw_clk() currently ignores errors from clock enable and parent operations.

Propagate these errors and clean up the clocks before returning the error.

Fixes dc637ffeed6c ("ASoC: mediatek: mt8189: support audio clock control")

---

Changes in v2:
- Update the names of the goto labels.

 sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 31 +++++++++++++++++-----
 1 file changed, 25 insertions(+), 6 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +25 -6
index 7fe9d01006d5..1132c924158d 100644
@@ -700,17 +700,36 @@ int mt8189_mck_disable(struct mtk_base_afe *afe, int mck_id)
 int mt8189_afe_enable_reg_rw_clk(struct mtk_base_afe *afe)
 {
 	struct mt8189_afe_private *afe_priv = afe->platform_priv;
+	int ret;
 
 	/* bus clock for AFE internal access, like AFE SRAM */
-	mt8189_afe_enable_clk(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIOINTBUS]);
-	mt8189_afe_set_clk_parent(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIOINTBUS],
-				  afe_priv->clk[MT8189_CLK_TOP_CLK26M]);
+	ret = mt8189_afe_enable_clk(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIOINTBUS]);
+	if (ret)
+		return ret;
+
+	ret = mt8189_afe_set_clk_parent(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIOINTBUS],
+					afe_priv->clk[MT8189_CLK_TOP_CLK26M]);
+	if (ret)
+		goto err_disable_audiointbus_clk;
+
 	/* enable audio clock source */
-	mt8189_afe_enable_clk(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIO_H]);
-	mt8189_afe_set_clk_parent(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIO_H],
-				  afe_priv->clk[MT8189_CLK_TOP_CLK26M]);
+	ret = mt8189_afe_enable_clk(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIO_H]);
+	if (ret)
+		goto err_disable_audiointbus_clk;
+
+	ret = mt8189_afe_set_clk_parent(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIO_H],
+					afe_priv->clk[MT8189_CLK_TOP_CLK26M]);
+	if (ret)
+		goto err_disable_audio_h_clk;
 
 	return 0;
+
+err_disable_audio_h_clk:
+	mt8189_afe_disable_clk(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIO_H]);
+err_disable_audiointbus_clk:
+	mt8189_afe_disable_clk(afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIOINTBUS]);
+
+	return ret;
 }
 
 int mt8189_afe_disable_reg_rw_clk(struct mtk_base_afe *afe)
Signature · 1 line
2.43.0
#5 lore raw

PATCH v3 5/12 ASoC: mediatek: mt8189: Use dev_err_probe() for clock errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

Use dev_err_probe() when obtaining clocks to avoid redundant error messages, particularly for probe deferral.

---

Changes in v3:
- Add Angelo's Reviewed-by tag.

 sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +3 -4
index 1132c924158d..594c214fa697 100644
@@ -793,10 +793,9 @@ int mt8189_init_clock(struct mtk_base_afe *afe)
 
 	for (i = 0; i < MT8189_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, "devm_clk_get %s fail\n", aud_clks[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]);
 	}
 
 	ret = mt8189_afe_disable_apll(afe);
Signature · 1 line
2.43.0
#6 lore raw

PATCH v3 6/12 ASoC: mediatek: mt8189: Propagate runtime resume errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

mt8189_afe_runtime_resume() currently ignores errors from regmap operations and mt8189_afe_enable_main_clock().

Propagate these errors and clean up the state before returning the error.

Fixes 7eb153585598 ("ASoC: mediatek: mt8189: add platform driver")

---

Changes in v2:
- Update the names of the goto labels.
Changes in v3:
- Add Angelo's Reviewed-by tag.
- Restore the existing afe->regmap checks.

 sound/soc/mediatek/mt8189/mt8189-afe-pcm.c | 32 ++++++++++++++++++----
 1 file changed, 26 insertions(+), 6 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-pcm.c +26 -6
index 77cf2b604f6c..39cc2804ad4c 100644
@@ -2332,20 +2332,40 @@ static int mt8189_afe_runtime_resume(struct device *dev)
 	}
 
 	regcache_cache_only(afe->regmap, false);
-	regcache_sync(afe->regmap);
+	ret = regcache_sync(afe->regmap);
+	if (ret)
+		goto err_set_cache_only;
 
 	/* set audio 26M request */
-	regmap_update_bits(afe->regmap, AFE_SPM_CONTROL_REQ, 0x1, 0x1);
-	regmap_update_bits(afe->regmap, AFE_CBIP_CFG0, 0x1, 0x1);
+	ret = regmap_update_bits(afe->regmap, AFE_SPM_CONTROL_REQ, 0x1, 0x1);
+	if (ret)
+		goto err_set_cache_only;
+
+	ret = regmap_update_bits(afe->regmap, AFE_CBIP_CFG0, 0x1, 0x1);
+	if (ret)
+		goto err_clear_26m_req;
 
 	/* force cpu use 8_24 format when writing 32bit data */
-	regmap_update_bits(afe->regmap, AFE_MEMIF_CON0,
-			   CPU_HD_ALIGN_MASK_SFT, 0 << CPU_HD_ALIGN_SFT);
+	ret = regmap_update_bits(afe->regmap, AFE_MEMIF_CON0,
+				 CPU_HD_ALIGN_MASK_SFT, 0 << CPU_HD_ALIGN_SFT);
+	if (ret)
+		goto err_clear_26m_req;
 
 	/* enable AFE */
-	mt8189_afe_enable_main_clock(afe);
+	ret = mt8189_afe_enable_main_clock(afe);
+	if (ret)
+		goto err_clear_26m_req;
 
 	return 0;
+
+err_clear_26m_req:
+	regmap_update_bits(afe->regmap,
+			   AFE_SPM_CONTROL_REQ, 0x1, 0x0);
+err_set_cache_only:
+	regcache_cache_only(afe->regmap, true);
+	mt8189_afe_disable_reg_rw_clk(afe);
+
+	return ret;
 }
 
 static int mt8189_afe_component_probe(struct snd_soc_component *component)
Signature · 1 line
2.43.0
#7 lore raw

PATCH v3 7/12 ASoC: mediatek: mt8189: Remove redundant error message

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() and dev_err_probe() call is redundant and can be removed.

---

Changes in v3:
- Add Angelo's Reviewed-by tag.

 sound/soc/mediatek/mt8189/mt8189-afe-pcm.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-pcm.c +5 -8
index 39cc2804ad4c..11dba86c3e81 100644
@@ -2486,13 +2486,12 @@ static int mt8189_afe_pcm_dev_probe(struct platform_device *pdev)
 
 	afe->base_addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(afe->base_addr))
-		return dev_err_probe(dev, PTR_ERR(afe->base_addr),
-				     "AFE base_addr not found\n");
+		return PTR_ERR(afe->base_addr);
 
 	/* init audio related clock */
 	ret = mt8189_init_clock(afe);
 	if (ret)
-		return dev_err_probe(dev, ret, "init clock error.\n");
+		return ret;
 
 	/* init memif */
 	/* IPM2.0 no need banding */
@@ -2526,13 +2525,13 @@ static int mt8189_afe_pcm_dev_probe(struct platform_device *pdev)
 	/* request irq */
 	irq_id = platform_get_irq(pdev, 0);
 	if (irq_id < 0)
-		return dev_err_probe(dev, irq_id, "no irq found");
+		return irq_id;
 
 	ret = devm_request_irq(dev, irq_id, mt8189_afe_irq_handler,
 			       IRQF_TRIGGER_NONE,
 			       "Afe_ISR_Handle", afe);
 	if (ret)
-		return dev_err_probe(dev, ret, "could not request_irq for Afe_ISR_Handle\n");
+		return ret;
 
 	/* init sub_dais */
 	INIT_LIST_HEAD(&afe->sub_dais);
@@ -2601,10 +2600,8 @@ static int mt8189_afe_pcm_dev_probe(struct platform_device *pdev)
 					      &mt8189_afe_component,
 					      afe->dai_drivers,
 					      afe->num_dai_drivers);
-	if (ret) {
-		dev_err(dev, "afe component err: %d\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	return 0;
 
Signature · 1 line
2.43.0
#8 lore raw

PATCH v3 8/12 ASoC: mediatek: mt8189: Propagate APLL errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

mtk_apll_event() currently ignores errors returned by the APLL enable functions.

Propagate these errors.

Fixes 7eb153585598 ("ASoC: mediatek: mt8189: add platform driver")
---
 sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
sound/soc/mediatek/mt8189/mt8189-dai-i2s.c +5 -2
index 94c49a662e2d..4826ee250d70 100644
@@ -485,6 +485,7 @@ static int mtk_apll_event(struct snd_soc_dapm_widget *w,
 {
 	struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
+	int ret;
 
 	dev_dbg(cmpnt->dev, "%s(), name %s, event 0x%x\n",
 		__func__, w->name, event);
@@ -492,9 +493,11 @@ static int mtk_apll_event(struct snd_soc_dapm_widget *w,
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
 		if (strcmp(w->name, APLL1_W_NAME) == 0)
-			mt8189_apll1_enable(afe);
+			ret = mt8189_apll1_enable(afe);
 		else
-			mt8189_apll2_enable(afe);
+			ret = mt8189_apll2_enable(afe);
+		if (ret)
+			return ret;
 		break;
 	case SND_SOC_DAPM_POST_PMD:
 		if (strcmp(w->name, APLL1_W_NAME) == 0)
Signature · 1 line
2.43.0
#9 lore raw

PATCH v3 9/12 ASoC: mediatek: mt8189: Propagate MCLK errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

mtk_mclk_en_event() currently ignores errors returned by the MCK enable and disable functions.

Propagate these errors.

Fixes 7eb153585598 ("ASoC: mediatek: mt8189: add platform driver")

---

Changes in v3:
- Add Angelo's Reviewed-by tag.
- Do not pre-initialize ret; set ret = 0 in the default case.

 sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
sound/soc/mediatek/mt8189/mt8189-dai-i2s.c +5 -3
index 4826ee250d70..8cd5ddf62d46 100644
@@ -519,6 +519,7 @@ static int mtk_mclk_en_event(struct snd_soc_dapm_widget *w,
 	struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
 	struct mtk_afe_i2s_priv *i2s_priv;
+	int ret;
 
 	dev_dbg(cmpnt->dev, "%s(), name %s, event 0x%x\n",
 		__func__, w->name, event);
@@ -529,17 +530,18 @@ static int mtk_mclk_en_event(struct snd_soc_dapm_widget *w,
 
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
-		mt8189_mck_enable(afe, i2s_priv->mclk_id, i2s_priv->mclk_rate);
+		ret = mt8189_mck_enable(afe, i2s_priv->mclk_id, i2s_priv->mclk_rate);
 		break;
 	case SND_SOC_DAPM_POST_PMD:
 		i2s_priv->mclk_rate = 0;
-		mt8189_mck_disable(afe, i2s_priv->mclk_id);
+		ret = mt8189_mck_disable(afe, i2s_priv->mclk_id);
 		break;
 	default:
+		ret = 0;
 		break;
 	}
 
-	return 0;
+	return ret;
 }
 
 static const struct snd_soc_dapm_widget mtk_dai_i2s_widgets[] = {
Signature · 1 line
2.43.0
#10 lore raw

PATCH v3 10/12 ASoC: mediatek: mt8189: Validate sysclk frequency

In reply to [email protected]

From: bui duc phuc <[email protected]>

A zero frequency causes a modulo-by-zero error when validating the I2S clock frequency. Reject it before performing the validation.

Fixes 7eb153585598 ("ASoC: mediatek: mt8189: add platform driver")

---

Changes in v2:
- Check for zero frequency since freq is unsigned.
Changes in v3:
- Add Angelo's Reviewed-by tag.

 sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 3 +++
 1 file changed, 3 insertions(+)
sound/soc/mediatek/mt8189/mt8189-dai-i2s.c +3 -0
index 8cd5ddf62d46..c9e80ef42f60 100644
@@ -1287,6 +1287,9 @@ static int mtk_dai_i2s_set_sysclk(struct snd_soc_dai *dai,
 	    dir != SND_SOC_CLOCK_OUT)
 		return -EINVAL;
 
+	if (!freq)
+		return -EINVAL;
+
 	i2s_priv = afe_priv->dai_priv[dai->id];
 	if (!i2s_priv)
 		return -EINVAL;
Signature · 1 line
2.43.0
#11 lore raw

PATCH v3 11/12 ASoC: mediatek: mt8189: Propagate TDM clock errors

In reply to [email protected]

From: bui duc phuc <[email protected]>

mtk_tdm_bck_en_event() and mtk_tdm_mck_en_event() currently ignore errors returned by the MCK enable and disable functions.

Propagate these errors.

Fixes 9f202872ba04 ("ASoC: mediatek: mt8189: support TDM in platform driver")

---

Changes in v3:
- Do not pre-initialize ret; set ret = 0 in the default case.

 sound/soc/mediatek/mt8189/mt8189-dai-tdm.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
sound/soc/mediatek/mt8189/mt8189-dai-tdm.c +10 -6
index 5d68a55ccc45..9aebd0320614 100644
@@ -261,22 +261,24 @@ static int mtk_tdm_bck_en_event(struct snd_soc_dapm_widget *w,
 	struct mt8189_afe_private *afe_priv = afe->platform_priv;
 	int dai_id = get_tdm_id_by_name(w->name);
 	struct mtk_afe_tdm_priv *tdm_priv = afe_priv->dai_priv[dai_id];
+	int ret;
 
 	dev_dbg(cmpnt->dev, "name %s, event 0x%x, dai_id %d, bck: %d\n",
 		w->name, event, dai_id, tdm_priv->bck_rate);
 
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
-		mt8189_mck_enable(afe, tdm_priv->bck_id, tdm_priv->bck_rate);
+		ret = mt8189_mck_enable(afe, tdm_priv->bck_id, tdm_priv->bck_rate);
 		break;
 	case SND_SOC_DAPM_POST_PMD:
-		mt8189_mck_disable(afe, tdm_priv->bck_id);
+		ret = mt8189_mck_disable(afe, tdm_priv->bck_id);
 		break;
 	default:
+		ret = 0;
 		break;
 	}
 
-	return 0;
+	return ret;
 }
 
 static int mtk_tdm_mck_en_event(struct snd_soc_dapm_widget *w,
@@ -288,23 +290,25 @@ static int mtk_tdm_mck_en_event(struct snd_soc_dapm_widget *w,
 	struct mt8189_afe_private *afe_priv = afe->platform_priv;
 	int dai_id = get_tdm_id_by_name(w->name);
 	struct mtk_afe_tdm_priv *tdm_priv = afe_priv->dai_priv[dai_id];
+	int ret;
 
 	dev_dbg(cmpnt->dev, "name %s, event 0x%x, dai_id %d, mclk %d\n",
 		w->name, event, dai_id, tdm_priv->mclk_rate);
 
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
-		mt8189_mck_enable(afe, tdm_priv->mclk_id, tdm_priv->mclk_rate);
+		ret = mt8189_mck_enable(afe, tdm_priv->mclk_id, tdm_priv->mclk_rate);
 		break;
 	case SND_SOC_DAPM_POST_PMD:
 		tdm_priv->mclk_rate = 0;
-		mt8189_mck_disable(afe, tdm_priv->mclk_id);
+		ret = mt8189_mck_disable(afe, tdm_priv->mclk_id);
 		break;
 	default:
+		ret = 0;
 		break;
 	}
 
-	return 0;
+	return ret;
 }
 
 static const struct snd_soc_dapm_widget mtk_dai_tdm_widgets[] = {
Signature · 1 line
2.43.0
#12 lore raw

PATCH v3 12/12 ASoC: mediatek: mt8189: Validate TDM MCLK frequency

In reply to [email protected]

From: bui duc phuc <[email protected]>

mtk_dai_tdm_cal_mclk() accepts zero or negative frequencies. Reject invalid frequencies before configuring the TDM clock.

Fixes 9f202872ba04 ("ASoC: mediatek: mt8189: support TDM in platform driver")

---

Changes in v3:
- Add Angelo's Reviewed-by tag.

 sound/soc/mediatek/mt8189/mt8189-dai-tdm.c | 3 +++
 1 file changed, 3 insertions(+)
sound/soc/mediatek/mt8189/mt8189-dai-tdm.c +3 -0
index 9aebd0320614..e113c52fe06a 100644
@@ -383,6 +383,9 @@ static int mtk_dai_tdm_cal_mclk(struct mtk_base_afe *afe,
 	int apll;
 	int apll_rate;
 
+	if (freq <= 0)
+		return -EINVAL;
+
 	apll = mt8189_get_apll_by_rate(afe, freq);
 	apll_rate = mt8189_get_apll_rate(afe, apll);
 
Signature · 1 line
2.43.0
maintainer Reviewed-by on the v3 series
#13 lore raw
In reply to [email protected]
Quoted 14 lines · On 9/16/26 07:00, [email protected] wrote:

From: bui duc phuc <[email protected]>

Hi all,

This series addresses part of the error handling in the MT8189 AFE driver.

It propagates errors from clock operations and runtime resume, validates clock parameters, and removes redundant error messages.

In v2, there was discussion about checking the return value of regmap_update_bits(). While opinions differed, the existing driver already checks it in mt8189_afe_enable_top_cg(), which is used by mt8189_apll1_enable() and mt8189_apll2_enable() touched by this series.

Quoted 4 lines

Therefore, I keep these checks and add the corresponding error paths for consistency.

Compile-tested only.

Series is

Cheers,
Angelo

Quoted 39 lines

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

Changes in v2:
- Update the names of the goto labels.
- Check for zero frequency since freq is unsigned.
Changes in v3:
- Add Angelo's Reviewed-by tag.
- Restore the existing afe->regmap checks,
making the separate patch for missing regmap unnecessary.
- Do not pre-initialize ret; set ret = 0 in the default case throughout the series.

Best regards,
Phuc

bui duc phuc (12):
ASoC: mediatek: mt8189: Propagate APLL enable errors
ASoC: mediatek: mt8189: Propagate MCK enable errors
ASoC: mediatek: mt8189: Validate MCK ID
ASoC: mediatek: mt8189: Propagate reg_rw clock errors
ASoC: mediatek: mt8189: Use dev_err_probe() for clock errors
ASoC: mediatek: mt8189: Propagate runtime resume errors
ASoC: mediatek: mt8189: Remove redundant error message
ASoC: mediatek: mt8189: Propagate APLL errors
ASoC: mediatek: mt8189: Propagate MCLK errors
ASoC: mediatek: mt8189: Validate sysclk frequency
ASoC: mediatek: mt8189: Propagate TDM clock errors
ASoC: mediatek: mt8189: Validate TDM MCLK frequency

  sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 134 +++++++++++++++------
  sound/soc/mediatek/mt8189/mt8189-afe-pcm.c |  45 ++++---
  sound/soc/mediatek/mt8189/mt8189-dai-i2s.c |  18 ++-
  sound/soc/mediatek/mt8189/mt8189-dai-tdm.c |  19 ++-
  4 files changed, 154 insertions(+), 62 deletions(-)
maintainer
#14 lore raw
In reply to [email protected]
Quoted 9 lines · On Wed, 16 Sep 2026 12:00:07 +0700, [email protected] wrote:

ASoC: mediatek: mt8189: Improve error handling

From: bui duc phuc <[email protected]>

Hi all,

This series addresses part of the error handling in the MT8189 AFE driver.

[...]

Applied to

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

Thanks!

[01/12] ASoC: mediatek: mt8189: Propagate APLL enable errors

        https://git.kernel.org/broonie/sound/c/5a3c23d4aa89
[02/12] ASoC: mediatek: mt8189: Propagate MCK enable errors
        https://git.kernel.org/broonie/sound/c/bd9a8d9def20
[03/12] ASoC: mediatek: mt8189: Validate MCK ID
        https://git.kernel.org/broonie/sound/c/b6b47040e897
[04/12] ASoC: mediatek: mt8189: Propagate reg_rw clock errors
        https://git.kernel.org/broonie/sound/c/29de66668799
[05/12] ASoC: mediatek: mt8189: Use dev_err_probe() for clock errors
        https://git.kernel.org/broonie/sound/c/ea8c96a05da7
[06/12] ASoC: mediatek: mt8189: Propagate runtime resume errors
        https://git.kernel.org/broonie/sound/c/483e827b005f
[07/12] ASoC: mediatek: mt8189: Remove redundant error message
        https://git.kernel.org/broonie/sound/c/e9e9e514eeef
[08/12] ASoC: mediatek: mt8189: Propagate APLL errors
        https://git.kernel.org/broonie/sound/c/6347f276da8c
[09/12] ASoC: mediatek: mt8189: Propagate MCLK errors
        https://git.kernel.org/broonie/sound/c/65b9d4a783cc
[10/12] ASoC: mediatek: mt8189: Validate sysclk frequency
        https://git.kernel.org/broonie/sound/c/9c73b5a43c99
[11/12] ASoC: mediatek: mt8189: Propagate TDM clock errors
        https://git.kernel.org/broonie/sound/c/242d0280eabb
[12/12] ASoC: mediatek: mt8189: Validate TDM MCLK frequency
        https://git.kernel.org/broonie/sound/c/197ec30e1b60

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