diff mbox

[v2] serial: imx: enable the clocks for console

Message ID 1370743279-15941-1-git-send-email-b32955@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Huang Shijie June 9, 2013, 2:01 a.m. UTC
The console's clocks are disabled after the uart driver is probed.
It makes that we can see less log from the console now
(though we still can get all the log by the `dmesg`).

So enable the clocks for console, and we can see all the log again.

This patch also disables the sport->clk_per when we fail to enable
the sport->clk_ipg;

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
v1 --> v2:
	disable the sport->clk_per when we fail to enable
	the sport->clk_ipg;
---
 drivers/tty/serial/imx.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

Comments

Shawn Guo June 9, 2013, 8:34 a.m. UTC | #1
On Sun, Jun 09, 2013 at 10:01:19AM +0800, Huang Shijie wrote:
> The console's clocks are disabled after the uart driver is probed.
> It makes that we can see less log from the console now
> (though we still can get all the log by the `dmesg`).
> 
> So enable the clocks for console, and we can see all the log again.
> 
> This patch also disables the sport->clk_per when we fail to enable
> the sport->clk_ipg;
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>

Tested-by: Shawn Guo <shawn.guo@linaro.org>
Shawn Guo June 9, 2013, 1:05 p.m. UTC | #2
On Sun, Jun 09, 2013 at 04:34:11PM +0800, Shawn Guo wrote:
> On Sun, Jun 09, 2013 at 10:01:19AM +0800, Huang Shijie wrote:
> > The console's clocks are disabled after the uart driver is probed.
> > It makes that we can see less log from the console now
> > (though we still can get all the log by the `dmesg`).
> > 
> > So enable the clocks for console, and we can see all the log again.
> > 
> > This patch also disables the sport->clk_per when we fail to enable
> > the sport->clk_ipg;
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> 
> Tested-by: Shawn Guo <shawn.guo@linaro.org>

Sorry.  I have to take that back.

Though it works fine with BusyBox, I'm getting the following warnings
when booting with an Ubuntu 13.04 rootfs, in which case imx_startup()
and imx_shutdown() will be called for a number of iterations.  That
causes unbalanced clk_prepare_enable vs. clk_disable_unprepare, since
the clk_prepare_enable is under condition if (!uart_console(port)) in
imx_startup(), while clk_disable_unprepare in imx_shutdown() is not.

Shawn

------------[ cut here ]------------
WARNING: CPU: 0 PID: 70 at /home/r65073/repos/next/linux-next/drivers/clk/clk.c:780 __clk_disable+0x68/0x84()
Modules linked in:
CPU: 0 PID: 70 Comm: mountall Not tainted 3.10.0-rc4-next-20130607+ #435
Backtrace: 
[<800116a4>] (dump_backtrace+0x0/0x10c) from [<80011844>] (show_stack+0x18/0x1c)
 r6:8069f4e8 r5:0000030c r4:00000000 r3:00000000
[<8001182c>] (show_stack+0x0/0x1c) from [<8053bce0>] (dump_stack+0x78/0x94)
[<8053bc68>] (dump_stack+0x0/0x94) from [<80023df8>] (warn_slowpath_common+0x6c/0x8c)
 r4:00000000 r3:00000000
[<80023d8c>] (warn_slowpath_common+0x0/0x8c) from [<80023e3c>] (warn_slowpath_null+0x24/0x2c)
 r8:bf2ed008 r7:bfaa9810 r6:000f0013 r5:bf824b80 r4:bf824b80
[<80023e18>] (warn_slowpath_null+0x0/0x2c) from [<8041af84>] (__clk_disable+0x68/0x84)
[<8041af1c>] (__clk_disable+0x0/0x84) from [<8041b098>] (clk_disable+0x20/0x2c)
 r4:600f0013 r3:00000001
[<8041b078>] (clk_disable+0x0/0x2c) from [<802c93e8>] (imx_shutdown+0xbc/0xec)
 r5:bf824b80 r4:bfaa9810
[<802c932c>] (imx_shutdown+0x0/0xec) from [<802c63a0>] (uart_port_shutdown+0x34/0x40)
 r5:bf86f860 r4:bfaa9810
[<802c636c>] (uart_port_shutdown+0x0/0x40) from [<802c68c0>] (uart_shutdown+0x98/0xc4)
 r4:bf86f800 r3:00000000
[<802c6828>] (uart_shutdown+0x0/0xc4) from [<802c7514>] (uart_close+0x5c/0x198)
 r7:bfaa9810 r6:bf274400 r5:bf86f86c r4:bf86f800
[<802c74b8>] (uart_close+0x0/0x198) from [<802ac648>] (tty_release+0xf8/0x500)
[<802ac550>] (tty_release+0x0/0x500) from [<800c5a30>] (__fput+0x9c/0x208)
[<800c5994>] (__fput+0x0/0x208) from [<800c5bac>] (____fput+0x10/0x14)
[<800c5b9c>] (____fput+0x0/0x14) from [<80040234>] (task_work_run+0xb4/0xec)
[<80040180>] (task_work_run+0x0/0xec) from [<80029238>] (do_exit+0x2b0/0x920)
 r8:8000e144 r7:000000f8 r6:bf306300 r5:00000000 r4:bfac1180
[<80028f88>] (do_exit+0x0/0x920) from [<80029a4c>] (do_group_exit+0x50/0xd4)
 r7:000000f8
[<800299fc>] (do_group_exit+0x0/0xd4) from [<80029ae8>] (__wake_up_parent+0x0/0x28)
 r7:000000f8 r6:00000001 r5:0006f7ae r4:0006f79a
[<80029ad0>] (SyS_exit_group+0x0/0x18) from [<8000dfc0>] (ret_fast_syscall+0x0/0x30)
---[ end trace 16d080eb7efea4e9 ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 70 at /home/r65073/repos/next/linux-next/drivers/clk/clk.c:689 __clk_unprepare+0x6c/0x8c()
Modules linked in:
CPU: 0 PID: 70 Comm: mountall Tainted: G        W    3.10.0-rc4-next-20130607+ #435
Backtrace: 
[<800116a4>] (dump_backtrace+0x0/0x10c) from [<80011844>] (show_stack+0x18/0x1c)
 r6:8069f4e8 r5:000002b1 r4:00000000 r3:00000000
[<8001182c>] (show_stack+0x0/0x1c) from [<8053bce0>] (dump_stack+0x78/0x94)
[<8053bc68>] (dump_stack+0x0/0x94) from [<80023df8>] (warn_slowpath_common+0x6c/0x8c)
 r4:00000000 r3:00000000
[<80023d8c>] (warn_slowpath_common+0x0/0x8c) from [<80023e3c>] (warn_slowpath_null+0x24/0x2c)
 r8:bf2ed008 r7:bfaa9810 r6:000f0013 r5:bf824b80 r4:bf824b80
[<80023e18>] (warn_slowpath_null+0x0/0x2c) from [<8041b68c>] (__clk_unprepare+0x6c/0x8c)
[<8041b620>] (__clk_unprepare+0x0/0x8c) from [<8041b6c8>] (clk_unprepare+0x1c/0x24)
 r4:bf824b80 r3:00000000
[<8041b6ac>] (clk_unprepare+0x0/0x24) from [<802c93f0>] (imx_shutdown+0xc4/0xec)
 r4:bfaa9810 r3:00000000
[<802c932c>] (imx_shutdown+0x0/0xec) from [<802c63a0>] (uart_port_shutdown+0x34/0x40)
 r5:bf86f860 r4:bfaa9810
[<802c636c>] (uart_port_shutdown+0x0/0x40) from [<802c68c0>] (uart_shutdown+0x98/0xc4)
 r4:bf86f800 r3:00000000
[<802c6828>] (uart_shutdown+0x0/0xc4) from [<802c7514>] (uart_close+0x5c/0x198)
 r7:bfaa9810 r6:bf274400 r5:bf86f86c r4:bf86f800
[<802c74b8>] (uart_close+0x0/0x198) from [<802ac648>] (tty_release+0xf8/0x500)
[<802ac550>] (tty_release+0x0/0x500) from [<800c5a30>] (__fput+0x9c/0x208)
[<800c5994>] (__fput+0x0/0x208) from [<800c5bac>] (____fput+0x10/0x14)
[<800c5b9c>] (____fput+0x0/0x14) from [<80040234>] (task_work_run+0xb4/0xec)
[<80040180>] (task_work_run+0x0/0xec) from [<80029238>] (do_exit+0x2b0/0x920)
 r8:8000e144 r7:000000f8 r6:bf306300 r5:00000000 r4:bfac1180
[<80028f88>] (do_exit+0x0/0x920) from [<80029a4c>] (do_group_exit+0x50/0xd4)
 r7:000000f8
[<800299fc>] (do_group_exit+0x0/0xd4) from [<80029ae8>] (__wake_up_parent+0x0/0x28)
 r7:000000f8 r6:00000001 r5:0006f7ae r4:0006f79a
[<80029ad0>] (SyS_exit_group+0x0/0x18) from [<8000dfc0>] (ret_fast_syscall+0x0/0x30)
---[ end trace 16d080eb7efea4ea ]---
------------[ cut here ]------------
Greg Kroah-Hartman June 9, 2013, 5:26 p.m. UTC | #3
On Sun, Jun 09, 2013 at 09:05:47PM +0800, Shawn Guo wrote:
> On Sun, Jun 09, 2013 at 04:34:11PM +0800, Shawn Guo wrote:
> > On Sun, Jun 09, 2013 at 10:01:19AM +0800, Huang Shijie wrote:
> > > The console's clocks are disabled after the uart driver is probed.
> > > It makes that we can see less log from the console now
> > > (though we still can get all the log by the `dmesg`).
> > > 
> > > So enable the clocks for console, and we can see all the log again.
> > > 
> > > This patch also disables the sport->clk_per when we fail to enable
> > > the sport->clk_ipg;
> > > 
> > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > 
> > Tested-by: Shawn Guo <shawn.guo@linaro.org>
> 
> Sorry.  I have to take that back.
> 
> Though it works fine with BusyBox, I'm getting the following warnings
> when booting with an Ubuntu 13.04 rootfs, in which case imx_startup()
> and imx_shutdown() will be called for a number of iterations.  That
> causes unbalanced clk_prepare_enable vs. clk_disable_unprepare, since
> the clk_prepare_enable is under condition if (!uart_console(port)) in
> imx_startup(), while clk_disable_unprepare in imx_shutdown() is not.

I had just applied this to my tree, should I revert it because of this
problem?

thanks,

greg k-h
Shawn Guo June 10, 2013, 12:55 a.m. UTC | #4
On Sun, Jun 09, 2013 at 10:26:28AM -0700, Greg KH wrote:
> On Sun, Jun 09, 2013 at 09:05:47PM +0800, Shawn Guo wrote:
> > On Sun, Jun 09, 2013 at 04:34:11PM +0800, Shawn Guo wrote:
> > > On Sun, Jun 09, 2013 at 10:01:19AM +0800, Huang Shijie wrote:
> > > > The console's clocks are disabled after the uart driver is probed.
> > > > It makes that we can see less log from the console now
> > > > (though we still can get all the log by the `dmesg`).
> > > > 
> > > > So enable the clocks for console, and we can see all the log again.
> > > > 
> > > > This patch also disables the sport->clk_per when we fail to enable
> > > > the sport->clk_ipg;
> > > > 
> > > > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > > 
> > > Tested-by: Shawn Guo <shawn.guo@linaro.org>
> > 
> > Sorry.  I have to take that back.
> > 
> > Though it works fine with BusyBox, I'm getting the following warnings
> > when booting with an Ubuntu 13.04 rootfs, in which case imx_startup()
> > and imx_shutdown() will be called for a number of iterations.  That
> > causes unbalanced clk_prepare_enable vs. clk_disable_unprepare, since
> > the clk_prepare_enable is under condition if (!uart_console(port)) in
> > imx_startup(), while clk_disable_unprepare in imx_shutdown() is not.
> 
> I had just applied this to my tree, should I revert it because of this
> problem?

Fabio just sent an incremental fix "serial: imx: Fix serial clock
unbalance" for that.

Shawn
diff mbox

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 7a761f7..78f8097 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -702,13 +702,16 @@  static int imx_startup(struct uart_port *port)
 	int retval;
 	unsigned long flags, temp;
 
-	retval = clk_prepare_enable(sport->clk_per);
-	if (retval)
-		goto error_out1;
-
-	retval = clk_prepare_enable(sport->clk_ipg);
-	if (retval)
-		goto error_out1;
+	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;
+		}
+	}
 
 	imx_setup_ufcr(sport, 0);
 
@@ -1578,8 +1581,10 @@  static int serial_imx_probe(struct platform_device *pdev)
 		goto deinit;
 	platform_set_drvdata(pdev, sport);
 
-	clk_disable_unprepare(sport->clk_per);
-	clk_disable_unprepare(sport->clk_ipg);
+	if (!uart_console(&sport->port)) {
+		clk_disable_unprepare(sport->clk_per);
+		clk_disable_unprepare(sport->clk_ipg);
+	}
 
 	return 0;
 deinit: