Re: [poky] [PATCH v2] linux-yocto: revert omap8250 power management changes on genericarm64

Ackd

Bruce

On Tue, Feb 11, 2025 at 6:56 AM Ross Burton via lists.yoctoproject.org
<[email protected]> wrote:

> The genericarm64 machine sets SERIAL_CONSOLES to a number of potential
> devices:
>
> SERIAL_CONSOLES ?= "115200;ttyAMA0 115200;hvc0 115200;ttyS0 115200;ttyS1
> 115200;ttyS2"
>
> With sysvinit this turns into getty lines in inittab, and with systemd
> the systemd-serialgetty recipe creates explicit units to spawn gettys.
>
> This worked fine with 6.6, but since "serial: 8250_omap: Drop
> pm_runtime_irq_safe()"[1] in 6.7 onwards we see kernel hangs:
>
> BUG: scheduling while atomic: getty/957/0x00000002
> Call trace:
>  dump_stack+0x1c/0x30
>  __schedule_bug+0x60/0x90
>  __schedule+0x83c/0xcf8
>  schedule+0x40/0x158
>  schedule_timeout+0xb0/0x1b0
>  wait_for_completion_timeout+0x84/0x188
>  ti_sci_set_device_state+0x134/0x220
>  ti_sci_cmd_get_device_exclusive+0x24/0x40
>  ti_sci_pd_power_on+0x34/0x68 [ti_sci_pm_domains]
>  _genpd_power_on+0xa4/0x178
>  genpd_power_on+0xb4/0x190
>  genpd_runtime_resume+0xc8/0x260
>  __rpm_callback+0x54/0x200
>  rpm_callback+0x78/0x90
>  rpm_resume+0x420/0x690
>  __pm_runtime_resume+0x5c/0xb0
>  omap8250_set_mctrl+0x38/0xe0 [8250_omap]
>  serial8250_set_mctrl+0x2c/0x60
>  uart_update_mctrl+0x98/0x120
>  uart_shutdown+0x124/0x180
>  uart_hangup+0x7c/0x180
>  __tty_hangup.part.0+0x408/0x440
>  tty_vhangup_session+0x24/0x40
>  disassociate_ctty.part.0+0x48/0x1b0
>  disassociate_ctty+0x30/0x48
> (full backtrace elided)
>
> With many thanks to TI, my understanding is that it was determined that
> the problem here is that we have a getty connected to ttyS1 which is
> actually the expansion port uart and on the BeaglePlay wired up to the
> wifi controller's debug port. The getty receives noise it doesn't know
> what to do with, and at some point the power management code does a
> suspend/result cycle of the device.  The serial drivers assume that
> child nodes use the serdev driver and they manage runtime_pm, but the
> getty opening the tty breaks a series of bad assumptions in the drivers.
>
> So, there are two bugs:
> 1) The kernel shouldn't crash if this tty is opened
> 2) The only serial port for a console on the BeaglePlay is ttyS2,
>    despite others existing.
>
> TI are looking at (1) and other patches to follow will deal with (2).
> Until one of these is resolved entirely, reverting this change to power
> management stops the crashes.
>
> [ YOCTO #15704 ]
> [1] linux 8700a7ea5519fb0b3bad2362adfeac358c2119ce
>
> Signed-off-by: Ross Burton <[email protected]>
> ---
>  ...l-8250_omap-Drop-pm_runtime_irq_safe.patch | 130 ++++++++++++++++++
>  .../linux/linux-yocto_6.12.bbappend           |   2 +
>  2 files changed, 132 insertions(+)
>  create mode 100644
> meta-yocto-bsp/recipes-kernel/linux/files/0001-Revert-serial-8250_omap-Drop-pm_runtime_irq_safe.patch
>
> diff --git
> a/meta-yocto-bsp/recipes-kernel/linux/files/0001-Revert-serial-8250_omap-Drop-pm_runtime_irq_safe.patch
> b/meta-yocto-bsp/recipes-kernel/linux/files/0001-Revert-serial-8250_omap-Drop-pm_runtime_irq_safe.patch
> new file mode 100644
> index 00000000000..8837dd23464
> --- /dev/null
> +++
> b/meta-yocto-bsp/recipes-kernel/linux/files/0001-Revert-serial-8250_omap-Drop-pm_runtime_irq_safe.patch
> @@ -0,0 +1,130 @@
> +From cc255f5132cf39e9154340cf58780f8c763c6481 Mon Sep 17 00:00:00 2001
> +From: Ross Burton <[email protected]>
> +Date: Thu, 23 Jan 2025 17:06:08 +0000
> +Subject: [PATCH] Revert "serial: 8250_omap: Drop pm_runtime_irq_safe()"
> +
> +This reverts commit 8700a7ea5519fb0b3bad2362adfeac358c2119ce.
> +
> +Upstream-Status: Inappropriate
> +Signed-off-by: Ross Burton <[email protected]>
> +---
> + drivers/tty/serial/8250/8250_omap.c | 29 ++++++++---------------------
> + 1 file changed, 8 insertions(+), 21 deletions(-)
> +
> +diff --git a/drivers/tty/serial/8250/8250_omap.c
> b/drivers/tty/serial/8250/8250_omap.c
> +index 0dd68bdbfbcf7..db24d7d1dcb67 100644
> +--- a/drivers/tty/serial/8250/8250_omap.c
> ++++ b/drivers/tty/serial/8250/8250_omap.c
> +@@ -8,7 +8,6 @@
> +  *
> +  */
> +
> +-#include <linux/atomic.h>
> + #include <linux/clk.h>
> + #include <linux/device.h>
> + #include <linux/io.h>
> +@@ -134,7 +133,6 @@ struct omap8250_priv {
> +
> +       u8 tx_trigger;
> +       u8 rx_trigger;
> +-      atomic_t active;
> +       bool is_suspending;
> +       int wakeirq;
> +       u32 latency;
> +@@ -636,23 +634,14 @@ 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);
> +-              pm_runtime_mark_last_busy(port->dev);
> +-              pm_runtime_put(port->dev);
> +               return IRQ_RETVAL(ret);
> +       }
> + #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);
> +@@ -701,8 +690,7 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
> +               schedule_delayed_work(&up->overrun_backoff, delay);
> +       }
> +
> +-      pm_runtime_mark_last_busy(port->dev);
> +-      pm_runtime_put(port->dev);
> ++      serial8250_rpm_put(up);
> +
> +       return IRQ_RETVAL(ret);
> + }
> +@@ -1314,8 +1302,11 @@ 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;
> +       }
> +
> +@@ -1348,6 +1339,7 @@ static int omap_8250_dma_handle_irq(struct
> uart_port *port)
> +
> +       uart_unlock_and_check_sysrq(port);
> +
> ++      serial8250_rpm_put(up);
> +       return 1;
> + }
> +
> +@@ -1539,6 +1531,8 @@ 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);
> +@@ -1776,7 +1770,6 @@ 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;
> + }
> +@@ -1786,10 +1779,6 @@ 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);
> +
> +@@ -1805,10 +1794,8 @@ 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;
> + }
> +
> +--
> +2.43.0
> +
> diff --git a/meta-yocto-bsp/recipes-kernel/linux/linux-yocto_6.12.bbappend
> b/meta-yocto-bsp/recipes-kernel/linux/linux-yocto_6.12.bbappend
> index 1ffd2194d96..831cb556be4 100644
> --- a/meta-yocto-bsp/recipes-kernel/linux/linux-yocto_6.12.bbappend
> +++ b/meta-yocto-bsp/recipes-kernel/linux/linux-yocto_6.12.bbappend
> @@ -7,3 +7,5 @@ KMACHINE:beaglebone-yocto ?= "beaglebone"
>  KMACHINE:genericx86 ?= "common-pc"
>  KMACHINE:genericx86-64 ?= "common-pc-64"
>
> +FILESEXTRAPATHS:prepend:genericarm64 := "${THISDIR}/files:"
> +SRC_URI:append:genericarm64 = "
> file://0001-Revert-serial-8250_omap-Drop-pm_runtime_irq_safe.patch"
> --
> 2.43.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#13530):
> https://lists.yoctoproject.org/g/poky/message/13530
> Mute This Topic: https://lists.yoctoproject.org/mt/111120723/1050810
> Group Owner: [email protected]
> Unsubscribe: https://lists.yoctoproject.org/g/poky/unsub [
> [email protected]]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II