| Message ID | 20241008-8250-omap-no-console-suspend-v1-2-e7f0365c02f0@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix no_console_suspend in the 8250_omap driver | expand |
On Tue, 8 Oct 2024 at 11:34, Thomas Richard <thomas.richard@bootlin.com> wrote: > > If the console suspend is disabled, mark the serial as on the wakeup path > to keep its PM domain powered on. > > Suggested-by: Théo Lebrun <theo.lebrun@bootlin.com> > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> > --- > drivers/tty/serial/8250/8250_omap.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c > index 88b58f44e4e9..6764fe2f9cad 100644 > --- a/drivers/tty/serial/8250/8250_omap.c > +++ b/drivers/tty/serial/8250/8250_omap.c > @@ -1660,8 +1660,12 @@ static int omap8250_suspend(struct device *dev) > if (!device_may_wakeup(dev)) > priv->wer = 0; > serial_out(up, UART_OMAP_WER, priv->wer); > - if (uart_console(&up->port) && console_suspend_enabled) > - err = pm_runtime_force_suspend(dev); > + if (uart_console(&up->port)) { > + if (console_suspend_enabled) > + err = pm_runtime_force_suspend(dev); > + else > + device_set_wakeup_path(dev); > + } Isn't this already managed by the serial core? See commit a47cf07f60dc ("serial: core: Call device_set_awake_path() for console port") [...] Kind regards Uffe
On 10/9/24 12:50, Ulf Hansson wrote: > On Tue, 8 Oct 2024 at 11:34, Thomas Richard <thomas.richard@bootlin.com> wrote: >> >> If the console suspend is disabled, mark the serial as on the wakeup path >> to keep its PM domain powered on. >> >> Suggested-by: Théo Lebrun <theo.lebrun@bootlin.com> >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> >> --- >> drivers/tty/serial/8250/8250_omap.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c >> index 88b58f44e4e9..6764fe2f9cad 100644 >> --- a/drivers/tty/serial/8250/8250_omap.c >> +++ b/drivers/tty/serial/8250/8250_omap.c >> @@ -1660,8 +1660,12 @@ static int omap8250_suspend(struct device *dev) >> if (!device_may_wakeup(dev)) >> priv->wer = 0; >> serial_out(up, UART_OMAP_WER, priv->wer); >> - if (uart_console(&up->port) && console_suspend_enabled) >> - err = pm_runtime_force_suspend(dev); >> + if (uart_console(&up->port)) { >> + if (console_suspend_enabled) >> + err = pm_runtime_force_suspend(dev); >> + else >> + device_set_wakeup_path(dev); >> + } > > Isn't this already managed by the serial core? See commit a47cf07f60dc > ("serial: core: Call device_set_awake_path() for console port") Yes you're right. So this patch is useless. Regards, Thomas
On Wed, 9 Oct 2024 at 14:17, Thomas Richard <thomas.richard@bootlin.com> wrote: > > On 10/9/24 12:50, Ulf Hansson wrote: > > On Tue, 8 Oct 2024 at 11:34, Thomas Richard <thomas.richard@bootlin.com> wrote: > >> > >> If the console suspend is disabled, mark the serial as on the wakeup path > >> to keep its PM domain powered on. > >> > >> Suggested-by: Théo Lebrun <theo.lebrun@bootlin.com> > >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> > >> --- > >> drivers/tty/serial/8250/8250_omap.c | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c > >> index 88b58f44e4e9..6764fe2f9cad 100644 > >> --- a/drivers/tty/serial/8250/8250_omap.c > >> +++ b/drivers/tty/serial/8250/8250_omap.c > >> @@ -1660,8 +1660,12 @@ static int omap8250_suspend(struct device *dev) > >> if (!device_may_wakeup(dev)) > >> priv->wer = 0; > >> serial_out(up, UART_OMAP_WER, priv->wer); > >> - if (uart_console(&up->port) && console_suspend_enabled) > >> - err = pm_runtime_force_suspend(dev); > >> + if (uart_console(&up->port)) { > >> + if (console_suspend_enabled) > >> + err = pm_runtime_force_suspend(dev); > >> + else > >> + device_set_wakeup_path(dev); > >> + } > > > > Isn't this already managed by the serial core? See commit a47cf07f60dc > > ("serial: core: Call device_set_awake_path() for console port") > > Yes you're right. > So this patch is useless. So patch 1/2 is all you need to get things working? Kind regards Uffe
On 10/9/24 14:39, Ulf Hansson wrote: > On Wed, 9 Oct 2024 at 14:17, Thomas Richard <thomas.richard@bootlin.com> wrote: >> >> On 10/9/24 12:50, Ulf Hansson wrote: >>> On Tue, 8 Oct 2024 at 11:34, Thomas Richard <thomas.richard@bootlin.com> wrote: >>>> >>>> If the console suspend is disabled, mark the serial as on the wakeup path >>>> to keep its PM domain powered on. >>>> >>>> Suggested-by: Théo Lebrun <theo.lebrun@bootlin.com> >>>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> >>>> --- >>>> drivers/tty/serial/8250/8250_omap.c | 8 ++++++-- >>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c >>>> index 88b58f44e4e9..6764fe2f9cad 100644 >>>> --- a/drivers/tty/serial/8250/8250_omap.c >>>> +++ b/drivers/tty/serial/8250/8250_omap.c >>>> @@ -1660,8 +1660,12 @@ static int omap8250_suspend(struct device *dev) >>>> if (!device_may_wakeup(dev)) >>>> priv->wer = 0; >>>> serial_out(up, UART_OMAP_WER, priv->wer); >>>> - if (uart_console(&up->port) && console_suspend_enabled) >>>> - err = pm_runtime_force_suspend(dev); >>>> + if (uart_console(&up->port)) { >>>> + if (console_suspend_enabled) >>>> + err = pm_runtime_force_suspend(dev); >>>> + else >>>> + device_set_wakeup_path(dev); >>>> + } >>> >>> Isn't this already managed by the serial core? See commit a47cf07f60dc >>> ("serial: core: Call device_set_awake_path() for console port") >> >> Yes you're right. >> So this patch is useless. > > So patch 1/2 is all you need to get things working? Yes only patch 1/2 is needed. Regards, Thomas
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index 88b58f44e4e9..6764fe2f9cad 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -1660,8 +1660,12 @@ static int omap8250_suspend(struct device *dev) if (!device_may_wakeup(dev)) priv->wer = 0; serial_out(up, UART_OMAP_WER, priv->wer); - if (uart_console(&up->port) && console_suspend_enabled) - err = pm_runtime_force_suspend(dev); + if (uart_console(&up->port)) { + if (console_suspend_enabled) + err = pm_runtime_force_suspend(dev); + else + device_set_wakeup_path(dev); + } flush_work(&priv->qos_work); return err;
If the console suspend is disabled, mark the serial as on the wakeup path to keep its PM domain powered on. Suggested-by: Théo Lebrun <theo.lebrun@bootlin.com> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> --- drivers/tty/serial/8250/8250_omap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)