diff mbox series

[v4,03/18] i2c: omap: wakeup the controller during suspend() callback

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

Commit Message

Thomas Richard March 4, 2024, 3:35 p.m. UTC
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(+)

Comments

Wolfram Sang March 12, 2024, 8:03 a.m. UTC | #1
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?
Andi Shyti March 13, 2024, 12:37 a.m. UTC | #2
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
Andi Shyti March 13, 2024, 12:48 a.m. UTC | #3
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
Wolfram Sang March 13, 2024, 6:21 a.m. UTC | #4
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 mbox series

Patch

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)
 };