bug in PL011 console

bug in PL011 console

[Uwe Kleine-König]

Hello,

since mxs switched to amba-pl011 I often hit warnings like this:

	[    9.280000] udevd (74): /proc/74/oom_adj is deprecated, please use /proc/74/oom_score_adj instead.
	[    9.280000] BUG: sleeping function called from invalid context at /ptx/work/octopus/WORK_2_A/ukl/backup/gsrc/linux-2.6/kernel/mutex.c:278
	[    9.280000] in_atomic(): 0, irqs_disabled(): 128, pid: 74, name: udevd
	[    9.280000] 2 locks held by udevd/74:
	[    9.280000]  #0:  (&(&p->alloc_lock)->rlock){+.+...}, at: [<c0132ff8>] oom_adjust_write+0xf8/0x290
	[    9.280000]  #1:  (&(&sighand->siglock)->rlock){......}, at: [<c005d1ec>] __lock_task_sighand+0x64/0xac
	[    9.280000] irq event stamp: 673
	[    9.280000] hardirqs last  enabled at (672): [<c0033fdc>] vector_swi+0x3c/0x90
	[    9.280000] hardirqs last disabled at (673): [<c02882e0>] _raw_spin_lock_irqsave+0x30/0x64
	[    9.280000] softirqs last  enabled at (0): [<c004a000>] copy_process+0x3a8/0xf90
	[    9.280000] softirqs last disabled at (0): [<  (null)>]   (null)
	[    9.280000] Backtrace: 
	[    9.280000] [<c0037b54>] (dump_backtrace+0x0/0x110) from [<c02853a4>] (dump_stack+0x1c/0x20)
	[    9.280000]  r7:c79f0000 r6:c003dd00 r5:c030ce72 r4:c79f0000
	[    9.280000] [<c0285388>] (dump_stack+0x0/0x20) from [<c0046a58>] (__might_sleep+0x100/0x120)
	[    9.280000] [<c0046958>] (__might_sleep+0x0/0x120) from [<c0287054>] (mutex_lock_nested+0x3c/0x2b4)
	[    9.280000]  r5:00000000 r4:00000000
	[    9.280000] [<c0287018>] (mutex_lock_nested+0x0/0x2b4) from [<c003dd00>] (clk_enable+0x30/0x58)
	[    9.280000] [<c003dcd0>] (clk_enable+0x0/0x58) from [<c01d9888>] (pl011_console_write+0x30/0x8c)
	[    9.280000]  r4:00000066
	[    9.280000] [<c01d9858>] (pl011_console_write+0x0/0x8c) from [<c004b9a4>] (__call_console_drivers+0x68/0x84)
	[    9.280000]  r6:00000066 r5:00007401 r4:c0372950
	[    9.280000] [<c004b93c>] (__call_console_drivers+0x0/0x84) from [<c004ba48>] (_call_console_drivers+0x88/0x9c)
	[    9.280000]  r8:ffff8b99 r7:60000093 r6:c0352c24 r5:c0352ba0 r4:00007467
	[    9.280000] [<c004b9c0>] (_call_console_drivers+0x0/0x9c) from [<c004c040>] (release_console_sem+0x158/0x244)
	[    9.280000]  r5:00007467 r4:00007467
	[    9.280000] [<c004bee8>] (release_console_sem+0x0/0x244) from [<c004c7fc>] (vprintk+0x388/0x404)
	[    9.280000] [<c004c474>] (vprintk+0x0/0x404) from [<c0285560>] (printk+0x20/0x28)
	[    9.280000] [<c0285540>] (printk+0x0/0x28) from [<c01330f0>] (oom_adjust_write+0x1f0/0x290)
	[    9.280000]  r3:0000004a r2:0000004a r1:c795595c r0:c031998c
	[    9.280000] [<c0132f00>] (oom_adjust_write+0x0/0x290) from [<c00eb008>] (vfs_write+0xb8/0x18c)
	[    9.280000]  r8:0001a4ac r7:c79f1f70 r6:0001a4ac r5:00000003 r4:c79dbc00
	[    9.280000] [<c00eaf50>] (vfs_write+0x0/0x18c) from [<c00eb1a8>] (sys_write+0x48/0x74)
	[    9.280000]  r8:0001a4ac r7:00000003 r6:c79dbc00 r5:00000000 r4:00000000
	[    9.280000] [<c00eb160>] (sys_write+0x0/0x74) from [<c0033e80>] (ret_fast_syscall+0x0/0x38)
	[    9.280000]  r8:c0034088 r7:00000004 r6:00000000 r5:00000003 r4:00000003

Steven told me on irc that sleeping was not allowed in the console write
callback.  Maybe this didn't show up earlier because not all clk
implementations sleep as mxs' does.

I think the only possible fix is to do the clk_enable in the setup
callback instead of per-write.

Will send a patch as follow up.

Best regards
Uwe

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

bug in PL011 console

[Russell King - ARM Linux]

On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
> Steven told me on irc that sleeping was not allowed in the console write
> callback.  Maybe this didn't show up earlier because not all clk
> implementations sleep as mxs' does.
> 
> I think the only possible fix is to do the clk_enable in the setup
> callback instead of per-write.
> 
> Will send a patch as follow up.

We really need to sort out what's expected from the CLK API.  The drivers
I write assume that it's absolutely fine to call clk_enable/clk_disable
from IRQ context, and for the platforms I implemented the CLK API for,
that's absolutely true.

I'd lobby for it because it allows for proper power saving management of
clocks for devices - PL011 only enables the clock when either the port is
open or it's actually sending data out the port.  So it's doing absolutely
the best power management that can be done with UARTs.

[PATCH] serial/amba-pl011: enable uart clk in setup for console

[Uwe Kleine-König]

The .write callback for console drivers must not sleep.  So
pl011_console_write must call clk_enable that is allowed to sleep (and
does sleep on mxs).

So enable the clock in the .setup callback instead of .write.

This fixes bugs like:

	udevd (74): /proc/74/oom_adj is deprecated, please use /proc/74/oom_score_adj instead.
	BUG: sleeping function called from invalid context at /ptx/work/octopus/WORK_2_A/ukl/backup/gsrc/linux-2.6/kernel/mutex.c:278
	in_atomic(): 0, irqs_disabled(): 128, pid: 74, name: udevd
	2 locks held by udevd/74:
	 #0:  (&(&p->alloc_lock)->rlock){+.+...}, at: [<c0132ff8>] oom_adjust_write+0xf8/0x290
	 #1:  (&(&sighand->siglock)->rlock){......}, at: [<c005d1ec>] __lock_task_sighand+0x64/0xac
	irq event stamp: 673
	hardirqs last  enabled at (672): [<c0033fdc>] vector_swi+0x3c/0x90
	hardirqs last disabled at (673): [<c02882e0>] _raw_spin_lock_irqsave+0x30/0x64
	softirqs last  enabled at (0): [<c004a000>] copy_process+0x3a8/0xf90
	softirqs last disabled at (0): [<  (null)>]   (null)
	Backtrace:
	[<c0037b54>] (dump_backtrace+0x0/0x110) from [<c02853a4>] (dump_stack+0x1c/0x20)
	 r7:c79f0000 r6:c003dd00 r5:c030ce72 r4:c79f0000
	[<c0285388>] (dump_stack+0x0/0x20) from [<c0046a58>] (__might_sleep+0x100/0x120)
	[<c0046958>] (__might_sleep+0x0/0x120) from [<c0287054>] (mutex_lock_nested+0x3c/0x2b4)
	 r5:00000000 r4:00000000
	[<c0287018>] (mutex_lock_nested+0x0/0x2b4) from [<c003dd00>] (clk_enable+0x30/0x58)
	[<c003dcd0>] (clk_enable+0x0/0x58) from [<c01d9888>] (pl011_console_write+0x30/0x8c)
	 r4:00000066
	[<c01d9858>] (pl011_console_write+0x0/0x8c) from [<c004b9a4>] (__call_console_drivers+0x68/0x84)
	 r6:00000066 r5:00007401 r4:c0372950
	[<c004b93c>] (__call_console_drivers+0x0/0x84) from [<c004ba48>] (_call_console_drivers+0x88/0x9c)
	 r8:ffff8b99 r7:60000093 r6:c0352c24 r5:c0352ba0 r4:00007467
	[<c004b9c0>] (_call_console_drivers+0x0/0x9c) from [<c004c040>] (release_console_sem+0x158/0x244)
	 r5:00007467 r4:00007467
	[<c004bee8>] (release_console_sem+0x0/0x244) from [<c004c7fc>] (vprintk+0x388/0x404)
	[<c004c474>] (vprintk+0x0/0x404) from [<c0285560>] (printk+0x20/0x28)
	[<c0285540>] (printk+0x0/0x28) from [<c01330f0>] (oom_adjust_write+0x1f0/0x290)
	 r3:0000004a r2:0000004a r1:c795595c r0:c031998c
	[<c0132f00>] (oom_adjust_write+0x0/0x290) from [<c00eb008>] (vfs_write+0xb8/0x18c)
	 r8:0001a4ac r7:c79f1f70 r6:0001a4ac r5:00000003 r4:c79dbc00
	[<c00eaf50>] (vfs_write+0x0/0x18c) from [<c00eb1a8>] (sys_write+0x48/0x74)
	 r8:0001a4ac r7:00000003 r6:c79dbc00 r5:00000000 r4:00000000
	[<c00eb160>] (sys_write+0x0/0x74) from [<c0033e80>] (ret_fast_syscall+0x0/0x38)
	 r8:c0034088 r7:00000004 r6:00000000 r5:00000003 r4:00000003

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Linus Walleij <linus.walleij@stericsson.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 drivers/serial/amba-pl011.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/serial/amba-pl011.c b/drivers/serial/amba-pl011.c
index 6ca7a44..e890ae8 100644
--- a/drivers/serial/amba-pl011.c
+++ b/drivers/serial/amba-pl011.c
@@ -710,8 +710,6 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
 	struct uart_amba_port *uap = amba_ports[co->index];
 	unsigned int status, old_cr, new_cr;
 
-	clk_enable(uap->clk);
-
 	/*
 	 *	First save the CR then disable the interrupts
 	 */
@@ -730,8 +728,6 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
 		status = readw(uap->port.membase + UART01x_FR);
 	} while (status & UART01x_FR_BUSY);
 	writew(old_cr, uap->port.membase + UART011_CR);
-
-	clk_disable(uap->clk);
 }
 
 static void __init
@@ -776,6 +772,7 @@ static int __init pl011_console_setup(struct console *co, char *options)
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';
+	int ret;
 
 	/*
 	 * Check whether an invalid uart number has been specified, and
@@ -788,6 +785,10 @@ static int __init pl011_console_setup(struct console *co, char *options)
 	if (!uap)
 		return -ENODEV;
 
+	ret = clk_enable(uap->clk);
+	if (ret)
+		return ret;
+
 	uap->port.uartclk = clk_get_rate(uap->clk);
 
 	if (options)
-- 
1.7.2.3

bug in PL011 console

[Uwe Kleine-König]

On Thu, Dec 23, 2010 at 03:08:41PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
> > Steven told me on irc that sleeping was not allowed in the console write
> > callback.  Maybe this didn't show up earlier because not all clk
> > implementations sleep as mxs' does.
> > 
> > I think the only possible fix is to do the clk_enable in the setup
> > callback instead of per-write.
> > 
> > Will send a patch as follow up.
> 
> We really need to sort out what's expected from the CLK API.  The drivers
> I write assume that it's absolutely fine to call clk_enable/clk_disable
> from IRQ context, and for the platforms I implemented the CLK API for,
> that's absolutely true.
The common struct clk patch[1] by Jeremy Kerr sleeps, too.  And I think
most people who commented to this series thought that this is the right
behaviour.

> I'd lobby for it because it allows for proper power saving management of
> clocks for devices - PL011 only enables the clock when either the port is
> open or it's actually sending data out the port.  So it's doing absolutely
> the best power management that can be done with UARTs.
Yeah, that makes fine-grained clk enabling harder/impossible.  So
ideally we'd have something that only makes clk_enable sleep iff
that's sensible for that clk.  And if you have a clock that can be
enabled "fast" it would not sleep.

Don't know if that works, maybe something like that:

	int clk_enable(struct clk *clk)
	{
		spin_lock(something);
		if (clk->flags & (SOME|FLAGS))
			goto out_busy;
		clk->flags |= ENABLING;
		spin_unlock(something);

		ret = clk->really_enable(...);

		spin_lock(something);
		clk->flags &= ~ENABLING;
		spin_unlock(something);
	}

Some things that need careful consideration are:

	- clk->flags already has ENABLING when clk_enable is entered.
	  (needs to sleep/poll then?)
	- clk->usecount already > 0
	  (early return unless clk->flags & DISABLING)
	- do we need the irqsaving spinlock variants?
	  (I assume yes)

Probably there are more.

Best regards
Uwe

[1] last submission: http://thread.gmane.org/gmane.linux.kernel/1073751

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

bug in PL011 console

[Richard Zhao]

2010/12/23 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Thu, Dec 23, 2010 at 03:08:41PM +0000, Russell King - ARM Linux wrote:
>> On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
>> > Steven told me on irc that sleeping was not allowed in the console write
>> > callback. ?Maybe this didn't show up earlier because not all clk
>> > implementations sleep as mxs' does.
>> >
>> > I think the only possible fix is to do the clk_enable in the setup
>> > callback instead of per-write.
>> >
>> > Will send a patch as follow up.
>>
>> We really need to sort out what's expected from the CLK API. ?The drivers
>> I write assume that it's absolutely fine to call clk_enable/clk_disable
>> from IRQ context, and for the platforms I implemented the CLK API for,
>> that's absolutely true.
> The common struct clk patch[1] by Jeremy Kerr sleeps, too. ?And I think
> most people who commented to this series thought that this is the right
> behaviour.
I vote atomic clock. All pure clock operations are atomic. I suggest
voltage change is not included in clock API. If we need a finegrained
power management, when we open a device, we can first request some
power conditions and release the power conditions when we close it.
Mixing sleepy and tomic functions together is likely making mess.

Thanks
Richard
>
>> I'd lobby for it because it allows for proper power saving management of
>> clocks for devices - PL011 only enables the clock when either the port is
>> open or it's actually sending data out the port. ?So it's doing absolutely
>> the best power management that can be done with UARTs.
> Yeah, that makes fine-grained clk enabling harder/impossible. ?So
> ideally we'd have something that only makes clk_enable sleep iff
> that's sensible for that clk. ?And if you have a clock that can be
> enabled "fast" it would not sleep.
>
> Don't know if that works, maybe something like that:
>
> ? ? ? ?int clk_enable(struct clk *clk)
> ? ? ? ?{
> ? ? ? ? ? ? ? ?spin_lock(something);
> ? ? ? ? ? ? ? ?if (clk->flags & (SOME|FLAGS))
> ? ? ? ? ? ? ? ? ? ? ? ?goto out_busy;
> ? ? ? ? ? ? ? ?clk->flags |= ENABLING;
> ? ? ? ? ? ? ? ?spin_unlock(something);
>
> ? ? ? ? ? ? ? ?ret = clk->really_enable(...);
>
> ? ? ? ? ? ? ? ?spin_lock(something);
> ? ? ? ? ? ? ? ?clk->flags &= ~ENABLING;
> ? ? ? ? ? ? ? ?spin_unlock(something);
> ? ? ? ?}
>
> Some things that need careful consideration are:
>
> ? ? ? ?- clk->flags already has ENABLING when clk_enable is entered.
> ? ? ? ? ?(needs to sleep/poll then?)
> ? ? ? ?- clk->usecount already > 0
> ? ? ? ? ?(early return unless clk->flags & DISABLING)
> ? ? ? ?- do we need the irqsaving spinlock variants?
> ? ? ? ? ?(I assume yes)
>
> Probably there are more.
>
> Best regards
> Uwe
>
> [1] last submission: http://thread.gmane.org/gmane.linux.kernel/1073751
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>

bug in PL011 console

[Jassi Brar]

2010/12/24 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Thu, Dec 23, 2010 at 03:08:41PM +0000, Russell King - ARM Linux wrote:
>> On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
>> > Steven told me on irc that sleeping was not allowed in the console write
>> > callback. ?Maybe this didn't show up earlier because not all clk
>> > implementations sleep as mxs' does.
>> >
>> > I think the only possible fix is to do the clk_enable in the setup
>> > callback instead of per-write.
>> >
>> > Will send a patch as follow up.
>>
>> We really need to sort out what's expected from the CLK API. ?The drivers
>> I write assume that it's absolutely fine to call clk_enable/clk_disable
>> from IRQ context, and for the platforms I implemented the CLK API for,
>> that's absolutely true.
> The common struct clk patch[1] by Jeremy Kerr sleeps, too. ?And I think
> most people who commented to this series thought that this is the right
> behaviour.

Personally, I am not as against atomic clocks as having two versions
of the clock - atomic
and sleepable, in the API.
I favor sleepable version because with the clock hierarchies becoming so complex
in latest SoCs, it would be difficult to write a 'smart' clock
management sub-system
that could setup and enable all 'parent' clocks upon enabling a clock.
Especially, managing mux'es at levels much lower than pll. FWIK,
drivers have tough
time managing such parent clocks.

bug in PL011 console

[Richard Zhao]

2010/12/24 Jassi Brar <jassisinghbrar@gmail.com>:
> 2010/12/24 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>> On Thu, Dec 23, 2010 at 03:08:41PM +0000, Russell King - ARM Linux wrote:
>>> On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
>>> > Steven told me on irc that sleeping was not allowed in the console write
>>> > callback. ?Maybe this didn't show up earlier because not all clk
>>> > implementations sleep as mxs' does.
>>> >
>>> > I think the only possible fix is to do the clk_enable in the setup
>>> > callback instead of per-write.
>>> >
>>> > Will send a patch as follow up.
>>>
>>> We really need to sort out what's expected from the CLK API. ?The drivers
>>> I write assume that it's absolutely fine to call clk_enable/clk_disable
>>> from IRQ context, and for the platforms I implemented the CLK API for,
>>> that's absolutely true.
>> The common struct clk patch[1] by Jeremy Kerr sleeps, too. ?And I think
>> most people who commented to this series thought that this is the right
>> behaviour.
>
> Personally, I am not as against atomic clocks as having two versions
> of the clock - atomic
> and sleepable, in the API.
> I favor sleepable version because with the clock hierarchies becoming so complex
> in latest SoCs, it would be difficult to write a 'smart' clock
> management sub-system
Can you let some intermediate clock on to shorten the time? It might
be better to use some new global power state management to let clock
become smart. Clock itself becomes simple.

Thanks
Richard
> that could setup and enable all 'parent' clocks upon enabling a clock.
> Especially, managing mux'es at levels much lower than pll. FWIK,
> drivers have tough
> time managing such parent clocks.
>

bug in PL011 console

[Jassi Brar]

On Fri, Dec 24, 2010 at 4:10 PM, Richard Zhao <linuxzsc@gmail.com> wrote:
> 2010/12/24 Jassi Brar <jassisinghbrar@gmail.com>:
>> 2010/12/24 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>>> On Thu, Dec 23, 2010 at 03:08:41PM +0000, Russell King - ARM Linux wrote:
>>>> On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
>>>> > Steven told me on irc that sleeping was not allowed in the console write
>>>> > callback. ?Maybe this didn't show up earlier because not all clk
>>>> > implementations sleep as mxs' does.
>>>> >
>>>> > I think the only possible fix is to do the clk_enable in the setup
>>>> > callback instead of per-write.
>>>> >
>>>> > Will send a patch as follow up.
>>>>
>>>> We really need to sort out what's expected from the CLK API. ?The drivers
>>>> I write assume that it's absolutely fine to call clk_enable/clk_disable
>>>> from IRQ context, and for the platforms I implemented the CLK API for,
>>>> that's absolutely true.
>>> The common struct clk patch[1] by Jeremy Kerr sleeps, too. ?And I think
>>> most people who commented to this series thought that this is the right
>>> behaviour.
>>
>> Personally, I am not as against atomic clocks as having two versions
>> of the clock - atomic
>> and sleepable, in the API.
>> I favor sleepable version because with the clock hierarchies becoming so complex
>> in latest SoCs, it would be difficult to write a 'smart' clock
>> management sub-system
> Can you let some intermediate clock on to shorten the time? It might
> be better to use some new global power state management to let clock
> become smart. Clock itself becomes simple.

Isn't a very neat way, imho. Who decides which clocks are enabled by
default - Platform or Machine code? Not to forget, power saving is one
of the intended benefits of
being able to manage clocks atomically and here we would be forced to keep
clocks enabled due to that very 'feature'.

Also, please remember that some intermediate clock sources (eg. scalars)
could provide to more than one peripheral, and it would be nice to have
mechanism to figure out if and when a peripheral can safely modify the clock to
its own need. And for that, AFAICT, we need to poll other users of the clock and
see if they can manage working with different speeds.

bug in PL011 console

[Shawn Guo]

On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
> Hello,
> 
> since mxs switched to amba-pl011 I often hit warnings like this:
> 
> 	[    9.280000] udevd (74): /proc/74/oom_adj is deprecated, please use /proc/74/oom_score_adj instead.
> 	[    9.280000] BUG: sleeping function called from invalid context at /ptx/work/octopus/WORK_2_A/ukl/backup/gsrc/linux-2.6/kernel/mutex.c:278
> 	[    9.280000] in_atomic(): 0, irqs_disabled(): 128, pid: 74, name: udevd
> 	[    9.280000] 2 locks held by udevd/74:
> 	[    9.280000]  #0:  (&(&p->alloc_lock)->rlock){+.+...}, at: [<c0132ff8>] oom_adjust_write+0xf8/0x290
> 	[    9.280000]  #1:  (&(&sighand->siglock)->rlock){......}, at: [<c005d1ec>] __lock_task_sighand+0x64/0xac
> 	[    9.280000] irq event stamp: 673
> 	[    9.280000] hardirqs last  enabled at (672): [<c0033fdc>] vector_swi+0x3c/0x90
> 	[    9.280000] hardirqs last disabled at (673): [<c02882e0>] _raw_spin_lock_irqsave+0x30/0x64
> 	[    9.280000] softirqs last  enabled at (0): [<c004a000>] copy_process+0x3a8/0xf90
> 	[    9.280000] softirqs last disabled at (0): [<  (null)>]   (null)
> 	[    9.280000] Backtrace: 
> 	[    9.280000] [<c0037b54>] (dump_backtrace+0x0/0x110) from [<c02853a4>] (dump_stack+0x1c/0x20)
> 	[    9.280000]  r7:c79f0000 r6:c003dd00 r5:c030ce72 r4:c79f0000
> 	[    9.280000] [<c0285388>] (dump_stack+0x0/0x20) from [<c0046a58>] (__might_sleep+0x100/0x120)
> 	[    9.280000] [<c0046958>] (__might_sleep+0x0/0x120) from [<c0287054>] (mutex_lock_nested+0x3c/0x2b4)
> 	[    9.280000]  r5:00000000 r4:00000000
> 	[    9.280000] [<c0287018>] (mutex_lock_nested+0x0/0x2b4) from [<c003dd00>] (clk_enable+0x30/0x58)
> 	[    9.280000] [<c003dcd0>] (clk_enable+0x0/0x58) from [<c01d9888>] (pl011_console_write+0x30/0x8c)
> 	[    9.280000]  r4:00000066
> 	[    9.280000] [<c01d9858>] (pl011_console_write+0x0/0x8c) from [<c004b9a4>] (__call_console_drivers+0x68/0x84)
> 	[    9.280000]  r6:00000066 r5:00007401 r4:c0372950
> 	[    9.280000] [<c004b93c>] (__call_console_drivers+0x0/0x84) from [<c004ba48>] (_call_console_drivers+0x88/0x9c)
> 	[    9.280000]  r8:ffff8b99 r7:60000093 r6:c0352c24 r5:c0352ba0 r4:00007467
> 	[    9.280000] [<c004b9c0>] (_call_console_drivers+0x0/0x9c) from [<c004c040>] (release_console_sem+0x158/0x244)
> 	[    9.280000]  r5:00007467 r4:00007467
> 	[    9.280000] [<c004bee8>] (release_console_sem+0x0/0x244) from [<c004c7fc>] (vprintk+0x388/0x404)
> 	[    9.280000] [<c004c474>] (vprintk+0x0/0x404) from [<c0285560>] (printk+0x20/0x28)
> 	[    9.280000] [<c0285540>] (printk+0x0/0x28) from [<c01330f0>] (oom_adjust_write+0x1f0/0x290)
> 	[    9.280000]  r3:0000004a r2:0000004a r1:c795595c r0:c031998c
> 	[    9.280000] [<c0132f00>] (oom_adjust_write+0x0/0x290) from [<c00eb008>] (vfs_write+0xb8/0x18c)
> 	[    9.280000]  r8:0001a4ac r7:c79f1f70 r6:0001a4ac r5:00000003 r4:c79dbc00
> 	[    9.280000] [<c00eaf50>] (vfs_write+0x0/0x18c) from [<c00eb1a8>] (sys_write+0x48/0x74)
> 	[    9.280000]  r8:0001a4ac r7:00000003 r6:c79dbc00 r5:00000000 r4:00000000
> 	[    9.280000] [<c00eb160>] (sys_write+0x0/0x74) from [<c0033e80>] (ret_fast_syscall+0x0/0x38)
> 	[    9.280000]  r8:c0034088 r7:00000004 r6:00000000 r5:00000003 r4:00000003
> 
It looks like a real problem. But the strange thing to me is that
it's never been hit on my side.

> Steven told me on irc that sleeping was not allowed in the console write
> callback.  Maybe this didn't show up earlier because not all clk
> implementations sleep as mxs' does.
> 
> I think the only possible fix is to do the clk_enable in the setup
> callback instead of per-write.
> 
> Will send a patch as follow up.
> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-K?nig            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Regards,
Shawn

bug in PL011 console

[Uwe Kleine-König]

Hello Shawn,

On Mon, Dec 27, 2010 at 07:57:19PM +0800, Shawn Guo wrote:
> On Thu, Dec 23, 2010 at 04:02:34PM +0100, Uwe Kleine-K?nig wrote:
> > Hello,
> > 
> > since mxs switched to amba-pl011 I often hit warnings like this:
> > 
> > 	[    9.280000] udevd (74): /proc/74/oom_adj is deprecated, please use /proc/74/oom_score_adj instead.
> > 	[    9.280000] BUG: sleeping function called from invalid context at /ptx/work/octopus/WORK_2_A/ukl/backup/gsrc/linux-2.6/kernel/mutex.c:278
> > 	[    9.280000] in_atomic(): 0, irqs_disabled(): 128, pid: 74, name: udevd
> > 	[    9.280000] 2 locks held by udevd/74:
> > 	[    9.280000]  #0:  (&(&p->alloc_lock)->rlock){+.+...}, at: [<c0132ff8>] oom_adjust_write+0xf8/0x290
> > 	[    9.280000]  #1:  (&(&sighand->siglock)->rlock){......}, at: [<c005d1ec>] __lock_task_sighand+0x64/0xac
> > 	[    9.280000] irq event stamp: 673
> > 	[    9.280000] hardirqs last  enabled at (672): [<c0033fdc>] vector_swi+0x3c/0x90
> > 	[    9.280000] hardirqs last disabled at (673): [<c02882e0>] _raw_spin_lock_irqsave+0x30/0x64
> > 	[    9.280000] softirqs last  enabled at (0): [<c004a000>] copy_process+0x3a8/0xf90
> > 	[    9.280000] softirqs last disabled at (0): [<  (null)>]   (null)
> > 	[    9.280000] Backtrace: 
> > 	[    9.280000] [<c0037b54>] (dump_backtrace+0x0/0x110) from [<c02853a4>] (dump_stack+0x1c/0x20)
> > 	[    9.280000]  r7:c79f0000 r6:c003dd00 r5:c030ce72 r4:c79f0000
> > 	[    9.280000] [<c0285388>] (dump_stack+0x0/0x20) from [<c0046a58>] (__might_sleep+0x100/0x120)
> > 	[    9.280000] [<c0046958>] (__might_sleep+0x0/0x120) from [<c0287054>] (mutex_lock_nested+0x3c/0x2b4)
> > 	[    9.280000]  r5:00000000 r4:00000000
> > 	[    9.280000] [<c0287018>] (mutex_lock_nested+0x0/0x2b4) from [<c003dd00>] (clk_enable+0x30/0x58)
> > 	[    9.280000] [<c003dcd0>] (clk_enable+0x0/0x58) from [<c01d9888>] (pl011_console_write+0x30/0x8c)
> > 	[    9.280000]  r4:00000066
> > 	[    9.280000] [<c01d9858>] (pl011_console_write+0x0/0x8c) from [<c004b9a4>] (__call_console_drivers+0x68/0x84)
> > 	[    9.280000]  r6:00000066 r5:00007401 r4:c0372950
> > 	[    9.280000] [<c004b93c>] (__call_console_drivers+0x0/0x84) from [<c004ba48>] (_call_console_drivers+0x88/0x9c)
> > 	[    9.280000]  r8:ffff8b99 r7:60000093 r6:c0352c24 r5:c0352ba0 r4:00007467
> > 	[    9.280000] [<c004b9c0>] (_call_console_drivers+0x0/0x9c) from [<c004c040>] (release_console_sem+0x158/0x244)
> > 	[    9.280000]  r5:00007467 r4:00007467
> > 	[    9.280000] [<c004bee8>] (release_console_sem+0x0/0x244) from [<c004c7fc>] (vprintk+0x388/0x404)
> > 	[    9.280000] [<c004c474>] (vprintk+0x0/0x404) from [<c0285560>] (printk+0x20/0x28)
> > 	[    9.280000] [<c0285540>] (printk+0x0/0x28) from [<c01330f0>] (oom_adjust_write+0x1f0/0x290)
> > 	[    9.280000]  r3:0000004a r2:0000004a r1:c795595c r0:c031998c
> > 	[    9.280000] [<c0132f00>] (oom_adjust_write+0x0/0x290) from [<c00eb008>] (vfs_write+0xb8/0x18c)
> > 	[    9.280000]  r8:0001a4ac r7:c79f1f70 r6:0001a4ac r5:00000003 r4:c79dbc00
> > 	[    9.280000] [<c00eaf50>] (vfs_write+0x0/0x18c) from [<c00eb1a8>] (sys_write+0x48/0x74)
> > 	[    9.280000]  r8:0001a4ac r7:00000003 r6:c79dbc00 r5:00000000 r4:00000000
> > 	[    9.280000] [<c00eb160>] (sys_write+0x0/0x74) from [<c0033e80>] (ret_fast_syscall+0x0/0x38)
> > 	[    9.280000]  r8:c0034088 r7:00000004 r6:00000000 r5:00000003 r4:00000003
> > 
> It looks like a real problem. But the strange thing to me is that
> it's never been hit on my side.
Try enabling CONFIG_DEBUG_SPINLOCK_SLEEP.

Best regards
Uwe

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