* [PATCH] serial: 8250: omap: Fix missing PM runtime calls for omap8250_set_mctrl()
@ 2022-10-24 6:36 Tony Lindgren
2022-12-03 15:17 ` Romain Naour
0 siblings, 1 reply; 3+ messages in thread
From: Tony Lindgren @ 2022-10-24 6:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johan Hovold, Sebastian Andrzej Siewior, Vignesh Raghavendra,
linux-serial, linux-omap, Merlijn Wajer, Romain Naour,
Ivaylo Dimitrov
There are cases where omap8250_set_mctrl() may get called after the
UART has already autoidled causing an asynchronous external abort.
This can happen on ttyport_open():
mem_serial_in from omap8250_set_mctrl+0x38/0xa0
omap8250_set_mctrl from uart_update_mctrl+0x4c/0x58
uart_update_mctrl from uart_dtr_rts+0x60/0xa8
uart_dtr_rts from tty_port_block_til_ready+0xd0/0x2a8
tty_port_block_til_ready from uart_open+0x14/0x1c
uart_open from ttyport_open+0x64/0x148
And on ttyport_close():
omap8250_set_mctrl from uart_update_mctrl+0x3c/0x48
uart_update_mctrl from uart_dtr_rts+0x54/0x9c
uart_dtr_rts from tty_port_shutdown+0x78/0x9c
tty_port_shutdown from tty_port_close+0x3c/0x74
tty_port_close from ttyport_close+0x40/0x58
It can also happen on disassociate_ctty() calling uart_shutdown()
that ends up calling omap8250_set_mctrl().
Let's fix the issue by adding missing PM runtime calls to
omap8250_set_mctrl(). To do this, we need to add __omap8250_set_mctrl()
that can be called from both omap8250_set_mctrl(), and from runtime PM
resume path when restoring the registers.
Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
Depends-on: dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
Reported-by: Merlijn Wajer <merlijn@wizzup.org>
Reported-by: Romain Naour <romain.naour@smile.fr>
Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/tty/serial/8250/8250_omap.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
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
@@ -157,7 +157,11 @@ static u32 uart_read(struct uart_8250_port *up, u32 reg)
return readl(up->port.membase + (reg << up->port.regshift));
}
-static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
+/*
+ * Called on runtime PM resume path from omap8250_restore_regs(), and
+ * omap8250_set_mctrl().
+ */
+static void __omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
struct uart_8250_port *up = up_to_u8250p(port);
struct omap8250_priv *priv = up->port.private_data;
@@ -181,6 +185,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
}
}
+static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ int err;
+
+ err = pm_runtime_resume_and_get(port->dev);
+ if (err)
+ return;
+
+ __omap8250_set_mctrl(port, mctrl);
+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+}
+
/*
* Work Around for Errata i202 (2430, 3430, 3630, 4430 and 4460)
* The access to uart register after MDR1 Access
@@ -341,7 +359,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
omap8250_update_mdr1(up, priv);
- up->port.ops->set_mctrl(&up->port, up->port.mctrl);
+ __omap8250_set_mctrl(&up->port, up->port.mctrl);
if (up->port.rs485.flags & SER_RS485_ENABLED)
serial8250_em485_stop_tx(up);
--
2.37.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] serial: 8250: omap: Fix missing PM runtime calls for omap8250_set_mctrl()
2022-10-24 6:36 [PATCH] serial: 8250: omap: Fix missing PM runtime calls for omap8250_set_mctrl() Tony Lindgren
@ 2022-12-03 15:17 ` Romain Naour
2022-12-06 6:02 ` Tony Lindgren
0 siblings, 1 reply; 3+ messages in thread
From: Romain Naour @ 2022-12-03 15:17 UTC (permalink / raw)
To: Tony Lindgren, Greg Kroah-Hartman
Cc: Johan Hovold, Sebastian Andrzej Siewior, Vignesh Raghavendra,
linux-serial, linux-omap, Merlijn Wajer, Ivaylo Dimitrov
Hello,
Le 24/10/2022 à 08:36, Tony Lindgren a écrit :
> There are cases where omap8250_set_mctrl() may get called after the
> UART has already autoidled causing an asynchronous external abort.
>
> This can happen on ttyport_open():
>
> mem_serial_in from omap8250_set_mctrl+0x38/0xa0
> omap8250_set_mctrl from uart_update_mctrl+0x4c/0x58
> uart_update_mctrl from uart_dtr_rts+0x60/0xa8
> uart_dtr_rts from tty_port_block_til_ready+0xd0/0x2a8
> tty_port_block_til_ready from uart_open+0x14/0x1c
> uart_open from ttyport_open+0x64/0x148
>
> And on ttyport_close():
>
> omap8250_set_mctrl from uart_update_mctrl+0x3c/0x48
> uart_update_mctrl from uart_dtr_rts+0x54/0x9c
> uart_dtr_rts from tty_port_shutdown+0x78/0x9c
> tty_port_shutdown from tty_port_close+0x3c/0x74
> tty_port_close from ttyport_close+0x40/0x58
>
> It can also happen on disassociate_ctty() calling uart_shutdown()
> that ends up calling omap8250_set_mctrl().
>
> Let's fix the issue by adding missing PM runtime calls to
> omap8250_set_mctrl(). To do this, we need to add __omap8250_set_mctrl()
> that can be called from both omap8250_set_mctrl(), and from runtime PM
> resume path when restoring the registers.
Sorry, I'm late but I confirm that this patch fixes my issue [1].
I checked without this patch applied with the 5.10.153-rt76+ kernel and I can
reproduce the issue.
Tested-by: Romain Naour <romain.naour@smile.fr>
[1] https://marc.info/?l=linux-omap&m=164398186306233&w=2
Best regards,
Romain
>
> Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
> Depends-on: dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> Reported-by: Merlijn Wajer <merlijn@wizzup.org>
> Reported-by: Romain Naour <romain.naour@smile.fr>
> Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/tty/serial/8250/8250_omap.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> 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
> @@ -157,7 +157,11 @@ static u32 uart_read(struct uart_8250_port *up, u32 reg)
> return readl(up->port.membase + (reg << up->port.regshift));
> }
>
> -static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +/*
> + * Called on runtime PM resume path from omap8250_restore_regs(), and
> + * omap8250_set_mctrl().
> + */
> +static void __omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> struct omap8250_priv *priv = up->port.private_data;
> @@ -181,6 +185,20 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
> }
> }
>
> +static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> + int err;
> +
> + err = pm_runtime_resume_and_get(port->dev);
> + if (err)
> + return;
> +
> + __omap8250_set_mctrl(port, mctrl);
> +
> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> +}
> +
> /*
> * Work Around for Errata i202 (2430, 3430, 3630, 4430 and 4460)
> * The access to uart register after MDR1 Access
> @@ -341,7 +359,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
>
> omap8250_update_mdr1(up, priv);
>
> - up->port.ops->set_mctrl(&up->port, up->port.mctrl);
> + __omap8250_set_mctrl(&up->port, up->port.mctrl);
>
> if (up->port.rs485.flags & SER_RS485_ENABLED)
> serial8250_em485_stop_tx(up);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] serial: 8250: omap: Fix missing PM runtime calls for omap8250_set_mctrl()
2022-12-03 15:17 ` Romain Naour
@ 2022-12-06 6:02 ` Tony Lindgren
0 siblings, 0 replies; 3+ messages in thread
From: Tony Lindgren @ 2022-12-06 6:02 UTC (permalink / raw)
To: Romain Naour
Cc: Greg Kroah-Hartman, Johan Hovold, Sebastian Andrzej Siewior,
Vignesh Raghavendra, linux-serial, linux-omap, Merlijn Wajer,
Ivaylo Dimitrov
Hi,
* Romain Naour <romain.naour@smile.fr> [221203 15:17]:
> Le 24/10/2022 à 08:36, Tony Lindgren a écrit :
> > Let's fix the issue by adding missing PM runtime calls to
> > omap8250_set_mctrl(). To do this, we need to add __omap8250_set_mctrl()
> > that can be called from both omap8250_set_mctrl(), and from runtime PM
> > resume path when restoring the registers.
>
> Sorry, I'm late but I confirm that this patch fixes my issue [1].
>
> I checked without this patch applied with the 5.10.153-rt76+ kernel and I can
> reproduce the issue.
OK good to hear, I was wondering about that.
Regards,
Tony
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-06 6:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-24 6:36 [PATCH] serial: 8250: omap: Fix missing PM runtime calls for omap8250_set_mctrl() Tony Lindgren
2022-12-03 15:17 ` Romain Naour
2022-12-06 6:02 ` Tony Lindgren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.