Re: [PATCH 2/3] ASoC: rockchip: spdif: Reorder clock enable sequence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Failure to restore regmap cache-only state if `regcache_sync()` fails during runtime resume.
--

commit 517eeaafec0deb4d7a25eb1c2641316c2b2e2b72
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: rockchip: spdif: Reorder clock enable sequence

This commit reorders the clock enablement sequence during runtime resume.
It ensures the bus clock (hclk) is prepared and enabled before the
controller clock (mclk) to guarantee proper register access.

> diff --git a/sound/soc/rockchip/rockchip_spdif.c b/sound/soc/rockchip/rockchip_spdif.c
> --- a/sound/soc/rockchip/rockchip_spdif.c
> +++ b/sound/soc/rockchip/rockchip_spdif.c
[ ... ]
> @@ -76,16 +76,16 @@ static int rk_spdif_runtime_resume(struct device *dev)
[ ... ]
> -	ret = clk_prepare_enable(spdif->hclk);
> +	ret = clk_prepare_enable(spdif->mclk);
>  	if (ret) {
> -		clk_disable_unprepare(spdif->mclk);
> -		dev_err(spdif->dev, "hclk clock enable failed %d\n", ret);
> +		clk_disable_unprepare(spdif->hclk);
> +		dev_err(spdif->dev, "mclk clock enable failed %d\n", ret);
>  		return ret;
>  	}
>  
>  	regcache_cache_only(spdif->regmap, false);
>  	regcache_mark_dirty(spdif->regmap);
>  
>  	ret = regcache_sync(spdif->regmap);
>  	if (ret) {
>  		clk_disable_unprepare(spdif->mclk);
>  		clk_disable_unprepare(spdif->hclk);

[Severity: High]
This is a pre-existing issue, but is the regmap cache-only state left
unrestored on this error path?

If regcache_sync() fails, the driver disables the clocks and returns an error.
This causes the runtime power management core to leave the device in the
suspended state.

However, the error path doesn't appear to restore the cache-only mode via
regcache_cache_only(spdif->regmap, true).

If a subsequent register read or write occurs while the device is in this
suspended state, could the regmap bypass the cache and attempt a physical
memory-mapped I/O access?

Since the generic power domain would be powered off when suspended,
attempting this access might trigger an asynchronous system error or
synchronous data abort.

>  	}
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522100318.73474-1-phucduc.bui@gmail.com?part=2