Re: [PATCH v2 05/14] ASoC: mediatek: mt8195: Fix paired memif clock error handling

[Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>]

On 10/5/2026 4:32 PM, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Check and propagate errors when preparing and enabling the paired
> DL8/DL10 memif clocks.
> 
> If enabling the second clock fails, undo the clock that was already
> prepared or enabled to keep the clock state balanced.
> 
> Fixes: 6746cc858259 ("ASoC: mediatek: mt8195: add platform driver")
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>   sound/soc/mediatek/mt8195/mt8195-afe-pcm.c | 24 ++++++++++++++++++----
>   1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c b/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
> index 52c3381e6766..1d3e01e2c3e7 100644
> --- a/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
> +++ b/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
> @@ -286,15 +286,23 @@ mt8195_afe_paired_memif_clk_prepare(struct snd_pcm_substream *substream,
>   	struct mt8195_afe_private *afe_priv = afe->platform_priv;
>   	int id = snd_soc_rtd_to_cpu(rtd, 0)->id;
>   	int clk_id;
> +	int ret;
>   
>   	if (id != MT8195_AFE_MEMIF_DL8 && id != MT8195_AFE_MEMIF_DL10)
>   		return 0;
>   
>   	if (enable) {
>   		clk_id = MT8195_CLK_AUD_MEMIF_DL10;
> -		mt8195_afe_prepare_clk(afe, afe_priv->clk[clk_id]);
> +		ret = mt8195_afe_prepare_clk(afe, afe_priv->clk[clk_id]);
> +		if (ret)
> +			return ret;
>   		clk_id = MT8195_CLK_AUD_MEMIF_DL8;
> -		mt8195_afe_prepare_clk(afe, afe_priv->clk[clk_id]);
> +		ret = mt8195_afe_prepare_clk(afe, afe_priv->clk[clk_id]);
> +		if (ret) {
> +			clk_id = MT8195_CLK_AUD_MEMIF_DL10;
> +			mt8195_afe_unprepare_clk(afe, afe_priv->clk[clk_id]);
> +			return ret;
> +		}
>   	} else {
>   		clk_id = MT8195_CLK_AUD_MEMIF_DL8;
>   		mt8195_afe_unprepare_clk(afe, afe_priv->clk[clk_id]);
> @@ -315,6 +323,7 @@ mt8195_afe_paired_memif_clk_enable(struct snd_pcm_substream *substream,
>   	struct mt8195_afe_private *afe_priv = afe->platform_priv;
>   	int id = snd_soc_rtd_to_cpu(rtd, 0)->id;
>   	int clk_id;
> +	int ret;
>   
>   	if (id != MT8195_AFE_MEMIF_DL8 && id != MT8195_AFE_MEMIF_DL10)
>   		return 0;
> @@ -322,11 +331,18 @@ mt8195_afe_paired_memif_clk_enable(struct snd_pcm_substream *substream,
>   	if (enable) {
>   		/* DL8_DL10_MEM */
>   		clk_id = MT8195_CLK_AUD_MEMIF_DL10;
> -		mt8195_afe_enable_clk_atomic(afe, afe_priv->clk[clk_id]);
> +		ret = mt8195_afe_enable_clk_atomic(afe, afe_priv->clk[clk_id]);
> +		if (ret)
> +			return ret;
>   		udelay(1);
>   		/* DL8_DL10_AGENT */
>   		clk_id = MT8195_CLK_AUD_MEMIF_DL8;
> -		mt8195_afe_enable_clk_atomic(afe, afe_priv->clk[clk_id]);
> +		ret = mt8195_afe_enable_clk_atomic(afe, afe_priv->clk[clk_id]);
> +		if (ret) {
> +			clk_id = MT8195_CLK_AUD_MEMIF_DL10;
> +			mt8195_afe_disable_clk_atomic(afe, afe_priv->clk[clk_id]);
> +			return ret;
> +		}

This makes mt8195_afe_paired_memif_clk_enable() return failures from the
paired DL8/DL10 clock enables, but the return value still appears to be
dropped by the trigger path.

After applying the full series, mt8195_afe_fe_trigger() still calls the
enable helper like this on START/RESUME:

    mt8195_afe_paired_memif_clk_enable(substream, dai, 1);

so a failure from clk_enable() would not be propagated back through the 
DAI .trigger callback. Patch 6 handles the prepare-side helper in 
startup(), but I do not see the enable-side helper being handled by any 
later patch.

Should the START/RESUME path check this return value and unwind the 
trigger state if enabling the paired MEMIF clock fails?

Thanks
Ajay Kumar Nandam


>   	} else {
>   		/* DL8_DL10_AGENT */
>   		clk_id = MT8195_CLK_AUD_MEMIF_DL8;