serial: 8250_omap: Drop pm_runtime_irq_safe()
PATCH 1/1 serial: 8250_omap: Drop pm_runtime_irq_safe()
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.
--- drivers/tty/serial/8250/8250_omap.c | 30 ++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)
drivers/tty/serial/8250/8250_omap.c +21 -9
@@ -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 ; }
Signature · 1 line
2.42.0
kernel test robot
build botkernel test robot noticed the following build warnings:
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/[email protected]/config) compiler: sh4-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230929/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Quoted 4 lines · kernel test robot
drivers/tty/serial/8250/8250_omap.c: In function 'omap8250_irq':Quoted 1 line
Quoted 1 line
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