Regression: serial: 8250_omap: error during suspend if no_console_suspend is set

Regression: serial: 8250_omap: error during suspend if no_console_suspend is set

[Thomas Richard]

Hi

After switching to Linux 6.6-rc1, i met an issue during suspend to idle
with 8250_omap driver (no_console_suspend is set).
The driver fails to suspend the uart port used for the serial console so
the suspend sequence is stopped.

[  114.629197] port 2800000.serial:0.0: PM: calling
pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2800000.serial:0
[  114.639617] port 2800000.serial:0.0: PM:
pm_runtime_force_suspend+0x0/0x134 returned 0 after 2 usecs
[  114.648739] omap8250 2800000.serial: PM: calling
omap8250_suspend+0x0/0xf4 @ 114, parent: bus@100000
[  114.657861] omap8250 2800000.serial: PM: dpm_run_callback():
omap8250_suspend+0x0/0xf4 returns -16
[  114.666808] omap8250 2800000.serial: PM: omap8250_suspend+0x0/0xf4
returned -16 after 8951 usecs
[  114.675580] omap8250 2800000.serial: PM: failed to suspend: error -16
[  114.682011] PM: suspend of devices aborted after 675.644 msecs
[  114.687833] PM: start suspend of devices aborted after 681.777 msecs
[  114.694175] PM: Some devices failed to suspend, or early wake event
detected

The following sequence is used to suspend to idle:
$ echo 1 > /sys/power/pm_debug_messages
$ echo 1 > /sys/power/pm_print_times
$ echo 8 > /proc/sys/kernel/printk
$ echo 0 > /sys/module/printk/parameters/console_suspend
$ echo enabled >
/sys/devices/platform/bus@100000/2800000.serial/tty/ttyS2/power/wakeup
$ echo s2idle > /sys/power/mem_sleep
$ echo mem > /sys/power/state

The regression was introduced in commit 20a41a62618d "serial: 8250_omap:
Use force_suspend and resume for system suspend"

Before commit 20a41a62618d, omap8250_suspend returned 0.
Now pm_runtime_force_suspend is called and its return code is used by
omap8250_suspend.

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

	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);
	err = pm_runtime_force_suspend(dev);
	flush_work(&priv->qos_work);

	return err;
}

The pm_runtime_force_suspend function calls omap8250_runtime_suspend
which returns -EBUSY because console suspend was disabled (which is my
case), as explained in the code.

/*
 * When using 'no_console_suspend', the console UART must not be
 * suspended. Since driver suspend is managed by runtime suspend,
 * preventing runtime suspend (by returning error) will keep device
 * active during suspend.
 */
if (priv->is_suspending && !console_suspend_enabled) {
	if (up && uart_console(&up->port))
		return -EBUSY;
}

The port is used by the console, so we don't want to suspend it (console
suspend was disabled).
Of course, if console_suspend is enabled and messages are disabled there
is no issue.

Best Regards,

Re: Regression: serial: 8250_omap: error during suspend if no_console_suspend is set

[Greg KH]

On Thu, Sep 14, 2023 at 06:26:44PM +0200, Thomas Richard wrote:
> Hi
> 
> After switching to Linux 6.6-rc1, i met an issue during suspend to idle
> with 8250_omap driver (no_console_suspend is set).
> The driver fails to suspend the uart port used for the serial console so
> the suspend sequence is stopped.
> 
> [  114.629197] port 2800000.serial:0.0: PM: calling
> pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2800000.serial:0
> [  114.639617] port 2800000.serial:0.0: PM:
> pm_runtime_force_suspend+0x0/0x134 returned 0 after 2 usecs
> [  114.648739] omap8250 2800000.serial: PM: calling
> omap8250_suspend+0x0/0xf4 @ 114, parent: bus@100000
> [  114.657861] omap8250 2800000.serial: PM: dpm_run_callback():
> omap8250_suspend+0x0/0xf4 returns -16
> [  114.666808] omap8250 2800000.serial: PM: omap8250_suspend+0x0/0xf4
> returned -16 after 8951 usecs
> [  114.675580] omap8250 2800000.serial: PM: failed to suspend: error -16
> [  114.682011] PM: suspend of devices aborted after 675.644 msecs
> [  114.687833] PM: start suspend of devices aborted after 681.777 msecs
> [  114.694175] PM: Some devices failed to suspend, or early wake event
> detected
> 
> The following sequence is used to suspend to idle:
> $ echo 1 > /sys/power/pm_debug_messages
> $ echo 1 > /sys/power/pm_print_times
> $ echo 8 > /proc/sys/kernel/printk
> $ echo 0 > /sys/module/printk/parameters/console_suspend
> $ echo enabled >
> /sys/devices/platform/bus@100000/2800000.serial/tty/ttyS2/power/wakeup
> $ echo s2idle > /sys/power/mem_sleep
> $ echo mem > /sys/power/state
> 
> The regression was introduced in commit 20a41a62618d "serial: 8250_omap:
> Use force_suspend and resume for system suspend"
> 
> Before commit 20a41a62618d, omap8250_suspend returned 0.
> Now pm_runtime_force_suspend is called and its return code is used by
> omap8250_suspend.
> 
> 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);
> 
> 	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);
> 	err = pm_runtime_force_suspend(dev);
> 	flush_work(&priv->qos_work);
> 
> 	return err;
> }
> 
> The pm_runtime_force_suspend function calls omap8250_runtime_suspend
> which returns -EBUSY because console suspend was disabled (which is my
> case), as explained in the code.
> 
> /*
>  * When using 'no_console_suspend', the console UART must not be
>  * suspended. Since driver suspend is managed by runtime suspend,
>  * preventing runtime suspend (by returning error) will keep device
>  * active during suspend.
>  */
> if (priv->is_suspending && !console_suspend_enabled) {
> 	if (up && uart_console(&up->port))
> 		return -EBUSY;
> }
> 
> The port is used by the console, so we don't want to suspend it (console
> suspend was disabled).
> Of course, if console_suspend is enabled and messages are disabled there
> is no issue.

Do you have a proposed patch to fix this?  Or should the original be
reverted?

thanks,

greg k-h

Re: Regression: serial: 8250_omap: error during suspend if no_console_suspend is set

[Thomas Richard]

On 9/18/23 09:35, Greg KH wrote:
> On Thu, Sep 14, 2023 at 06:26:44PM +0200, Thomas Richard wrote:
>> Hi
>>
>> After switching to Linux 6.6-rc1, i met an issue during suspend to idle
>> with 8250_omap driver (no_console_suspend is set).
>> The driver fails to suspend the uart port used for the serial console so
>> the suspend sequence is stopped.
>>
>> [  114.629197] port 2800000.serial:0.0: PM: calling
>> pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2800000.serial:0
>> [  114.639617] port 2800000.serial:0.0: PM:
>> pm_runtime_force_suspend+0x0/0x134 returned 0 after 2 usecs
>> [  114.648739] omap8250 2800000.serial: PM: calling
>> omap8250_suspend+0x0/0xf4 @ 114, parent: bus@100000
>> [  114.657861] omap8250 2800000.serial: PM: dpm_run_callback():
>> omap8250_suspend+0x0/0xf4 returns -16
>> [  114.666808] omap8250 2800000.serial: PM: omap8250_suspend+0x0/0xf4
>> returned -16 after 8951 usecs
>> [  114.675580] omap8250 2800000.serial: PM: failed to suspend: error -16
>> [  114.682011] PM: suspend of devices aborted after 675.644 msecs
>> [  114.687833] PM: start suspend of devices aborted after 681.777 msecs
>> [  114.694175] PM: Some devices failed to suspend, or early wake event
>> detected
>>
>> The following sequence is used to suspend to idle:
>> $ echo 1 > /sys/power/pm_debug_messages
>> $ echo 1 > /sys/power/pm_print_times
>> $ echo 8 > /proc/sys/kernel/printk
>> $ echo 0 > /sys/module/printk/parameters/console_suspend
>> $ echo enabled >
>> /sys/devices/platform/bus@100000/2800000.serial/tty/ttyS2/power/wakeup
>> $ echo s2idle > /sys/power/mem_sleep
>> $ echo mem > /sys/power/state
>>
>> The regression was introduced in commit 20a41a62618d "serial: 8250_omap:
>> Use force_suspend and resume for system suspend"
>>
>> Before commit 20a41a62618d, omap8250_suspend returned 0.
>> Now pm_runtime_force_suspend is called and its return code is used by
>> omap8250_suspend.
>>
>> 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);
>>
>> 	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);
>> 	err = pm_runtime_force_suspend(dev);
>> 	flush_work(&priv->qos_work);
>>
>> 	return err;
>> }
>>
>> The pm_runtime_force_suspend function calls omap8250_runtime_suspend
>> which returns -EBUSY because console suspend was disabled (which is my
>> case), as explained in the code.
>>
>> /*
>>  * When using 'no_console_suspend', the console UART must not be
>>  * suspended. Since driver suspend is managed by runtime suspend,
>>  * preventing runtime suspend (by returning error) will keep device
>>  * active during suspend.
>>  */
>> if (priv->is_suspending && !console_suspend_enabled) {
>> 	if (up && uart_console(&up->port))
>> 		return -EBUSY;
>> }
>>
>> The port is used by the console, so we don't want to suspend it (console
>> suspend was disabled).
>> Of course, if console_suspend is enabled and messages are disabled there
>> is no issue.
> 
> Do you have a proposed patch to fix this?  Or should the original be
> reverted?

I created a new thread including this issue and a PM issue
(https://lore.kernel.org/all/59b13c93-6637-3050-c145-31be0d6c12c9@bootlin.com/).

Regards,

Thomas Richard

> 
> thanks,
> 
> greg k-h

Re: Regression: serial: 8250_omap: error during suspend if no_console_suspend is set

[Linux regression tracking #adding (Thorsten Leemhuis)]

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

[Note: part of this is now handled here:
https://lore.kernel.org/all/59b13c93-6637-3050-c145-31be0d6c12c9@bootlin.com/]

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.]

On 14.09.23 18:26, Thomas Richard wrote:
> 
> After switching to Linux 6.6-rc1, i met an issue during suspend to idle
> with 8250_omap driver (no_console_suspend is set).
> The driver fails to suspend the uart port used for the serial console so
> the suspend sequence is stopped.
> 
> [  114.629197] port 2800000.serial:0.0: PM: calling
> pm_runtime_force_suspend+0x0/0x134 @ 114, parent: 2800000.serial:0
> [  114.639617] port 2800000.serial:0.0: PM:
> pm_runtime_force_suspend+0x0/0x134 returned 0 after 2 usecs
> [  114.648739] omap8250 2800000.serial: PM: calling
> omap8250_suspend+0x0/0xf4 @ 114, parent: bus@100000
> [  114.657861] omap8250 2800000.serial: PM: dpm_run_callback():
> omap8250_suspend+0x0/0xf4 returns -16
> [  114.666808] omap8250 2800000.serial: PM: omap8250_suspend+0x0/0xf4
> returned -16 after 8951 usecs
> [  114.675580] omap8250 2800000.serial: PM: failed to suspend: error -16
> [  114.682011] PM: suspend of devices aborted after 675.644 msecs
> [  114.687833] PM: start suspend of devices aborted after 681.777 msecs
> [  114.694175] PM: Some devices failed to suspend, or early wake event
> detected
> 
> The following sequence is used to suspend to idle:
> $ echo 1 > /sys/power/pm_debug_messages
> $ echo 1 > /sys/power/pm_print_times
> $ echo 8 > /proc/sys/kernel/printk
> $ echo 0 > /sys/module/printk/parameters/console_suspend
> $ echo enabled >
> /sys/devices/platform/bus@100000/2800000.serial/tty/ttyS2/power/wakeup
> $ echo s2idle > /sys/power/mem_sleep
> $ echo mem > /sys/power/state
> 
> The regression was introduced in commit 20a41a62618d "serial: 8250_omap:
> Use force_suspend and resume for system suspend"
> 
> Before commit 20a41a62618d, omap8250_suspend returned 0.
> Now pm_runtime_force_suspend is called and its return code is used by
> omap8250_suspend.
> 
> 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);
> 
> 	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);
> 	err = pm_runtime_force_suspend(dev);
> 	flush_work(&priv->qos_work);
> 
> 	return err;
> }
> 
> The pm_runtime_force_suspend function calls omap8250_runtime_suspend
> which returns -EBUSY because console suspend was disabled (which is my
> case), as explained in the code.
> 
> /*
>  * When using 'no_console_suspend', the console UART must not be
>  * suspended. Since driver suspend is managed by runtime suspend,
>  * preventing runtime suspend (by returning error) will keep device
>  * active during suspend.
>  */
> if (priv->is_suspending && !console_suspend_enabled) {
> 	if (up && uart_console(&up->port))
> 		return -EBUSY;
> }
> 
> The port is used by the console, so we don't want to suspend it (console
> suspend was disabled).
> Of course, if console_suspend is enabled and messages are disabled there
> is no issue.


Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 20a41a62618d
#regzbot title serial: 8250_omap: suspend issue with console_suspend
disabled
#regzbot monitor:
https://lore.kernel.org/all/59b13c93-6637-3050-c145-31be0d6c12c9@bootlin.com/
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

Re: Regression: serial: 8250_omap: error during suspend if no_console_suspend is set

[Linux regression tracking #update (Thorsten Leemhuis)]

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 20.09.23 16:36, Linux regression tracking #adding (Thorsten Leemhuis)
wrote:
> On 14.09.23 18:26, Thomas Richard wrote:
>>
>> After switching to Linux 6.6-rc1, i met an issue during suspend to idle
>> with 8250_omap driver (no_console_suspend is set).
>> The driver fails to suspend the uart port used for the serial console so
>> the suspend sequence is stopped.

#regzbot fix: 560706eff7c8e5621b0
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.