| Message ID | 20240102-j7200-pcie-s2r-v4-3-6f1f53390c85@bootlin.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add suspend to ram support for PCIe on J7200 | expand |
* Thomas Richard <thomas.richard@bootlin.com> [240304 15:36]: > A device may need the controller up during suspend_noirq() or > resume_noirq(). > But if the controller is autosuspended, there is no way to wakeup it during > suspend_noirq() or resume_noirq() because runtime pm is disabled at this > time. > > The suspend() callback wakes up the controller, so it is available until > its suspend_noirq() callback (pm_runtime_force_suspend()). > During the resume, it's restored by resume_noirq() callback > (pm_runtime_force_resume()). Then resume() callback enables autosuspend. Reviewed-by: Tony Lindgren <tony@atomide.com>
On Fri, Mar 08, 2024 at 10:42:40AM +0200, Tony Lindgren wrote: > * Thomas Richard <thomas.richard@bootlin.com> [240304 15:36]: > > A device may need the controller up during suspend_noirq() or > > resume_noirq(). > > But if the controller is autosuspended, there is no way to wakeup it during > > suspend_noirq() or resume_noirq() because runtime pm is disabled at this > > time. > > > > The suspend() callback wakes up the controller, so it is available until > > its suspend_noirq() callback (pm_runtime_force_suspend()). > > During the resume, it's restored by resume_noirq() callback > > (pm_runtime_force_resume()). Then resume() callback enables autosuspend. > > Reviewed-by: Tony Lindgren <tony@atomide.com> I fully trust Tony and I assume that this series should go in via some other tree than I2C. So: Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Andi, do you agree?
Hi Thomas, On Mon, Mar 04, 2024 at 04:35:46PM +0100, Thomas Richard wrote: > A device may need the controller up during suspend_noirq() or > resume_noirq(). > But if the controller is autosuspended, there is no way to wakeup it during > suspend_noirq() or resume_noirq() because runtime pm is disabled at this > time. > > The suspend() callback wakes up the controller, so it is available until > its suspend_noirq() callback (pm_runtime_force_suspend()). > During the resume, it's restored by resume_noirq() callback > (pm_runtime_force_resume()). Then resume() callback enables autosuspend. > > So the controller is up during a little time slot in suspend and resume > sequences even if it's not used. > > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
Hi Wolfram, On Tue, Mar 12, 2024 at 09:03:27AM +0100, Wolfram Sang wrote: > On Fri, Mar 08, 2024 at 10:42:40AM +0200, Tony Lindgren wrote: > > * Thomas Richard <thomas.richard@bootlin.com> [240304 15:36]: > > > A device may need the controller up during suspend_noirq() or > > > resume_noirq(). > > > But if the controller is autosuspended, there is no way to wakeup it during > > > suspend_noirq() or resume_noirq() because runtime pm is disabled at this > > > time. > > > > > > The suspend() callback wakes up the controller, so it is available until > > > its suspend_noirq() callback (pm_runtime_force_suspend()). > > > During the resume, it's restored by resume_noirq() callback > > > (pm_runtime_force_resume()). Then resume() callback enables autosuspend. > > > > Reviewed-by: Tony Lindgren <tony@atomide.com> > > I fully trust Tony and I assume that this series should go in via some > other tree than I2C. So: > > Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > Andi, do you agree? Agree... but who is going to take this? Eventually I can just this one. Andi
Hi Andi, > Agree... but who is going to take this? Eventually I can just > this one. Usually, it should be stated in the coverletter what the suggested path for upstreaming is. If it is not, then I apply my gut feeling what should be better. With this series (and seeing that there are no other omap patches pending), I assume some other tree so I ack in advance. If I am wrong and should apply it independently, people will tell me at this stage. That's how I handle it. Happy hacking, Wolfram
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 42165ef57946..28417b2a18b0 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1575,9 +1575,31 @@ static int __maybe_unused omap_i2c_runtime_resume(struct device *dev) return 0; } +static int omap_i2c_suspend(struct device *dev) +{ + /* + * If the controller is autosuspended, there is no way to wakeup it once + * runtime pm is disabled (in suspend_late()). + * But a device may need the controller up during suspend_noirq() or + * resume_noirq(). + * Wakeup the controller while runtime pm is enabled, so it is available + * until its suspend_noirq(), and from resume_noirq(). + */ + return pm_runtime_resume_and_get(dev); +} + +static int omap_i2c_resume(struct device *dev) +{ + pm_runtime_mark_last_busy(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) };
A device may need the controller up during suspend_noirq() or resume_noirq(). But if the controller is autosuspended, there is no way to wakeup it during suspend_noirq() or resume_noirq() because runtime pm is disabled at this time. The suspend() callback wakes up the controller, so it is available until its suspend_noirq() callback (pm_runtime_force_suspend()). During the resume, it's restored by resume_noirq() callback (pm_runtime_force_resume()). Then resume() callback enables autosuspend. So the controller is up during a little time slot in suspend and resume sequences even if it's not used. Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> --- drivers/i2c/busses/i2c-omap.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)