[PATCH v2] serial: qcom-geni: Set the serial engine up again after hibernation

[PATCH v2] serial: qcom-geni: Set the serial engine up again after hibernation

[Birk Skyum]

A hibernation image is restored after a power cycle, which resets every
serial engine. The kernel that loads the image only sets up the engines
it uses itself, and on laptops the Bluetooth UART is opened from the
root file system, so that kernel never touches it.

The restored kernel still has port->setup set from before, so the
startup in uart_resume_port() skips qcom_geni_serial_port_setup() and
the port runs on an engine whose FIFOs, packing and transfer mode were
never configured. On a Lenovo Yoga Slim 7x (X1E80100) the WCN7850's
Bluetooth controller then never comes back: every setup attempt of
hci_qca's error recovery ends in "command 0xfc00 tx timeout" and
"Reading QCA version information failed (-110)".

Clear port->setup in a restore callback of its own, so the next startup
sets the engine up again. Leave console ports alone: the kernel that
loads the image sets up its console when it registers it, and setting
the engine up again in qcom_geni_serial_startup() would run without the
port lock while the console may be writing. The other system sleep
callbacks are unchanged.

Signed-off-by: Birk Skyum <birk.skyum@pm.me>
---
Changes in v2:
- Leave console ports alone. Clearing port->setup on one made the next
  startup run qcom_geni_serial_port_setup() without the port lock,
  which can race with console output (Sashiko).
- v1: https://lore.kernel.org/all/20261027203136.95971-1-birk.skyum@pm.me/

The change from v1 only affects console ports, which this laptop does
not have, so the v1 test still applies. It ran on 7.3 with the other
hibernation fixes for this laptop that are in review (pinctrl, PCI,
smp2p and hci_qca, linked in v1), on one build with a switch for this
change, each a real hibernation with a power-off from a fresh boot:

  without   restored after 112 s, Bluetooth never comes back
  with      restored after 89 s, hci_qca's recovery sets the controller
            up about 18 s after the restore and a scan finds devices

A console that the kernel loading the image did not set up stays as
broken as it is without this patch. Setting one up under the port lock
before uart_resume_port() would be a separate change, and this laptop
has no serial console to test it on.

Built with W=1 and sparse on tty-next with hibernation on, with
hibernation off, and without system sleep.

 drivers/tty/serial/qcom_geni_serial.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index bfdc155c1..5a3bf51a3 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -2131,6 +2131,22 @@ static int qcom_geni_serial_resume(struct device *dev)
 	return ret;
 }
 
+/*
+ * A hibernation image is restored after a power cycle, which reset the serial
+ * engine. The kernel that loaded the image only set up the engines it used
+ * itself, so set this one up again when the port starts. A console is left as
+ * it is, since setting it up at startup would race with console output.
+ */
+static int qcom_geni_serial_restore(struct device *dev)
+{
+	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
+
+	if (!uart_console(&port->uport))
+		port->setup = false;
+
+	return qcom_geni_serial_resume(dev);
+}
+
 #if IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE)
 static const struct qcom_geni_device_data qcom_geni_console_data = {
 	.console = true,
@@ -2168,7 +2184,12 @@ static const struct qcom_geni_device_data sa8255p_qcom_geni_uart_data = {
 static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
 	SET_RUNTIME_PM_OPS(qcom_geni_serial_runtime_suspend,
 			   qcom_geni_serial_runtime_resume, NULL)
-	SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_suspend, qcom_geni_serial_resume)
+	.suspend = pm_sleep_ptr(qcom_geni_serial_suspend),
+	.resume = pm_sleep_ptr(qcom_geni_serial_resume),
+	.freeze = pm_sleep_ptr(qcom_geni_serial_suspend),
+	.thaw = pm_sleep_ptr(qcom_geni_serial_resume),
+	.poweroff = pm_sleep_ptr(qcom_geni_serial_suspend),
+	.restore = pm_sleep_ptr(qcom_geni_serial_restore),
 };
 
 static void qcom_geni_serial_sys_shutdown(struct platform_device *pdev)
-- 
2.56.0

Re: [PATCH v2] serial: qcom-geni: Set the serial engine up again after hibernation

[sashiko-bot]

> A hibernation image is restored after a power cycle, which resets every
> serial engine. The kernel that loads the image only sets up the engines
> it uses itself, and on laptops the Bluetooth UART is opened from the
> root file system, so that kernel never touches it.
> 
> The restored kernel still has port->setup set from before, so the
> startup in uart_resume_port() skips qcom_geni_serial_port_setup() and
> the port runs on an engine whose FIFOs, packing and transfer mode were
> never configured. On a Lenovo Yoga Slim 7x (X1E80100) the WCN7850's
> Bluetooth controller then never comes back: every setup attempt of
> hci_qca's error recovery ends in "command 0xfc00 tx timeout" and
> "Reading QCA version information failed (-110)".
> 
> Clear port->setup in a restore callback of its own, so the next startup
> sets the engine up again. Leave console ports alone: the kernel that
> loads the image sets up its console when it registers it, and setting
> the engine up again in qcom_geni_serial_startup() would run without the
> port lock while the console may be writing. The other system sleep
> callbacks are unchanged.
> 
> Signed-off-by: Birk Skyum <birk.skyum@pm.me>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027224800.86418-1-birk.skyum@pm.me?part=1