Re: [PATCH 2/3] serial: imx: unprepare console clocks on remove

From: Uwe Kleine-König
Date: Fri Nov 23 2018 - 03:08:18 EST


On Wed, Nov 14, 2018 at 06:49:39PM +0100, Stefan Agner wrote:
> Currently imx_uart_console_setup() prepares clocks which do not
> get unprepared anywhere. Check whether the console has been used
> by testing if index is set and unprepare clocks in this case.
>
> This makes sure that clocks are properly unprepared after the
> console device has been unbound.
>
> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
> ---
> drivers/tty/serial/imx.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 313c3b1900a8..757c91e5105a 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -2085,7 +2085,7 @@ static struct console imx_uart_console = {
> .data = &imx_uart_uart_driver,
> };
>
> -#define IMX_CONSOLE &imx_uart_console
> +#define IMX_CONSOLE (&imx_uart_console)
>
> #ifdef CONFIG_OF
> static void imx_uart_console_early_putchar(struct uart_port *port, int ch)
> @@ -2378,8 +2378,17 @@ static int imx_uart_probe(struct platform_device *pdev)
> static int imx_uart_remove(struct platform_device *pdev)
> {
> struct imx_port *sport = platform_get_drvdata(pdev);
> + int ret;
> +
> + ret = uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
> +
> + if (IS_ENABLED(CONFIG_SERIAL_IMX_CONSOLE) && IMX_CONSOLE->index >= 0) {
> + clk_unprepare(sport->clk_ipg);
> + clk_unprepare(sport->clk_per);
> + IMX_CONSOLE->index = -1;
> + }
>
> - return uart_remove_one_port(&imx_uart_uart_driver, &sport->port);
> + return ret;

I doubt this is right. imx_uart_console_setup is called once, and
if the console is on (say) ttymxc0 you don't want to unprepare the
clocks if ttymxc3 gets unbound.

So I think this cleanup must go into imx_uart_exit().

Best regards
Uwe

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