diff mbox series

[3/7] serial: 8250_port: properly handle runtime PM in IRQ

Message ID 20211115084203.56478-4-tony@atomide.com (mailing list archive)
State New, archived
Headers show
Series Serial port generic PM to fix 8250 PM | expand

Commit Message

Tony Lindgren Nov. 15, 2021, 8:41 a.m. UTC
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

We can't and basically don't need to call runtime PM in IRQ handler. If
IRQ is ours, device must be powered on. Otherwise check if the device is
powered off and return immediately.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
[tony@atomide.com: use port->runtime_suspended]
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/tty/serial/8250/8250_port.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Johan Hovold Nov. 30, 2021, 10:36 a.m. UTC | #1
On Mon, Nov 15, 2021 at 10:41:59AM +0200, Tony Lindgren wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> We can't and basically don't need to call runtime PM in IRQ handler. If
> IRQ is ours, device must be powered on. Otherwise check if the device is
> powered off and return immediately.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> [tony@atomide.com: use port->runtime_suspended]
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/tty/serial/8250/8250_port.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1939,17 +1939,19 @@ EXPORT_SYMBOL_GPL(serial8250_handle_irq);
>  
>  static int serial8250_default_handle_irq(struct uart_port *port)
>  {
> -	struct uart_8250_port *up = up_to_u8250p(port);
>  	unsigned int iir;
> -	int ret;
>  
> -	serial8250_rpm_get(up);
> +	/*
> +	 * The IRQ might be shared with other peripherals so we must first
> +	 * check that are we RPM suspended or not. If we are we assume that
> +	 * the IRQ was not for us (we shouldn't be RPM suspended when the
> +	 * interrupt is enabled).
> +	 */
> +	if (port->runtime_suspended)
> +		return 0;

This function is called without holding the port lock that protects
this flag.

Also what prevents the device from being suspended after checking it?

Note that this handler is called from a timer callback when polling and
that case needs to be considered too.

>  
>  	iir = serial_port_in(port, UART_IIR);
> -	ret = serial8250_handle_irq(port, iir);
> -
> -	serial8250_rpm_put(up);
> -	return ret;
> +	return serial8250_handle_irq(port, iir);
>  }
>  
>  /*

Johan
diff mbox series

Patch

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1939,17 +1939,19 @@  EXPORT_SYMBOL_GPL(serial8250_handle_irq);
 
 static int serial8250_default_handle_irq(struct uart_port *port)
 {
-	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned int iir;
-	int ret;
 
-	serial8250_rpm_get(up);
+	/*
+	 * The IRQ might be shared with other peripherals so we must first
+	 * check that are we RPM suspended or not. If we are we assume that
+	 * the IRQ was not for us (we shouldn't be RPM suspended when the
+	 * interrupt is enabled).
+	 */
+	if (port->runtime_suspended)
+		return 0;
 
 	iir = serial_port_in(port, UART_IIR);
-	ret = serial8250_handle_irq(port, iir);
-
-	serial8250_rpm_put(up);
-	return ret;
+	return serial8250_handle_irq(port, iir);
 }
 
 /*