Re: [PATCH v2 07/13] ASoC: mediatek: mt8189: Propagate runtime resume errors

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

On 9/15/26 10:23, Bui Duc Phuc wrote:
> Hi Angelo,
> 
> 
>>>
>>> In the probe function, there's a call to devm_regmap_init_mmio(), and that's being
>>> correctly checked for error as in, if any, probe will fail.
>>>
>>> So... during suspend or resume or anywhere else in this driver really, the regmap
>>> pointer can't be NULL.
>>> The right thing to do here would be to just remove the useless check.
>>>
>>> Mind you, this comment applies to some other commits in this series as well.
>>>
> 
> I found something interesting:
> 
> in mt8189_afe_pcm_dev_probe()
> 
> -------------------------------------
> afe->runtime_resume = mt8189_afe_runtime_resume;
> afe->runtime_suspend = mt8189_afe_runtime_suspend;
> 
> ret = devm_pm_runtime_enable(dev);
> if (ret)
> return ret;
> 
> /*
> * Audio device is part of genpd. Registering it as a syscore device
> * ensure the proper power-on sequence of the AFE device.
> */
> dev_pm_syscore_device(dev, true);
> 
> /* enable clock for regcache get default value from hw */
> ret = pm_runtime_resume_and_get(dev);
> if (ret)
> return dev_err_probe(dev, ret, "failed to resume device\n");
> 
> afe->regmap = devm_regmap_init_mmio(dev, afe->base_addr,
>     &mt8189_afe_regmap_config);
> if (IS_ERR(afe->regmap)) {
> ret = PTR_ERR(afe->regmap);
> goto err_pm_put;
> }
> -----------------------------------------------
> 
> Here, it looks like mt8189_afe_runtime_resume() gets called
> before devm_regmap_init_mmio().
> 
> So if we remove this part:
> ---------------------
> if (!afe->regmap) {
>          dev_warn(afe->dev, "skip regmap\n");
>          return 0;
>      }
> -----------------------
> from mt8189_afe_runtime_resume(), that could actually trigger the bug
> we were just discussing.
> 
Well, then...

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

...still, please check the other cases.