On Wed, Nov 11, 2020 at 10:51:36AM +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();
>
> 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.
>
> Fixes: 1cf93e0d5488 ("serial: imx: remove the uart_console() check")
> Signed-off-by: Fugang Duan <fugang.duan@xxxxxxx>
> ---
> v2: Add fixes tag in commit message.
> ---
> 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
>
Did you test build this change and totally ignore the build warning you
now get:
drivers/tty/serial/imx.c: In function ‘imx_uart_console_write’:
drivers/tty/serial/imx.c:2011:6: warning: unused variable ‘retval’ [-Wunused-variable]
2011 | int retval;
| ^~~~~~
Not good...
I'll go fix it.
greg k-h
