Re: [PATCH v2 4/5] ASoC: rockchip: spdif: Restore regcache cache-only mode on sync failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The error paths in `i2s_runtime_resume`, `i2s_tdm_runtime_resume`, and `rockchip_sai_runtime_resume` fail to restore `regcache_cache_only(..., true)` when `regcache_sync()` fails.
- [High] The regmap is initialized with `cache_only = false` but the device is placed in `RPM_SUSPENDED` state at probe completion without setting `cache_only = true`.
--

commit 92568295e5eeb9d7ffb897ac4754fcc60c989c10
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: rockchip: spdif: Restore regcache cache-only mode on sync failure

If regcache_sync() fails during runtime resume, the driver disables the
clocks and returns an error. This restores cache-only mode in the error
path so subsequent register accesses continue to use the cache while the
device is inactive.

Reported-by: Sashiko AI Review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260522103713.6C09D1F000E9@smtp.kernel.org/

> 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
> @@ -94,6 +94,7 @@ static int rk_spdif_runtime_resume(struct device *dev)
>  
>  	ret = regcache_sync(spdif->regmap);
>  	if (ret) {
> +		regcache_cache_only(spdif->regmap, true);
>  		clk_disable_unprepare(spdif->mclk);
>  		clk_disable_unprepare(spdif->hclk);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while this change fixes the
issue in rockchip_spdif.c, the exact same error path exists in other
Rockchip ASoC drivers.

For example, in sound/soc/rockchip/rockchip_i2s.c:i2s_runtime_resume():

    ret = regcache_sync(i2s->regmap);
    if (ret)
        clk_disable_unprepare(i2s->mclk);

Because the error path drops the device back to the suspended state
without restoring cache-only mode, subsequent regmap accesses (like
reading from debugfs /sys/kernel/debug/regmap/) will bypass the -EBUSY
check in _regmap_read() and attempt to read directly from memory-mapped
registers. Accessing registers in a powered-off domain causes the AHB bus
to hang, resulting in an imprecise external abort and kernel panic.

Similar paths exist in rockchip_i2s_tdm.c:i2s_tdm_runtime_resume() and
rockchip_sai.c:rockchip_sai_runtime_resume(). Should these drivers be
updated as well to fully resolve the issue across the subsystem?

[Severity: High]
This is also a pre-existing issue, but does rk_spdif_probe() leave the
device exposed to a similar abort before the first resume?

In sound/soc/rockchip/rockchip_spdif.c:rk_spdif_probe():

    devm_pm_runtime_enable(&pdev->dev);

    if (!pm_runtime_enabled(&pdev->dev)) {

The regmap is initialized with cache_only = false by default. When runtime
PM is enabled, the device is placed in the RPM_SUSPENDED state and the
hardware power domain can be powered off.

Because config->pm_runtime is not set to true in the regmap configuration,
if a regmap read occurs (e.g., via debugfs) before the device is first
resumed via ALSA, wouldn't regmap attempt to read physical registers while
the power domain is off? Should rk_spdif_probe() explicitly call
regcache_cache_only(spdif->regmap, true) before completing?

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