Re: [PATCH 2/2] i3c: master: svc: remove redundant pm_runtime_dont_use_autosuspend() calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] i3c: master: svc: clock and pinctrl leak in
  svc_i3c_master_remove()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] i3c: master: svc: broken probe deferral in
  svc_i3c_master_probe() [1]
--

commit bd316ea905bc7cd25e557b9e7ceaaf86534b31c5
Author: Joshua Crofts <crofts@kernel.org>

i3c: master: svc: remove redundant pm_runtime_dont_use_autosuspend() calls

This commit removes pm_runtime_dont_use_autosuspend() calls from the probe
error path and remove callback in svc-i3c-master.

> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e700392f0801f..9b845b2955be4 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -2084,7 +2084,6 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	return 0;
>  
>  rpm_disable:
> -	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_set_suspended(&pdev->dev);

[Severity: Medium]
This problem wasn't introduced by this patch, but earlier in
svc_i3c_master_probe() [1], does passing -EINVAL instead of
master->num_clks to dev_err_probe() break probe deferral?

drivers/i3c/master/svc-i3c-master.c:svc_i3c_master_probe() {
	...
	master->num_clks = devm_clk_bulk_get_all(dev, &master->clks);
	if (master->num_clks < 0)
		return dev_err_probe(dev, -EINVAL, "can't get I3C clocks\n");
	...
}

If the clock provider is not ready and devm_clk_bulk_get_all() returns
-EPROBE_DEFER, probe fails permanently with -EINVAL instead of deferring.

> @@ -2101,7 +2100,6 @@ static void svc_i3c_master_remove(struct platform_device *pdev)
>  
>  	i3c_master_unregister(&master->base);
>  
> -	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  }

[Severity: Medium]
Does removing pm_runtime_dont_use_autosuspend() here leave master->clks
prepared and enabled and pinctrl in the active state after driver removal?

i3c_master_unregister() calls i3c_master_bus_cleanup() ->
svc_i3c_master_bus_cleanup(), which wakes &pdev->dev via
pm_runtime_resume_and_get() and then calls pm_runtime_put_autosuspend().
Because autosuspend is enabled with a 1000 ms delay, that call only arms
the autosuspend timer and returns with &pdev->dev still active and
master->clks enabled.

Previously, calling pm_runtime_dont_use_autosuspend() before
pm_runtime_disable() caused the runtime PM core to synchronously run
svc_i3c_runtime_suspend(), which disables and unprepares master->clks and
selects the sleep pinctrl state:

drivers/i3c/master/svc-i3c-master.c:svc_i3c_runtime_suspend() {
	struct svc_i3c_master *master = dev_get_drvdata(dev);

	svc_i3c_save_regs(master);
	clk_bulk_disable_unprepare(master->num_clks, master->clks);
	pinctrl_pm_select_sleep_state(dev);

	return 0;
}

With pm_runtime_dont_use_autosuspend() removed, pm_runtime_disable()
cancels the pending autosuspend timer without running
svc_i3c_runtime_suspend(), and the later pm_runtime_reinit() in the driver
core runs after runtime PM is already disabled.

[1] https://sashiko.dev/#/bug/linux-7f6f30e6-7dc6-4272-b9a7-e0326db501a1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-dont-use-autosuspend-i3c-v1-0-80a1546354ce@kernel.org?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c