[PATCH 1/6] tty: serial: change uart_ops.pm callback to return int
Praveen Talari
praveen.talari at oss.qualcomm.com
Wed Jul 8 23:25:13 PDT 2026
The uart_ops.pm callback is currently declared void, causing
uart_change_pm() to silently discard any error from a driver's power
management implementation. Worse, state->pm_state is unconditionally
updated even when the hardware transition failed, causing the serial core
to track a power state that does not reflect reality. Subsequent calls to
uart_change_pm() will then see the (stale) state as matching and skip the
callback entirely, leaving the hardware stuck in the wrong state with no
further recovery attempt.
Change the uart_ops.pm callback signature from void to int. Update
uart_change_pm() to propagate the driver's return value and only commit
state->pm_state on success, preserving consistency between software state
and hardware state across all transitions.
Update all call sites in serial_core.c:
uart_port_startup(): propagate the error so that port open fails
cleanly if the hardware cannot be powered on, rather than proceeding
to call ops->startup() on an unpowered port.
uart_suspend_port(): return the error directly so the PM framework
is aware of the failed transition and can react accordingly.
uart_resume_port(): propagate on both the console-resume and the
suspended-port-restore paths so a failed power-on is not hidden
from the PM core.
uart_configure_port(): log and return early if power-on fails at
probe time; log a warning if power-down fails after configuration,
since the port is already registered at that point.
uart_tty_port_shutdown(), uart_hangup(): log via dev_err() since
these are void paths where propagation is not meaningful; teardown
continues regardless.
uart_line_info(): skip the modem-control status read if power-on
fails to avoid accessing an unpowered port; log a warning if the
original power state cannot be restored afterward.
uart_poll_init(): propagate the power-on error; log a warning if
the saved power state cannot be restored after a failed poll init.
Also update the uart_port.pm field (used by 8250 sub-drivers that
install a per-port pm function pointer directly) and its kernel-doc to
the new int-returning signature.
No functional change for any existing driver since all current .pm
implementations will be updated to return 0.
Signed-off-by: Praveen Talari <praveen.talari at oss.qualcomm.com>
---
drivers/tty/serial/serial_core.c | 89 ++++++++++++++++++++++++++++------------
include/linux/serial_core.h | 10 +++--
2 files changed, 69 insertions(+), 30 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a530ad372b43..e624a67a9395 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -52,8 +52,8 @@ static struct lock_class_key port_lock_key;
*/
#define RS485_MAX_RTS_DELAY 100 /* msecs */
-static void uart_change_pm(struct uart_state *state,
- enum uart_pm_state pm_state);
+static int uart_change_pm(struct uart_state *state,
+ enum uart_pm_state pm_state);
static void uart_port_shutdown(struct tty_port *port);
@@ -312,7 +312,9 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
/*
* Make sure the device is in D0 state.
*/
- uart_change_pm(state, UART_PM_STATE_ON);
+ retval = uart_change_pm(state, UART_PM_STATE_ON);
+ if (retval)
+ return retval;
retval = uart_alloc_xmit_buf(&state->port);
if (retval)
@@ -1741,7 +1743,8 @@ static void uart_tty_port_shutdown(struct tty_port *port)
uart_free_xmit_buf(port);
- uart_change_pm(state, UART_PM_STATE_OFF);
+ if (uart_change_pm(state, UART_PM_STATE_OFF))
+ dev_err(uport->dev, "failed to set power state off on shutdown\n");
}
static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
@@ -1831,8 +1834,13 @@ static void uart_hangup(struct tty_struct *tty)
port->count = 0;
tty_port_set_active(port, false);
tty_port_tty_set(port, NULL);
- if (uport && !uart_console(uport))
- uart_change_pm(state, UART_PM_STATE_OFF);
+ if (uport && !uart_console(uport)) {
+ int ret = uart_change_pm(state, UART_PM_STATE_OFF);
+
+ if (ret)
+ dev_err(uport->dev,
+ "failed to set power state off on hangup\n");
+ }
wake_up_interruptible(&port->open_wait);
wake_up_interruptible(&port->delta_msr_wait);
}
@@ -1994,12 +2002,17 @@ static void uart_line_info(struct seq_file *m, struct uart_state *state)
if (capable(CAP_SYS_ADMIN)) {
pm_state = state->pm_state;
- if (pm_state != UART_PM_STATE_ON)
- uart_change_pm(state, UART_PM_STATE_ON);
+ if (pm_state != UART_PM_STATE_ON) {
+ if (uart_change_pm(state, UART_PM_STATE_ON))
+ goto line_info_end;
+ }
scoped_guard(uart_port_lock_irq, uport)
status = uport->ops->get_mctrl(uport);
- if (pm_state != UART_PM_STATE_ON)
- uart_change_pm(state, pm_state);
+ if (pm_state != UART_PM_STATE_ON) {
+ if (uart_change_pm(state, pm_state))
+ dev_err(uport->dev,
+ "failed to restore power state after line info\n");
+ }
seq_printf(m, " tx:%u rx:%u",
uport->icount.tx, uport->icount.rx);
@@ -2036,6 +2049,7 @@ static void uart_line_info(struct seq_file *m, struct uart_state *state)
seq_puts(m, stat_buf);
}
+line_info_end:
seq_putc(m, '\n');
#undef STATBIT
#undef INFOBIT
@@ -2265,17 +2279,24 @@ EXPORT_SYMBOL_GPL(uart_set_options);
* @pm_state: new state
*
* Locking: port->mutex has to be held
+ *
+ * Returns 0 on success, negative error code on failure.
*/
-static void uart_change_pm(struct uart_state *state,
- enum uart_pm_state pm_state)
+static int uart_change_pm(struct uart_state *state,
+ enum uart_pm_state pm_state)
{
struct uart_port *port = uart_port_check(state);
if (state->pm_state != pm_state) {
- if (port && port->ops->pm)
- port->ops->pm(port, pm_state, state->pm_state);
+ if (port && port->ops->pm) {
+ int ret = port->ops->pm(port, pm_state, state->pm_state);
+
+ if (ret)
+ return ret;
+ }
state->pm_state = pm_state;
}
+ return 0;
}
struct uart_match {
@@ -2364,9 +2385,7 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
if (uart_console(uport))
console_suspend(uport->cons);
- uart_change_pm(state, UART_PM_STATE_OFF);
-
- return 0;
+ return uart_change_pm(state, UART_PM_STATE_OFF);
}
EXPORT_SYMBOL(uart_suspend_port);
@@ -2408,8 +2427,12 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
if (port->tty && termios.c_cflag == 0)
termios = port->tty->termios;
- if (console_suspend_enabled)
- uart_change_pm(state, UART_PM_STATE_ON);
+ if (console_suspend_enabled) {
+ int ret = uart_change_pm(state, UART_PM_STATE_ON);
+
+ if (ret)
+ return ret;
+ }
uport->ops->set_termios(uport, &termios, NULL);
if (!console_suspend_enabled && uport->ops->start_rx) {
guard(uart_port_lock_irq)(uport);
@@ -2423,7 +2446,9 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
const struct uart_ops *ops = uport->ops;
int ret;
- uart_change_pm(state, UART_PM_STATE_ON);
+ ret = uart_change_pm(state, UART_PM_STATE_ON);
+ if (ret)
+ return ret;
scoped_guard(uart_port_lock_irq, uport)
if (!(uport->rs485.flags & SER_RS485_ENABLED))
ops->set_mctrl(uport, 0);
@@ -2541,7 +2566,12 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
console_lock();
/* Power up port for set_mctrl() */
- uart_change_pm(state, UART_PM_STATE_ON);
+ if (uart_change_pm(state, UART_PM_STATE_ON)) {
+ dev_err(port->dev, "failed to power up port\n");
+ if (uart_console(port))
+ console_unlock();
+ return;
+ }
/*
* Ensure that the modem control lines are de-activated.
@@ -2578,8 +2608,10 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
* Power down all ports by default, except the
* console if we have one.
*/
- if (!uart_console(port))
- uart_change_pm(state, UART_PM_STATE_OFF);
+ if (!uart_console(port)) {
+ if (uart_change_pm(state, UART_PM_STATE_OFF))
+ dev_err(port->dev, "failed to power down port\n");
+ }
}
}
@@ -2608,7 +2640,9 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
return -1;
pm_state = state->pm_state;
- uart_change_pm(state, UART_PM_STATE_ON);
+ ret = uart_change_pm(state, UART_PM_STATE_ON);
+ if (ret)
+ return ret;
if (port->ops->poll_init) {
/*
@@ -2626,8 +2660,11 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
console_list_unlock();
}
- if (ret)
- uart_change_pm(state, pm_state);
+ if (ret) {
+ if (uart_change_pm(state, pm_state))
+ dev_err(port->dev,
+ "failed to restore power state after poll init failure\n");
+ }
return ret;
}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index bdc214386e4a..c82839028220 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -269,8 +269,8 @@ struct gpio_desc;
*
* Locking: caller holds tty_port->mutex
*
- * @pm: ``void ()(struct uart_port *port, unsigned int state,
- * unsigned int oldstate)``
+ * @pm: ``int ()(struct uart_port *port, unsigned int state,
+ * unsigned int oldstate)``
*
* Perform any power management related activities on the specified @port.
* @state indicates the new state (defined by enum uart_pm_state),
@@ -282,6 +282,8 @@ struct gpio_desc;
* closed, except when the @port is also the system console. This will
* occur even if %CONFIG_PM is not set.
*
+ * Returns 0 on success, negative error code on failure.
+ *
* Locking: none.
* Interrupts: caller dependent.
*
@@ -391,7 +393,7 @@ struct uart_ops {
void (*set_termios)(struct uart_port *, struct ktermios *new,
const struct ktermios *old);
void (*set_ldisc)(struct uart_port *, struct ktermios *);
- void (*pm)(struct uart_port *, unsigned int state,
+ int (*pm)(struct uart_port *port, unsigned int state,
unsigned int oldstate);
const char *(*type)(struct uart_port *);
void (*release_port)(struct uart_port *);
@@ -464,7 +466,7 @@ struct uart_port {
void (*throttle)(struct uart_port *port);
void (*unthrottle)(struct uart_port *port);
int (*handle_irq)(struct uart_port *);
- void (*pm)(struct uart_port *, unsigned int state,
+ int (*pm)(struct uart_port *port, unsigned int state,
unsigned int old);
void (*handle_break)(struct uart_port *);
int (*rs485_config)(struct uart_port *,
--
2.34.1
More information about the Linux-mediatek
mailing list