[PATCH tty/serial 1/1] tty: serial: imx: keep console clocks always on

[PATCH tty/serial 1/1] tty: serial: imx: keep console clocks always on

[Fugang Duan]

For below code, there has chance to cause deadlock in SMP system:
Thread 1:
clk_enable_lock();
pr_info("debug message");
clk_enable_unlock(flags);

Thread 2:
imx_uart_console_write()
	clk_enable()
		clk_enable_lock();

Thread 1:
Acuired clk enable_lock -> printk -> console_trylock_spinning
Thread 2:
console_unlock() -> imx_uart_console_write -> clk_disable -> Acquite clk enable_lock

So the patch is to keep console port clocks always on like
other console drivers.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 drivers/tty/serial/imx.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1731d9728865..4d6c009ddc31 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2004,15 +2004,6 @@ imx_uart_console_write(struct console *co, const char *s, unsigned int count)
 	int locked = 1;
 	int retval;
 
-	retval = clk_enable(sport->clk_per);
-	if (retval)
-		return;
-	retval = clk_enable(sport->clk_ipg);
-	if (retval) {
-		clk_disable(sport->clk_per);
-		return;
-	}
-
 	if (sport->port.sysrq)
 		locked = 0;
 	else if (oops_in_progress)
@@ -2047,9 +2038,6 @@ imx_uart_console_write(struct console *co, const char *s, unsigned int count)
 
 	if (locked)
 		spin_unlock_irqrestore(&sport->port.lock, flags);
-
-	clk_disable(sport->clk_ipg);
-	clk_disable(sport->clk_per);
 }
 
 /*
@@ -2150,15 +2138,14 @@ imx_uart_console_setup(struct console *co, char *options)
 
 	retval = uart_set_options(&sport->port, co, baud, parity, bits, flow);
 
-	clk_disable(sport->clk_ipg);
 	if (retval) {
-		clk_unprepare(sport->clk_ipg);
+		clk_disable_unprepare(sport->clk_ipg);
 		goto error_console;
 	}
 
-	retval = clk_prepare(sport->clk_per);
+	retval = clk_prepare_enable(sport->clk_per);
 	if (retval)
-		clk_unprepare(sport->clk_ipg);
+		clk_disable_unprepare(sport->clk_ipg);
 
 error_console:
 	return retval;
-- 
2.17.1

Re: [PATCH tty/serial 1/1] tty: serial: imx: keep console clocks always on

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

On Mon, Nov 09, 2020 at 05:14:03PM +0800, Fugang Duan wrote:
> For below code, there has chance to cause deadlock in SMP system:
> Thread 1:
> clk_enable_lock();
> pr_info("debug message");
> clk_enable_unlock(flags);

flags?

> 
> Thread 2:
> imx_uart_console_write()
> 	clk_enable()
> 		clk_enable_lock();
> 
> Thread 1:
> Acuired clk enable_lock -> printk -> console_trylock_spinning
> Thread 2:
> console_unlock() -> imx_uart_console_write -> clk_disable -> Acquite clk enable_lock
> 
> So the patch is to keep console port clocks always on like
> other console drivers.
> 
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>

Depending on how old this problem is, identifying the commit that
introduces the problem (and noting it in a Fixes: line) would be good.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [EXT] Re: [PATCH tty/serial 1/1] tty: serial: imx: keep console clocks always on

[Andy Duan]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Sent: Wednesday, November 11, 2020 4:31 AM
> On Mon, Nov 09, 2020 at 05:14:03PM +0800, Fugang Duan wrote:
> > For below code, there has chance to cause deadlock in SMP system:
> > Thread 1:
> > clk_enable_lock();
> > pr_info("debug message");
> > clk_enable_unlock(flags);
> 
> flags?

Will remove it, thanks.
> 
> >
> > Thread 2:
> > imx_uart_console_write()
> > 	clk_enable()
> > 		clk_enable_lock();
> >
> > Thread 1:
> > Acuired clk enable_lock -> printk -> console_trylock_spinning Thread
> > 2:
> > console_unlock() -> imx_uart_console_write -> clk_disable -> Acquite
> > clk enable_lock
> >
> > So the patch is to keep console port clocks always on like other
> > console drivers.
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> 
> Depending on how old this problem is, identifying the commit that introduces
> the problem (and noting it in a Fixes: line) would be good.
> 
In normal boot, there has no problem since there have no printk
between clk_enable_lock() and clk_enable_unlock(). If you add debug
message between them or enable track in clk driver, the issue should be
triggered.

Will add the Fixes tag in v2, thanks.

> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König
> |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |