Re: [PATCH] serial: 8250_port: Fix imprecise external abort for mctrl if inactive

[Tony Lindgren]

* Tony Lindgren <t...@atomide.com> [200602 13:38]:
> * Andy Shevchenko <andy.shevche...@gmail.com> [200602 08:33]:
> Now that we can detach and reattach the kernel serial console,
> there should not be any need for pm_runtime_irq_safe() anymore :)

Below is a hastily tested RFC patch to remove pm_runtime_irq_safe()
for 8250_omap.c that seems to work for idle use case :)

> And the UART wake-up from deeper idle states can only happen with
> help of external hardware like GPIO controller or pinctrl controller.

It does not yet include the check for configured wakeirq though.
And omap-serial.c needs a similar patch or maybe we can attempt
to just drop it this time as 8250_omap.c should be used nowadays.
Or just drop PM from omap-serial.c if it can't be dropped.

Andy, is the change below the only remaining blocker now for
your serial PM runtime changes?

Regards,

Tony

8< -------------------------------
diff --git a/drivers/tty/serial/8250/8250_omap.c 
b/drivers/tty/serial/8250/8250_omap.c
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -123,6 +123,7 @@ struct omap8250_priv {
        spinlock_t rx_dma_lock;
        bool rx_dma_broken;
        bool throttled;
+       unsigned int active:1;
 };
 
 struct omap8250_dma_params {
@@ -598,9 +599,13 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
 {
        struct uart_port *port = dev_id;
        struct uart_8250_port *up = up_to_u8250p(port);
+       struct omap8250_priv *priv = up->port.private_data;
        unsigned int iir;
        int ret;
 
+       if (!priv->active)
+               return IRQ_NONE;
+
 #ifdef CONFIG_SERIAL_8250_DMA
        if (up->dma) {
                ret = omap_8250_dma_handle_irq(port);
@@ -608,10 +613,10 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
        }
 #endif
 
-       serial8250_rpm_get(up);
        iir = serial_port_in(port, UART_IIR);
        ret = serial8250_handle_irq(port, iir);
-       serial8250_rpm_put(up);
+
+       pm_runtime_mark_last_busy(port->dev);
 
        return IRQ_RETVAL(ret);
 }
@@ -1110,13 +1115,9 @@ static int omap_8250_dma_handle_irq(struct uart_port 
*port)
        unsigned long flags;
        u8 iir;
 
-       serial8250_rpm_get(up);
-
        iir = serial_port_in(port, UART_IIR);
-       if (iir & UART_IIR_NO_INT) {
-               serial8250_rpm_put(up);
+       if (iir & UART_IIR_NO_INT)
                return IRQ_HANDLED;
-       }
 
        spin_lock_irqsave(&port->lock, flags);
 
@@ -1144,7 +1145,7 @@ static int omap_8250_dma_handle_irq(struct uart_port 
*port)
        }
 
        uart_unlock_and_check_sysrq(port, flags);
-       serial8250_rpm_put(up);
+
        return 1;
 }
 
@@ -1329,11 +1330,10 @@ static int omap8250_probe(struct platform_device *pdev)
        if (!of_get_available_child_count(pdev->dev.of_node))
                pm_runtime_set_autosuspend_delay(&pdev->dev, -1);
 
-       pm_runtime_irq_safe(&pdev->dev);
-
        pm_runtime_get_sync(&pdev->dev);
 
        omap_serial_fill_features_erratas(&up, priv);
+       priv->active = pm_runtime_enabled(&pdev->dev);
        up.port.handle_irq = omap8250_no_handle_irq;
        priv->rx_trigger = RX_TRIGGER;
        priv->tx_trigger = TX_TRIGGER;
@@ -1517,6 +1517,8 @@ static int omap8250_runtime_suspend(struct device *dev)
        if (!priv)
                return 0;
 
+       priv->active = 0;
+
        up = serial8250_get_port(priv->line);
        /*
         * When using 'no_console_suspend', the console UART must not be
@@ -1575,6 +1577,8 @@ static int omap8250_runtime_resume(struct device *dev)
 
        pinctrl_pm_select_default_state(dev);
 
+       priv->active = 1;
+
        return 0;
 }
 #endif
-- 
2.26.2