Re: [PATCH 0/6] tty: serial: propagate errors from uart_ops.pm callback
Praveen Talari <[email protected]> Thu, 9 Jul 2026 14:22:17 +0530
| Newsgroups | org.kernel.vger.linux-omap,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-mediatek,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mips,org.kernel.vger.linux-samsung-soc,org.kernel.vger.linux-serial |
|---|---|
| Message-ID | <[email protected]> |
HI Jiri On 09-07-2026 12:23, Jiri Slaby wrote: > On 09. 07. 26, 8:25, Praveen Talari wrote: >> The uart_ops.pm callback has been declared void since its introduction, >> which means any error from a driver's power management implementation is >> silently discarded by uart_change_pm(). Beyond losing the error >> information, uart_change_pm() unconditionally updates state->pm_state >> even when the underlying hardware transition failed. This causes the >> serial core to track a power state that does not reflect reality: >> subsequent calls to uart_change_pm() see the stale cached state as >> matching the requested state and skip the callback entirely, leaving the >> hardware permanently stuck with no further recovery attempt. >> >> On modern platforms where the .pm callback performs real work — >> enabling clock trees, interacting with runtime PM, asserting voltage >> regulators — this is a correctness gap. Failures are invisible to the >> PM framework, the port proceeds to call ops->startup() on potentially >> unpowered hardware, and suspend/resume errors are hidden from the core >> that needs to handle them. >> >> This series fixes the problem in four steps: >> >> Patch 1 changes the uart_ops.pm callback signature from void to int, >> updates uart_change_pm() to propagate errors and only commit >> state->pm_state on success, and handles the return value at every >> call site in serial_core.c with appropriate policy per context >> (propagate, log, or skip-on-failure). > > So does this break build without the below applied? IOW: breaks > bisectability? You are right, patch 1 alone breaks the build since the driver implementations are still void until patches 2–4. The series as structured is not bisect-safe. Do you have any suggestions on how to fix this issue? > >> Patch 2 updates the 8250 driver family: serial8250_do_pm() and >> serial8250_pm() are updated to return int (with the exported symbol >> declaration updated in serial_8250.h), and the 8250 sub-driver >> pm callbacks are updated to return 0. >> >> Patch 3 updates the remaining non-8250 serial drivers. All .pm >> implementations are updated to return 0. The sh-sci forward >> declaration shared with rsci is also updated. >> >> Patch 4 updates arch-level implementations: SA1100 (assabet, h3xxx), >> OMAP1/ams-delta (modem_pm, now propagates regulator errors), and >> MIPS/Alchemy (alchemy_8250_pm). >> >> All existing .pm implementations return 0, so there is no functional >> change for any current driver. The series purely adds the infrastructure >> for drivers to report errors going forward, with the serial core ready >> to handle them correctly. > > OK, now I miss the rationale behind the patchset. Neither there is a > possible code path to actually test this? The rationale is that qcom_geni_serial_pm() calls pm_runtime_resume_and_get() which can fail, but its return value is currently discarded because the callback is void. Patch 6 in this series is the concrete user: it makes qcom_geni_serial_pm() propagate the pm_runtime_resume_and_get() error so that a failure to resume the UART power domain is visible at uart_port_startup() time rather than silently proceeding to call ops->startup() on an unpowered port. A code path to test: on qcom platforms with CONFIG_SERIAL_QCOM_GENI enabled, if pm_runtime_resume_and_get() fails during uart_configure_port() or uart_port_startup(), the error now propagates to the caller instead of being dropped. The test case from development was injecting a failure in qcom_geni_serial_pm() and observing that uart_add_one_port() returns an error rather than proceeding silently. Thanks, Praveen Talari > > thanks,