diff mbox series

serial: 8250_omap: Drop pm_runtime_irq_safe()

Message ID 20230928080358.2693-1-tony@atomide.com
State New
Headers
Series serial: 8250_omap: Drop pm_runtime_irq_safe() |

Commit Message

Tony Lindgren Sept. 28, 2023, 8:03 a.m. UTC 
  Let's drop the use of pm_runtime_irq_safe() for 8250_omap. The use of
pm_runtime_irq_safe() is not nice as it takes a permanent usage count on
the parent device.

We can finally drop pm_runtime_irq_safe() safely as the kernel now knows
when the uart port tx is active. This changed with commit 84a9582fd203
("serial: core: Start managing serial controllers to enable runtime PM").

For serial port rx, we already use Linux generic wakeirqs for 8250_omap.

To drop pm_runtime_irq_safe(), we need to add handling for shallow idle
state where the port hardware may already be awake and an IO interrupt
happens. We also need to replace the serial8250_rpm sync calls in the
interrupt handlers with async runtime PM calls.

Note that omap8250_irq() calls omap_8250_dma_handle_irq(), so we don't
need separate runtime PM calls in omap_8250_dma_handle_irq().

While at it, let's also add the missing line break to the end of
omap8250_runtime_resume() to group the calls.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/tty/serial/8250/8250_omap.c | 30 ++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

Comments

kernel test robot Sept. 28, 2023, 6:11 p.m. UTC | #1 
Hi Tony,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.6-rc3 next-20230928]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tony-Lindgren/serial-8250_omap-Drop-pm_runtime_irq_safe/20230928-160501
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230928080358.2693-1-tony%40atomide.com
patch subject: [PATCH] serial: 8250_omap: Drop pm_runtime_irq_safe()
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20230929/202309290255.uEGVqQAE-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230929/202309290255.uEGVqQAE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309290255.uEGVqQAE-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/tty/serial/8250/8250_omap.c: In function 'omap8250_irq':
>> drivers/tty/serial/8250/8250_omap.c:688:1: warning: label 'out_runtime_put' defined but not used [-Wunused-label]
     688 | out_runtime_put:
         | ^~~~~~~~~~~~~~~


vim +/out_runtime_put +688 drivers/tty/serial/8250/8250_omap.c

   651	
   652		lsr = serial_port_in(port, UART_LSR);
   653		iir = serial_port_in(port, UART_IIR);
   654		ret = serial8250_handle_irq(port, iir);
   655	
   656		/*
   657		 * On K3 SoCs, it is observed that RX TIMEOUT is signalled after
   658		 * FIFO has been drained, in which case a dummy read of RX FIFO
   659		 * is required to clear RX TIMEOUT condition.
   660		 */
   661		if (priv->habit & UART_RX_TIMEOUT_QUIRK &&
   662		    (iir & UART_IIR_RX_TIMEOUT) == UART_IIR_RX_TIMEOUT &&
   663		    serial_port_in(port, UART_OMAP_RX_LVL) == 0) {
   664			serial_port_in(port, UART_RX);
   665		}
   666	
   667		/* Stop processing interrupts on input overrun */
   668		if ((lsr & UART_LSR_OE) && up->overrun_backoff_time_ms > 0) {
   669			unsigned long delay;
   670	
   671			/* Synchronize UART_IER access against the console. */
   672			uart_port_lock(port);
   673			up->ier = port->serial_in(port, UART_IER);
   674			if (up->ier & (UART_IER_RLSI | UART_IER_RDI)) {
   675				port->ops->stop_rx(port);
   676			} else {
   677				/* Keep restarting the timer until
   678				 * the input overrun subsides.
   679				 */
   680				cancel_delayed_work(&up->overrun_backoff);
   681			}
   682			uart_port_unlock(port);
   683	
   684			delay = msecs_to_jiffies(up->overrun_backoff_time_ms);
   685			schedule_delayed_work(&up->overrun_backoff, delay);
   686		}
   687	
 > 688	out_runtime_put:
   689		pm_runtime_mark_last_busy(port->dev);
   690		pm_runtime_put(port->dev);
   691	
   692		return IRQ_RETVAL(ret);
   693	}
   694
Tony Lindgren Sept. 29, 2023, 7:27 a.m. UTC | #2 
* kernel test robot <lkp@intel.com> [230928 18:13]:
>    drivers/tty/serial/8250/8250_omap.c: In function 'omap8250_irq':
> >> drivers/tty/serial/8250/8250_omap.c:688:1: warning: label 'out_runtime_put' defined but not used [-Wunused-label]
>      688 | out_runtime_put:
>          | ^~~~~~~~~~~~~~~

Looks like this happens if CONFIG_SERIAL_8250_DMA is not set, I'll take
a look.

Regards,

Tony
diff mbox series

Patch

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
@@ -8,6 +8,7 @@ 
  *
  */
 
+#include <linux/atomic.h>
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/io.h>
@@ -130,6 +131,7 @@  struct omap8250_priv {
 
 	u8 tx_trigger;
 	u8 rx_trigger;
+	atomic_t active;
 	bool is_suspending;
 	int wakeirq;
 	int wakeups_enabled;
@@ -632,14 +634,21 @@  static irqreturn_t omap8250_irq(int irq, void *dev_id)
 	unsigned int iir, lsr;
 	int ret;
 
+	pm_runtime_get_noresume(port->dev);
+
+	/* Shallow idle state wake-up to an IO interrupt? */
+	if (atomic_add_unless(&priv->active, 1, 1)) {
+		priv->latency = priv->calc_latency;
+		schedule_work(&priv->qos_work);
+	}
+
 #ifdef CONFIG_SERIAL_8250_DMA
 	if (up->dma) {
 		ret = omap_8250_dma_handle_irq(port);
-		return IRQ_RETVAL(ret);
+		goto out_runtime_put;
 	}
 #endif
 
-	serial8250_rpm_get(up);
 	lsr = serial_port_in(port, UART_LSR);
 	iir = serial_port_in(port, UART_IIR);
 	ret = serial8250_handle_irq(port, iir);
@@ -676,7 +685,9 @@  static irqreturn_t omap8250_irq(int irq, void *dev_id)
 		schedule_delayed_work(&up->overrun_backoff, delay);
 	}
 
-	serial8250_rpm_put(up);
+out_runtime_put:
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put(port->dev);
 
 	return IRQ_RETVAL(ret);
 }
@@ -1270,11 +1281,8 @@  static int omap_8250_dma_handle_irq(struct uart_port *port)
 	u16 status;
 	u8 iir;
 
-	serial8250_rpm_get(up);
-
 	iir = serial_port_in(port, UART_IIR);
 	if (iir & UART_IIR_NO_INT) {
-		serial8250_rpm_put(up);
 		return IRQ_HANDLED;
 	}
 
@@ -1305,7 +1313,6 @@  static int omap_8250_dma_handle_irq(struct uart_port *port)
 
 	uart_unlock_and_check_sysrq(port);
 
-	serial8250_rpm_put(up);
 	return 1;
 }
 
@@ -1503,8 +1510,6 @@  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);
@@ -1743,6 +1748,7 @@  static int omap8250_runtime_suspend(struct device *dev)
 
 	priv->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
 	schedule_work(&priv->qos_work);
+	atomic_set(&priv->active, 0);
 
 	return 0;
 }
@@ -1752,6 +1758,10 @@  static int omap8250_runtime_resume(struct device *dev)
 	struct omap8250_priv *priv = dev_get_drvdata(dev);
 	struct uart_8250_port *up = NULL;
 
+	/* Did the hardware wake to a device IO interrupt before a wakeirq? */
+	if (atomic_read(&priv->active))
+		return 0;
+
 	if (priv->line >= 0)
 		up = serial8250_get_port(priv->line);
 
@@ -1767,8 +1777,10 @@  static int omap8250_runtime_resume(struct device *dev)
 		uart_port_unlock_irq(&up->port);
 	}
 
+	atomic_set(&priv->active, 1);
 	priv->latency = priv->calc_latency;
 	schedule_work(&priv->qos_work);
+
 	return 0;
 }