On Wed, Jun 26, 2013 at 05:35:58PM +0800, Huang Shijie wrote:
> The uart_console() check makes the clocks(clk_per and clk_ipg) opened
> even when we close the console uart.
>
> This patch enable/disable the clocks in imx_console_write(),
> so we can keep the clocks closed when the console uart is closed.
>
> Signed-off-by: Huang Shijie <b32955@xxxxxxxxxxxxx>
> ---
> drivers/tty/serial/imx.c | 63 ++++++++++++++++++++++++++++++++-------------
> 1 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 415cec6..bfdf764 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -702,15 +702,13 @@ static int imx_startup(struct uart_port *port)
> int retval;
> unsigned long flags, temp;
>
> - if (!uart_console(port)) {
> - retval = clk_prepare_enable(sport->clk_per);
> - if (retval)
> - goto error_out1;
> - retval = clk_prepare_enable(sport->clk_ipg);
> - if (retval) {
> - clk_disable_unprepare(sport->clk_per);
> - goto error_out1;
> - }
> + retval = clk_prepare_enable(sport->clk_per);
> + if (retval)
> + goto error_out1;
> + retval = clk_prepare_enable(sport->clk_ipg);
> + if (retval) {
> + clk_disable_unprepare(sport->clk_per);
> + goto error_out1;
> }
>
> imx_setup_ufcr(sport, 0);
> @@ -901,10 +899,8 @@ static void imx_shutdown(struct uart_port *port)
> writel(temp, sport->port.membase + UCR1);
> spin_unlock_irqrestore(&sport->port.lock, flags);
>
> - if (!uart_console(&sport->port)) {
> - clk_disable_unprepare(sport->clk_per);
> - clk_disable_unprepare(sport->clk_ipg);
> - }
> + clk_disable_unprepare(sport->clk_per);
> + clk_disable_unprepare(sport->clk_ipg);
> }
>
> static void
> @@ -1251,6 +1247,16 @@ imx_console_write(struct console *co, const char *s, unsigned int count)
> unsigned int ucr1;
> unsigned long flags = 0;
> 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;
> @@ -1286,6 +1292,9 @@ imx_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);
> }
>
> /*
> @@ -1359,6 +1368,7 @@ imx_console_setup(struct console *co, char *options)
> int bits = 8;
> int parity = 'n';
> int flow = 'n';
> + int retval;
>
> /*
> * Check whether an invalid uart number has been specified, and
> @@ -1371,6 +1381,15 @@ imx_console_setup(struct console *co, char *options)
> if (sport == NULL)
> return -ENODEV;
>
> + retval = clk_prepare_enable(sport->clk_per);
> + if (retval)
> + goto error_console;
> + retval = clk_prepare_enable(sport->clk_ipg);
> + if (retval) {
> + clk_disable_unprepare(sport->clk_per);
> + goto error_console;
> + }
> +
Why do we need clk_enable() here at all? The amba-pl011 driver only
calls clk_prepare() in console .setup().
> if (options)
> uart_parse_options(options, &baud, &parity, &bits, &flow);
> else
> @@ -1378,7 +1397,17 @@ imx_console_setup(struct console *co, char *options)
>
> imx_setup_ufcr(sport, 0);
>
> - return uart_set_options(&sport->port, co, baud, parity, bits, flow);
> + retval = uart_set_options(&sport->port, co, baud, parity, bits, flow);
> +
> + clk_disable(sport->clk_per);
> + clk_disable(sport->clk_ipg);
> + if (retval) {
> + clk_unprepare(sport->clk_per);
> + clk_unprepare(sport->clk_ipg);
> + }
> +
> +error_console:
> + return retval;
> }
>
> static struct uart_driver imx_reg;
> @@ -1583,10 +1612,8 @@ static int serial_imx_probe(struct platform_device *pdev)
> goto deinit;
> platform_set_drvdata(pdev, sport);
>
> - if (!uart_console(&sport->port)) {
> - clk_disable_unprepare(sport->clk_per);
> - clk_disable_unprepare(sport->clk_ipg);
> - }
> + clk_disable_unprepare(sport->clk_per);
> + clk_disable_unprepare(sport->clk_ipg);
I also had a hard time to understand why we need to turn on the clocks
in .probe() for a while and then turn them off.
It just reminds me a thing. Did you test CONFIG_CONSOLE_POLL support
when your commit 28eb427 (serial: imx: enable the clocks only when the
uart is used) went in? The commit turns off the clocks at the
end of .probe(), but who will enable the clocks for .poll_get_char()
and .poll_put_char()? The amba-pl011 driver does that in .poll_init().
Shawn
>
> return 0;
> deinit:
> --
> 1.7.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
