Re: omap-i2c: runtime pm issue during suspend to ram

[Thomas Richard <thomas.richard@bootlin.com>]

On 12/19/23 18:15, Thomas Richard wrote:
> Hello,

I add some people in this thread.

> 
> I have a gpio expander (pca953x driver) connected to an i2c controller
> managed by the omap-i2c driver.
> And I have some issues with pm_runtime_force_suspend/resume during
> suspend to ram.
> For some reasons, related to hardware design, I need to access to this
> gpio expander during suspend_noirq and resume_noirq. So I had to move
> the suspend/resume of the pca953x to suspend_noirq/resume_noirq.
> 
> The i2c controller is autosuspended when I start the suspend sequence.
> In suspend_noirq, I access to one gpio of the expander, so rpm_resume is
> called to resume the i2c controller.
> And rpm_resume returns an error because disable_depth > 0 [1]. In
> suspend_noirq, runtime pm is disabled (disable_depth is incremented when
> runtime pm is disabled [2]). So the expander is not reachable, and the
> access fails.
> 
> [1]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/base/power/runtime.c#L773
> [2]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/base/power/runtime.c#L1474
> 
> The suspend_noirq of the gpio expander don't do i2c access, so no
> problem for pca953x suspend.
> The pm_runtime_force_suspend (suspend_noirq [3]) of the i2c controller
> does nothing as the device is already suspended [4].
> 
> [3]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/i2c/busses/i2c-omap.c#L1579
> [4]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/base/power/runtime.c#L1878
> 
> Then during the pm_runtime_force_resume (resume_noirq [3]) the i2c
> controller is not resumed because needs_for_resume is equal to 0 [5].
> The needs_for_resume flag is set in pm_runtime_force_suspend [6] but we
> don't reach this point, because the device is already suspended [4].
> 
> [5]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/base/power/runtime.c#L1929
> [6]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/base/power/runtime.c#L1900
> 
> Then the resume_noirq of the pca953x driver is called, consequently
> rpm_resume is called to resume the i2c controller. But it is never
> resumed because disable_depth > 0 [7] (runtime pm is still disabled in
> resume_noirq). So the resume_noirq fails.
> 
> [7]
> https://elixir.bootlin.com/linux/v6.7-rc6/source/drivers/base/power/runtime.c#L773
> 
> I found a workaround which is to resume the controller and disable
> runtime pm during suspend, then runtime pm is enabled during resume.
> But there is probably a better solution to fix this issue.
> 
> Best Regards,
> 
> Thomas Richard
> 
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 42165ef57946..fe79b27b46fd 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1575,9 +1575,24 @@ static int __maybe_unused
> omap_i2c_runtime_resume(struct device *dev)
>         return 0;
>  }
> 
> +static int omap_i2c_suspend(struct device *dev)
> +{
> +       pm_runtime_get_sync(dev);
> +       pm_runtime_disable(dev);
> +       return 0;
> +}
> +
> +static int omap_i2c_resume(struct device *dev)
> +{
> +       pm_runtime_enable(dev);
> +       pm_runtime_put_autosuspend(dev);
> +       return 0;
> +}
> +
>  static const struct dev_pm_ops omap_i2c_pm_ops = {
>         SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>                                       pm_runtime_force_resume)
> +       SET_SYSTEM_SLEEP_PM_OPS(omap_i2c_suspend, omap_i2c_resume)
>         SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,
>                            omap_i2c_runtime_resume, NULL)
>  };
> 
> 
> 
-- 
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com