2023-06-14 05:11:08

by Tony Lindgren

Subject: [PATCH] serial: 8250_omap: Use force_suspend and resume for system suspend

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 <[email protected]>
---
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;
}

--
2.41.0

2023-06-14 06:13:58

by Dhruva Gole

Subject: Re: [PATCH] serial: 8250_omap: Use force_suspend and resume for system suspend

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 <[email protected]>
> ---

Tested-by: Dhruva Gole <[email protected]>

> drivers/tty/serial/8250/8250_omap.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
> [...]


--
Thanks and Regards,
Dhruva Gole