From: Praveen Talari <praveen.talari@oss.qualcomm.com>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Jiri Slaby" <jirislaby@kernel.org>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"AngeloGioacchino Del Regno"
<angelogioacchino.delregno@collabora.com>,
"Richard Genoud" <richard.genoud@bootlin.com>,
"Nicolas Ferre" <nicolas.ferre@microchip.com>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
"Krzysztof Kozlowski" <krzk@kernel.org>,
"Peter Griffin" <peter.griffin@linaro.org>,
"Alim Akhtar" <alim.akhtar@samsung.com>,
"Orson Zhai" <orsonzhai@gmail.com>,
"Baolin Wang" <baolin.wang@linux.alibaba.com>,
"Chunyan Zhang" <zhang.lyra@gmail.com>,
"Patrice Chotard" <patrice.chotard@foss.st.com>,
"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
"Peter Korsgaard" <jacmet@sunsite.dk>,
"Michal Simek" <michal.simek@amd.com>,
"Aaro Koskinen" <aaro.koskinen@iki.fi>,
"Janusz Krzysztofik" <jmkrzyszt@gmail.com>,
"Tony Lindgren" <tony@atomide.com>,
"Russell King" <linux@armlinux.org.uk>,
"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
bjorn.andersson@oss.qualcomm.com,
"Konrad Dybcio" <konrad.dybcio@oss.qualcomm.com>
Cc: linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-omap@vger.kernel.org, linux-mips@vger.kernel.org,
Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>,
aniket.randive@oss.qualcomm.com,
chandana.chiluveru@oss.qualcomm.com,
Praveen Talari <praveen.talari@oss.qualcomm.com>
Subject: [PATCH 1/6] tty: serial: change uart_ops.pm callback to return int
Date: Thu, 09 Jul 2026 11:55:13 +0530 [thread overview]
Message-ID: <20260709-add_return_check_for_uart_change_pm-v1-1-e85c6ffa8ec4@oss.qualcomm.com> (raw)
In-Reply-To: <20260709-add_return_check_for_uart_change_pm-v1-0-e85c6ffa8ec4@oss.qualcomm.com>
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@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
next prev parent reply other threads:[~2026-07-09 6:25 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-09 6:25 [PATCH 0/6] tty: serial: propagate errors from uart_ops.pm callback Praveen Talari
2026-07-09 6:25 ` Praveen Talari [this message]
2026-07-09 6:25 ` [PATCH 2/6] serial: 8250: update .pm callbacks to return int Praveen Talari
2026-07-09 6:25 ` [PATCH 3/6] tty: serial: " Praveen Talari
2026-07-09 6:25 ` [PATCH 4/6] arch: update uart pm " Praveen Talari
2026-07-09 6:25 ` [PATCH 5/6] tty: serial: propagate uart_configure_port failure to uart_add_one_port Praveen Talari
2026-07-09 6:25 ` [PATCH 6/6] serial: qcom-geni: check return value of pm_runtime_resume_and_get() Praveen Talari
2026-07-09 6:53 ` [PATCH 0/6] tty: serial: propagate errors from uart_ops.pm callback Jiri Slaby
2026-07-09 8:52 ` Praveen Talari
2026-07-09 10:16 ` Andy Shevchenko
2026-07-09 12:45 ` Praveen Talari
2026-07-09 7:31 ` Andy Shevchenko
2026-07-10 4:37 ` Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260709-add_return_check_for_uart_change_pm-v1-1-e85c6ffa8ec4@oss.qualcomm.com \
--to=praveen.talari@oss.qualcomm.com \
--cc=aaro.koskinen@iki.fi \
--cc=alexandre.belloni@bootlin.com \
--cc=alexandre.torgue@foss.st.com \
--cc=alim.akhtar@samsung.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=aniket.randive@oss.qualcomm.com \
--cc=baolin.wang@linux.alibaba.com \
--cc=bjorn.andersson@oss.qualcomm.com \
--cc=chandana.chiluveru@oss.qualcomm.com \
--cc=claudiu.beznea@tuxon.dev \
--cc=gregkh@linuxfoundation.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jacmet@sunsite.dk \
--cc=jirislaby@kernel.org \
--cc=jmkrzyszt@gmail.com \
--cc=konrad.dybcio@oss.qualcomm.com \
--cc=krzk@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux@armlinux.org.uk \
--cc=matthias.bgg@gmail.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=michal.simek@amd.com \
--cc=mukesh.savaliya@oss.qualcomm.com \
--cc=nicolas.ferre@microchip.com \
--cc=orsonzhai@gmail.com \
--cc=patrice.chotard@foss.st.com \
--cc=peter.griffin@linaro.org \
--cc=richard.genoud@bootlin.com \
--cc=tony@atomide.com \
--cc=tsbogend@alpha.franken.de \
--cc=zhang.lyra@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox