Re: [PATCH v2 2/4] ASoC: mediatek: mt8183: Fix APLL enable error handling

[Cezary Rojewski]

On 8/21/2026 2:29 PM, phucduc.bui@xxxxxxxxx wrote:
> From: bui duc phuc <phucduc.bui@xxxxxxxxx>
>
> 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.


I have mixed feelings about appl*_mux_setting().  Take a look at its 
disable-path: if clk_set_parent() fails the follow up 
clk_disable_unprepare() is skipped possibly leaving one of the clks 
hanging.  I'd expect error paths of callers (of said mux_setting() 
function) to ensure all the clks are disabled and unprepared 
unconditionally.

> Fixes: a94aec035a12 ("ASoC: mediatek: mt8183: add platform driver")
> Signed-off-by: bui duc phuc <phucduc.bui@xxxxxxxxx>
> ---
>   sound/soc/mediatek/mt8183/mt8183-afe-clk.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/mediatek/mt8183/mt8183-afe-clk.c b/sound/soc/mediatek/mt8183/mt8183-afe-clk.c
> index 6ab0734ad136..0790d8123179 100644
> --- a/sound/soc/mediatek/mt8183/mt8183-afe-clk.c
> +++ b/sound/soc/mediatek/mt8183/mt8183-afe-clk.c
> @@ -382,7 +382,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)
> +		goto ERR_APLL1_MUX_SETTING;

Why goto? The check is valid but the label in my opinion is unnecessary.

>   
>   	ret = clk_prepare_enable(afe_priv->clk[CLK_APLL22M]);
>   	if (ret) {
> @@ -411,6 +413,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);
> +ERR_APLL1_MUX_SETTING:

While UPPER case for goto-labels is not part of the coding style I see 
why you did it - to be cohesive with the rest of the file.