ASoC: mediatek: mt8189: Improve error handling
Subsystems (3)
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.
Compile-tested only.
Best regards,
Phuc
bui duc phuc (13):
ASoC: mediatek: mt8189: Return error for missing regmap
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 | 136 +++++++++++++++------ sound/soc/mediatek/mt8189/mt8189-afe-pcm.c | 49 +++++--- sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 17 ++- sound/soc/mediatek/mt8189/mt8189-dai-tdm.c | 17 ++- 4 files changed, 155 insertions(+), 64 deletions(-)
Signature · 1 line
2.43.0
PATCH 1/13 ASoC: mediatek: mt8189: Return error for missing regmap
From: bui duc phuc <[email protected]>
mt8189_afe_enable_top_cg() currently returns success when the AFE regmap is unavailable.
Return -EINVAL so callers can handle the error.
--- sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +1 -1
@@ -236,7 +236,7 @@ static int mt8189_afe_enable_top_cg(struct mtk_base_afe *afe, unsigned int cg_tyif (!afe->regmap) {dev_err (afe->dev,"afe regmap is null !!!\n" ); -return 0 ; +return -EINVAL ; }dev_dbg (afe->dev,"reg: 0x%x, mask: 0x%x, val: 0x%x\n" , reg, mask, val);
Signature · 1 line
2.43.0
PATCH 2/13 ASoC: mediatek: mt8189: Propagate APLL enable errors
In reply to [email protected]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.
--- 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
@@ -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_apll1_ck; ret =mt8189_afe_enable_top_cg (afe,MT8189_PDN_APLL_TUNER1 );if (ret) -return ret; + goto err_apll_tuner1;/* 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_tuner;/* 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_tuner;/* audio apll1 on */ ret =mt8189_afe_enable_top_cg (afe,MT8189_AUDIO_APLL1_EN_ON );if (ret) -return ret; + goto err_apll1_en_on;return 0 ; + +err_apll1_en_on: +regmap_update_bits (afe->regmap,AFE_APLL1_TUNER_CFG , +FREQ_TUNER_EN_MASK_SFT , +0x0 ); +err_tuner: +mt8189_afe_disable_top_cg (afe,MT8189_PDN_APLL_TUNER1 ); +err_apll_tuner1: +mt8189_afe_disable_top_cg (afe,MT8189_CG_APLL1_CK ); +err_apll1_ck: +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_apll2_ck; ret =mt8189_afe_enable_top_cg (afe,MT8189_PDN_APLL_TUNER2 );if (ret) -return ret; + goto err_apll_tuner2;/* 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_tuner;/* 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_tuner;/* audio apll2 on */ ret =mt8189_afe_enable_top_cg (afe,MT8189_AUDIO_APLL2_EN_ON );if (ret) -return ret; + goto err_apll2_en_on;return 0 ; + +err_apll2_en_on: +regmap_update_bits (afe->regmap,AFE_APLL2_TUNER_CFG , +FREQ_TUNER_EN_MASK_SFT , +0x0 ); +err_tuner: +mt8189_afe_disable_top_cg (afe,MT8189_PDN_APLL_TUNER2 ); +err_apll_tuner2: +mt8189_afe_disable_top_cg (afe,MT8189_CG_APLL2_CK ); +err_apll2_ck: +apll2_mux_setting (afe, false); + +return ret; }void mt8189_apll2_disable (struct mtk_base_afe *afe)
Signature · 1 line
2.43.0
Re: [PATCH 02/13] ASoC: mediatek: mt8189: Propagate APLL enable errors
sound/soc/mediatek/mt8189/mt8189-afe-clk.c int mt8189_apll1_enable(struct mtk_base_afe *afe) · line 457
Quoted 13 lines · On Thu, Sep 10, 2026 at 07:57:31PM +0700, [email protected] wrote:
sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 78 ++++++++++++++++------ 1 file changed, 56 insertions(+), 22 deletions(-)sound/soc/mediatek/mt8189/mt8189-afe-clk.c +1
-1@@ -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_apll1_ck;
I shouldn't complain about this, but I am going to... I don't like ComeFrom label names at all. Imagine if we named functions that way, there would be a thousand functions named called_from_probe(). We already are looking at the goto so we know where the goto is, but what we want to know is what the goto does.
Better to name it err_clear_mux_setting or something.
Quoted 7 lines
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +1
-1ret =mt8189_afe_enable_top_cg (afe,MT8189_PDN_APLL_TUNER1 );if (ret) -return ret; + goto err_apll_tuner1;/* sel 44.1kHz:1, apll_div:7, upper bound:3 */
sound/soc/mediatek/mt8189/mt8189-afe-clk.c int mt8189_apll1_enable(struct mtk_base_afe *afe) · line 468
Quoted 10 lines
sound/soc/mediatek/mt8189/mt8189-afe-clk.c +5
-5-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 ));
Since you can't test it, it's a bit risky to start caring about errors.
regards,
dan carpener
Re: [PATCH 02/13] ASoC: mediatek: mt8189: Propagate APLL enable errors
Hi Dan,
Quoted 3 lines
Better to name it
err_clear_mux_settingor something.
Thanks for pointing this out, I'll rename the labels accordingly.
Quoted 4 lines
Since you can't test it, it's a bit risky to start caring about errors.
Fair point — though ignoring all errors isn't ideal either.
Best regards,
Phuc
PATCH 3/13 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.
--- 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
@@ -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_m_sel; }/* 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_m_sel; ret =mt8189_afe_set_clk_rate (afe, afe_priv->clk[div_clk_id], rate);if (ret) -return ret; + goto err_div;return 0 ; + +err_div: +mt8189_afe_disable_clk (afe, afe_priv->clk[div_clk_id]); +err_m_sel: +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
PATCH 4/13 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.
--- 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
@@ -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
PATCH 5/13 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.
--- 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
@@ -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_audiointbus; +/* 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_audiointbus; + + 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_audio_h;return 0 ; + +err_audio_h: +mt8189_afe_disable_clk (afe, afe_priv->clk[MT8189_CLK_TOP_MUX_AUDIO_H ]); +err_audiointbus: +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
PATCH 6/13 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.
--- 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
@@ -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
PATCH 7/13 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.
--- sound/soc/mediatek/mt8189/mt8189-afe-pcm.c | 36 +++++++++++++++++----- 1 file changed, 29 insertions(+), 7 deletions(-)
sound/soc/mediatek/mt8189/mt8189-afe-pcm.c +29 -7
@@ -2328,24 +2328,46 @@ static int mt8189_afe_runtime_resume(struct device *dev)if (!afe->regmap) {dev_warn (afe->dev,"skip regmap\n" ); -return 0 ; + ret = -EINVAL ; + goto err_reg_rw_clk; }regcache_cache_only (afe->regmap, false); -regcache_sync (afe->regmap); + ret =regcache_sync (afe->regmap); +if (ret) + goto err_regcache_sync;/* 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_regcache_sync; + + ret =regmap_update_bits (afe->regmap,AFE_CBIP_CFG0 ,0x1 ,0x1 ); +if (ret) + goto err_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_26m_req;/* enable AFE */ -mt8189_afe_enable_main_clock (afe); + ret =mt8189_afe_enable_main_clock (afe); +if (ret) + goto err_26m_req;return 0 ; + +err_26m_req: +regmap_update_bits (afe->regmap, +AFE_SPM_CONTROL_REQ ,0x1 ,0x0 ); +err_regcache_sync: +regcache_cache_only (afe->regmap, true); +err_reg_rw_clk: +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
PATCH 8/13 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.
--- 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
@@ -2488,13 +2488,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 */
@@ -2528,13 +2527,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);
@@ -2603,10 +2602,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
PATCH 9/13 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.
--- 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
@@ -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
PATCH 10/13 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.
--- sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
sound/soc/mediatek/mt8189/mt8189-dai-i2s.c +4 -3
@@ -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 =0 ;dev_dbg (cmpnt->dev,"%s(), name %s, event 0x%x\n" , __func__, w->name, event);
@@ -529,17 +530,17 @@ 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 :break ; } -return 0 ; +return ret; }static const struct snd_soc_dapm_widget mtk_dai_i2s_widgets[] = {
Signature · 1 line
2.43.0
PATCH 11/13 ASoC: mediatek: mt8189: Validate sysclk frequency
In reply to [email protected]From: bui duc phuc <[email protected]>
mtk_dai_i2s_set_sysclk() accepts zero or negative frequencies. Reject invalid frequencies before configuring the I2S clock.
--- sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 3 +++ 1 file changed, 3 insertions(+)
sound/soc/mediatek/mt8189/mt8189-dai-i2s.c +3 -0
@@ -1286,6 +1286,9 @@ static int mtk_dai_i2s_set_sysclk(struct snd_soc_dai *dai, dir != SND_SOC_CLOCK_OUT)return -EINVAL ; +if (freq <=0 ) +return -EINVAL ; + i2s_priv = afe_priv->dai_priv[dai->id];if (!i2s_priv)return -EINVAL ;
Signature · 1 line
2.43.0
Re: [PATCH 11/13] ASoC: mediatek: mt8189: Validate sysclk frequency
Quoted 3 lines · On Thu, Sep 10, 2026 at 07:57:40PM +0700, [email protected] wrote:
From: bui duc phuc <[email protected]>
mtk_dai_i2s_set_sysclk()accepts zero or negative frequencies.
^^^^^^^^^^^^^^^^^^^^
It's unsigned int so it can't be negative.
regards,
dan carpenter
Quoted 24 lines
Reject invalid frequencies before configuring the I2S clock.
Fixes 7eb153585598 ("ASoC: mediatek: mt8189: add platform driver")Signed-off-by bui duc phuc <[email protected]>--- sound/soc/mediatek/mt8189/mt8189-dai-i2s.c | 3 +++ 1 file changed, 3 insertions(+)sound/soc/mediatek/mt8189/mt8189-dai-i2s.c +3
-0@@ -1286,6 +1286,9 @@ static int mtk_dai_i2s_set_sysclk(struct snd_soc_dai *dai, dir != SND_SOC_CLOCK_OUT)return -EINVAL ; +if (freq <=0 ) +return -EINVAL ; + i2s_priv = afe_priv->dai_priv[dai->id];if (!i2s_priv)return -EINVAL ;Signature · 1 line
2.43.0
Re: [PATCH 11/13] ASoC: mediatek: mt8189: Validate sysclk frequency
Hi Dan
Quoted 4 lines
Quoted 1 line
mtk_dai_i2s_set_sysclk()accepts zero or negative frequencies.^^^^^^^^^^^^^^^^^^^^It's unsigned int so it can't be negative.
You're right, thanks for catching that. I'll fix it.
Best regards,
Phuc
PATCH 12/13 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.
--- sound/soc/mediatek/mt8189/mt8189-dai-tdm.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
sound/soc/mediatek/mt8189/mt8189-dai-tdm.c +8 -6
@@ -261,22 +261,23 @@ 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 =0 ;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 :break ; } -return 0 ; +return ret; } static int mtk_tdm_mck_en_event(struct snd_soc_dapm_widget *w,
@@ -288,23 +289,24 @@ 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 =0 ;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 :break ; } -return 0 ; +return ret; }static const struct snd_soc_dapm_widget mtk_dai_tdm_widgets[] = {
Signature · 1 line
2.43.0
PATCH 13/13 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.
--- sound/soc/mediatek/mt8189/mt8189-dai-tdm.c | 3 +++ 1 file changed, 3 insertions(+)
sound/soc/mediatek/mt8189/mt8189-dai-tdm.c +3 -0
@@ -381,6 +381,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