| Message ID | 20230614045922.4798-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | serial: 8250_omap: Use force_suspend and resume for system suspend | expand |
Hi Tony, Thanks for the fix! On 14/06/23 10:29, Tony Lindgren wrote: > We should not rely on autosuspend timeout for system suspend. Instead, > let's use force_suspend and force_resume functions. Otherwise the serial > port controller device may not be idled on suspend. > > As we are doing a register write on suspend to configure the serial port, > we still need to runtime PM resume the port on suspend. > > While at it, let's switch to pm_runtime_resume_and_get() and check for > errors returned. And let's add the missing line break before return to the > suspend function while at it. > > Fixes: 09d8b2bdbc5c ("serial: 8250: omap: Provide ability to enable/disable UART as wakeup source") > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- Tested-by: Dhruva Gole <d-gole@ti.com> > drivers/tty/serial/8250/8250_omap.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > [...]
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -1618,25 +1618,35 @@ static int omap8250_suspend(struct device *dev) { struct omap8250_priv *priv = dev_get_drvdata(dev); struct uart_8250_port *up = serial8250_get_port(priv->line); + int err; serial8250_suspend_port(priv->line); - pm_runtime_get_sync(dev); + err = pm_runtime_resume_and_get(dev); + if (err) + return err; if (!device_may_wakeup(dev)) priv->wer = 0; serial_out(up, UART_OMAP_WER, priv->wer); - pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); - + err = pm_runtime_force_suspend(dev); flush_work(&priv->qos_work); - return 0; + + return err; } static int omap8250_resume(struct device *dev) { struct omap8250_priv *priv = dev_get_drvdata(dev); + int err; + err = pm_runtime_force_resume(dev); + if (err) + return err; serial8250_resume_port(priv->line); + /* Paired with pm_runtime_resume_and_get() in omap8250_suspend() */ + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + return 0; }
We should not rely on autosuspend timeout for system suspend. Instead, let's use force_suspend and force_resume functions. Otherwise the serial port controller device may not be idled on suspend. As we are doing a register write on suspend to configure the serial port, we still need to runtime PM resume the port on suspend. While at it, let's switch to pm_runtime_resume_and_get() and check for errors returned. And let's add the missing line break before return to the suspend function while at it. Fixes: 09d8b2bdbc5c ("serial: 8250: omap: Provide ability to enable/disable UART as wakeup source") Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/tty/serial/8250/8250_omap.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)