Messages in this thread Patch in this message | | | Date | Tue, 3 Oct 2023 13:57:33 +0200 | Subject | Re: [PATCH v12 1/1] serial: core: Start managing serial controllers to enable runtime PM | From | Maximilian Luz <> |
| |
On 5/25/23 13:30, Tony Lindgren wrote: > We want to enable runtime PM for serial port device drivers in a generic > way. To do this, we want to have the serial core layer manage the > registered physical serial controller devices. > > To manage serial controllers, let's set up a struct bus and struct device > for the serial core controller as suggested by Greg and Jiri. The serial > core controller devices are children of the physical serial port device. > The serial core controller device is needed to support multiple different > kind of ports connected to single physical serial port device. > > Let's also set up a struct device for the serial core port. The serial > core port instances are children of the serial core controller device. > > With the serial core port device we can now flush pending TX on the > runtime PM resume as suggested by Johan.
Hi,
Unfortunately, this patch (now commit 84a9582fd203 with v6.5) breaks suspend on a bunch of Microsoft Surface devices (confirmed via git bisect).
The root cause is that when we enter system suspend with the serial port in runtime suspend, all transfers will be paused until the serial port is runtime-resumed, which will only happen after complete() is called, so basically after we are done resuming the system. In short: This patch essentially blocks all serial communication in system suspend transitions.
The affected devices have an EC (the Surface Aggregator Module) which needs some communication when entering system suspend. In particular, we need to tell it to stop sending us events, turn off the keyboard backlight, and transition it to a lower power mode. With this patch, all of these operations now time out, preventing us from entering suspend.
A bad workaround is to disable runtime PM, e.g. via
echo on > /sys/bus/serial-base/devices/dw-apb-uart.4:0/dw-apb-uart.4:0.0/power/control
or the diff attached below, but obviously that's not a great solution and can be broken quite easily from userspace in the same way (and without users really actively doing so through tools like TLP).
Any ideas on how this can be fixed without reverting?
See also https://github.com/linux-surface/linux-surface/issues/1258.
Regards, Max
--- drivers/tty/serial/serial_port.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c index 862423237007..6ceacea5e790 100644 --- a/drivers/tty/serial/serial_port.c +++ b/drivers/tty/serial/serial_port.c @@ -55,6 +55,8 @@ static int serial_port_probe(struct device *dev) pm_runtime_set_autosuspend_delay(dev, SERIAL_PORT_AUTOSUSPEND_DELAY_MS); pm_runtime_use_autosuspend(dev); + pm_runtime_forbid(dev); + return 0; } -- 2.42.0
| |