[PATCH] serial: 8250_omap: fix wake irq cleared during suspend

[PATCH] serial: 8250_omap: fix wake irq cleared during suspend

[Kendall Willis]

The wake irq was cleared in shutdown(), which runs during the suspend
sequence, making it impossible to wake the system via UART.
Move wake irq setup to probe() and teardown to remove() so the irq
remains armed during suspend when the UART is a wakeup source.

Cc: stable@vger.kernel.org
Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
Signed-off-by: Kendall Willis <k-willis@ti.com>
---
 drivers/tty/serial/8250/8250_omap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 3c7775df27effd309419b413c791b46688d4aacd..9cc88d2c510ce1c4e2fd63bf1976f23b9d7871e2 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -711,12 +711,6 @@ static int omap_8250_startup(struct uart_port *port)
 	struct uart_8250_dma *dma = &priv->omap8250_dma;
 	int ret;
 
-	if (priv->wakeirq) {
-		ret = dev_pm_set_dedicated_wake_irq(port->dev, priv->wakeirq);
-		if (ret)
-			return ret;
-	}
-
 #ifdef CONFIG_PM
 	up->capabilities |= UART_CAP_RPM;
 #endif
@@ -787,7 +781,6 @@ static void omap_8250_shutdown(struct uart_port *port)
 	}
 
 	disable_irq_nosync(port->irq);
-	dev_pm_clear_wake_irq(port->dev);
 
 	serial8250_release_dma(up);
 	up->dma = NULL;
@@ -1588,6 +1581,12 @@ static int omap8250_probe(struct platform_device *pdev)
 
 	priv->wakeirq = irq_of_parse_and_map(np, 1);
 
+	if (priv->wakeirq) {
+		ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, priv->wakeirq);
+		if (ret)
+			goto err;
+	}
+
 	ret = serial8250_register_8250_port(&up);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "unable to register 8250 port\n");
@@ -1608,6 +1607,7 @@ static int omap8250_probe(struct platform_device *pdev)
 	flush_work(&priv->qos_work);
 	pm_runtime_disable(&pdev->dev);
 	cpu_latency_qos_remove_request(&priv->pm_qos_request);
+	dev_pm_clear_wake_irq(&pdev->dev);
 	return ret;
 }
 
@@ -1630,6 +1630,7 @@ static void omap8250_remove(struct platform_device *pdev)
 	flush_work(&priv->qos_work);
 	pm_runtime_disable(&pdev->dev);
 	cpu_latency_qos_remove_request(&priv->pm_qos_request);
+	dev_pm_clear_wake_irq(&pdev->dev);
 	device_set_wakeup_capable(&pdev->dev, false);
 }
 

---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260824-uart-wakeirq-fix-33f470ac7245

Best regards,
-- 
Kendall Willis <k-willis@ti.com>

Re: [PATCH] serial: 8250_omap: fix wake irq cleared during suspend

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Calling `dev_pm_clear_wake_irq()` after `pm_runtime_put_sync()` in the teardown paths creates a race window that can leave the device permanently active.

Pre-existing issues:
- [High] Invalid hardware access via `pm_runtime_get_noresume()` in the IRQ handler allows access to powered-off hardware.
--

--- Patch [1]: [PATCH] serial: 8250_omap: fix wake irq cleared during suspend ---
commit 69c0ce16c163036cdb1ba2600546648c535da1c2
Author: Kendall Willis <k-willis@ti.com>

serial: 8250_omap: fix wake irq cleared during suspend
    
The wake irq was cleared in shutdown(), which runs during the suspend
sequence, making it impossible to wake the system via UART. Move wake
irq setup to probe() and teardown to remove() so the irq remains armed
during suspend when the UART is a wakeup source.

> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 3c7775df27eff..9cc88d2c510ce 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c

[ ... ]

> @@ -1608,6 +1607,7 @@ static int omap8250_probe(struct platform_device *pdev)
>  	flush_work(&priv->qos_work);
>  	pm_runtime_disable(&pdev->dev);
>  	cpu_latency_qos_remove_request(&priv->pm_qos_request);
> +	dev_pm_clear_wake_irq(&pdev->dev);

[Severity: Medium]
Does calling dev_pm_clear_wake_irq() after pm_runtime_put_sync() create a
race condition?

When pm_runtime_put_sync() synchronously suspends the device earlier in the
cleanup path, the PM core enables the wake IRQ. If the wake IRQ fires before
dev_pm_clear_wake_irq() can disable it, the threaded handler may call
pm_runtime_resume().

The subsequent pm_runtime_disable() will then wait for the resume to finish,
leaving the device permanently locked in the RPM_ACTIVE state.

>  	return ret;
>  }
>  
> @@ -1630,6 +1630,7 @@ static void omap8250_remove(struct platform_device *pdev)
>  	flush_work(&priv->qos_work);
>  	pm_runtime_disable(&pdev->dev);
>  	cpu_latency_qos_remove_request(&priv->pm_qos_request);
> +	dev_pm_clear_wake_irq(&pdev->dev);

[Severity: Medium]
Could this teardown order leave the device permanently active here as well?

Similarly to the probe error path above, calling dev_pm_clear_wake_irq()
after pm_runtime_put_sync() opens a window where a wake IRQ could cause the
device to resume.

The pm_runtime_disable() would then wait for the resume to finish, locking
the device in RPM_ACTIVE.

>  	device_set_wakeup_capable(&pdev->dev, false);
>  }

[Severity: High]
This is a pre-existing issue, but does using pm_runtime_get_noresume() in
the interrupt handler allow access to powered-off hardware?

Looking at omap8250_irq() in drivers/tty/serial/8250/8250_omap.c:

omap8250_irq() {
	pm_runtime_get_noresume(port->dev);

	/* Shallow idle state wake-up to an IO interrupt? */
	...
	lsr = serial_port_in(port, UART_LSR);
	iir = serial_port_in(port, UART_IIR);
	...
}

pm_runtime_get_noresume() only increments the usage counter but does not
initiate a resume or check the runtime status.

If the IO interrupt fires while the device is in a shallow idle state, the
code may proceed to read registers while the hardware is suspended or has
lost context.

Should this use pm_runtime_get_if_active() to verify the device is actually
active before accessing hardware registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-uart-wakeirq-fix-v1-1-6d81a230a1c4@ti.com?part=1

Re: [PATCH] serial: 8250_omap: fix wake irq cleared during suspend

[Sebastian Andrzej Siewior]

On 2026-08-24 11:28:51 [-0500], Kendall Willis wrote:
> The wake irq was cleared in shutdown(), which runs during the suspend
> sequence, making it impossible to wake the system via UART.
> Move wake irq setup to probe() and teardown to remove() so the irq
> remains armed during suspend when the UART is a wakeup source.
> 
> Cc: stable@vger.kernel.org
> Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
> Signed-off-by: Kendall Willis <k-willis@ti.com>

I leave this to Tony. I tested it back then on bbb and wake up from idle
was a requirement which worked. Tony had other boards which passed the
test, too.
The omap driver had and has the same logic here.

Sebastian

Re: [PATCH] serial: 8250_omap: fix wake irq cleared during suspend

[Kendall Willis]

On 09:27-20260825, Sebastian Andrzej Siewior wrote:
> On 2026-08-24 11:28:51 [-0500], Kendall Willis wrote:
> > The wake irq was cleared in shutdown(), which runs during the suspend
> > sequence, making it impossible to wake the system via UART.
> > Move wake irq setup to probe() and teardown to remove() so the irq
> > remains armed during suspend when the UART is a wakeup source.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
> > Signed-off-by: Kendall Willis <k-willis@ti.com>
> 
> I leave this to Tony. I tested it back then on bbb and wake up from idle
> was a requirement which worked. Tony had other boards which passed the
> test, too.
> The omap driver had and has the same logic here.
> 
> Sebastian

Hi Sebastian,

When I was testing the dedicated wake irq on AM62P EVM, I was not seeing
the interrupt count increase for the wake irq in /proc/interrupts after
resume. This patch fixes that and after resume the wake irq interrupt
count increases when UART is a wakeup source.

Best,
Kendall