Re: [PATCH v3 2/5] ASoC: mediatek: mt8183: Fix clock handling in mux disable path

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 8/28/2026 7:05 AM, phucduc.bui@gmail.com wrote:

> @@ -339,25 +340,26 @@ static int apll2_mux_setting(struct mtk_base_afe *afe, bool enable)
>   			goto ERR_SELECT_CLK_TOP_MUX_AUD_ENG2;
>   		}
>   	} else {
> -		ret = clk_set_parent(afe_priv->clk[CLK_TOP_MUX_AUD_ENG2],
> -				     afe_priv->clk[CLK_CLK26M]);
> -		if (ret) {
> -			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
> -				__func__, aud_clks[CLK_TOP_MUX_AUD_ENG2],
> -				aud_clks[CLK_CLK26M], ret);
> -			goto EXIT;
> -		}
>   		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_ENG2]);
> -
> -		ret = clk_set_parent(afe_priv->clk[CLK_TOP_MUX_AUD_2],
> -				     afe_priv->clk[CLK_CLK26M]);
> -		if (ret) {
> -			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
> -				__func__, aud_clks[CLK_TOP_MUX_AUD_2],
> -				aud_clks[CLK_CLK26M], ret);
> -			goto EXIT;
> -		}
>   		clk_disable_unprepare(afe_priv->clk[CLK_TOP_MUX_AUD_2]);
> +
> +		ret = clk_set_parent(afe_priv->clk[CLK_TOP_MUX_AUD_ENG2],
> +				     afe_priv->clk[CLK_CLK26M]);
> +		if (ret) {
> +			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
> +				__func__, aud_clks[CLK_TOP_MUX_AUD_ENG2],
> +				aud_clks[CLK_CLK26M], ret);
> +			goto EXIT;
> +		}
> +
> +		ret = clk_set_parent(afe_priv->clk[CLK_TOP_MUX_AUD_2],
> +				     afe_priv->clk[CLK_CLK26M]);
> +		if (ret) {
> +			dev_err(afe->dev, "%s clk_set_parent %s-%s fail %d\n",
> +				__func__, aud_clks[CLK_TOP_MUX_AUD_2],
> +				aud_clks[CLK_CLK26M], ret);
> +			goto EXIT;
> +		}
>   	}
>   
>   	return 0;


I was thinking off a typical teardown procedure - everything is permissive:

// the disable path:
	ret = clk_set_parent(CLK_TOP_MUX_AUD_ENG2)
	if (ret)
		// just error reporting
	ret = clk_disable_unprepare(CLK_TOP_MUX_AUD_ENG2)
	if (ret)
		// just error reporting
	ret = clk_set_parent(CLK_TOP_MUX_AUD_2)
	if (ret)
		// just error reporting
	ret = clk_disable_unprepare(CLK_TOP_MUX_AUD_2)
	if (ret)
		// just error reporting

or, if one wants to do this properly: split apll1/2_mux_setting() 
functions into:
	void apll1/2_mux_disable()
	int apll1/2_mux_enable()

You do not have to go for the split, the first option is just fine.