Re: [PATCH 02/13] ASoC: mediatek: mt8189: Propagate APLL enable errors

[Dan Carpenter]

On Thu, Sep 10, 2026 at 07:57:31PM +0700, phucduc.bui@xxxxxxxxx wrote:
>  sound/soc/mediatek/mt8189/mt8189-afe-clk.c | 78 ++++++++++++++++------
>  1 file changed, 56 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/soc/mediatek/mt8189/mt8189-afe-clk.c b/sound/soc/mediatek/mt8189/mt8189-afe-clk.c
> index 63e03a40dbbe..a901be97e75f 100644
> --- a/sound/soc/mediatek/mt8189/mt8189-afe-clk.c
> +++ b/sound/soc/mediatek/mt8189/mt8189-afe-clk.c
> @@ -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.

>  
>  	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));

Since you can't test it, it's a bit risky to start caring about
errors.

regards,
dan carpener