* bug in PL011 console
@ 2010-12-23 15:02 Uwe Kleine-König
2010-12-23 15:08 ` Russell King - ARM Linux
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2010-12-23 15:02 UTC (permalink / raw)
To: linux-arm-kernel
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/ |
^ permalink raw reply [flat|nested] 10+ messages in thread* bug in PL011 console 2010-12-23 15:02 bug in PL011 console Uwe Kleine-König @ 2010-12-23 15:08 ` Russell King - ARM Linux 2010-12-23 15:42 ` Uwe Kleine-König 2010-12-23 15:21 ` [PATCH] serial/amba-pl011: enable uart clk in setup for console Uwe Kleine-König 2010-12-27 11:57 ` bug in PL011 console Shawn Guo 2 siblings, 1 reply; 10+ messages in thread From: Russell King - ARM Linux @ 2010-12-23 15:08 UTC (permalink / raw) To: linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-23 15:08 ` Russell King - ARM Linux @ 2010-12-23 15:42 ` Uwe Kleine-König 2010-12-24 5:49 ` Richard Zhao 2010-12-24 6:35 ` Jassi Brar 0 siblings, 2 replies; 10+ messages in thread From: Uwe Kleine-König @ 2010-12-23 15:42 UTC (permalink / raw) To: linux-arm-kernel 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/ | ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-23 15:42 ` Uwe Kleine-König @ 2010-12-24 5:49 ` Richard Zhao 2010-12-24 6:35 ` Jassi Brar 1 sibling, 0 replies; 10+ messages in thread From: Richard Zhao @ 2010-12-24 5:49 UTC (permalink / raw) To: linux-arm-kernel 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/ ?| > ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-23 15:42 ` Uwe Kleine-König 2010-12-24 5:49 ` Richard Zhao @ 2010-12-24 6:35 ` Jassi Brar 2010-12-24 7:10 ` Richard Zhao 1 sibling, 1 reply; 10+ messages in thread From: Jassi Brar @ 2010-12-24 6:35 UTC (permalink / raw) To: linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-24 6:35 ` Jassi Brar @ 2010-12-24 7:10 ` Richard Zhao 2010-12-27 1:00 ` Jassi Brar 0 siblings, 1 reply; 10+ messages in thread From: Richard Zhao @ 2010-12-24 7:10 UTC (permalink / raw) To: linux-arm-kernel 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. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-24 7:10 ` Richard Zhao @ 2010-12-27 1:00 ` Jassi Brar 0 siblings, 0 replies; 10+ messages in thread From: Jassi Brar @ 2010-12-27 1:00 UTC (permalink / raw) To: linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] serial/amba-pl011: enable uart clk in setup for console 2010-12-23 15:02 bug in PL011 console Uwe Kleine-König 2010-12-23 15:08 ` Russell King - ARM Linux @ 2010-12-23 15:21 ` Uwe Kleine-König 2010-12-27 11:57 ` bug in PL011 console Shawn Guo 2 siblings, 0 replies; 10+ messages in thread From: Uwe Kleine-König @ 2010-12-23 15:21 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-23 15:02 bug in PL011 console Uwe Kleine-König 2010-12-23 15:08 ` Russell King - ARM Linux 2010-12-23 15:21 ` [PATCH] serial/amba-pl011: enable uart clk in setup for console Uwe Kleine-König @ 2010-12-27 11:57 ` Shawn Guo 2010-12-27 20:37 ` Uwe Kleine-König 2 siblings, 1 reply; 10+ messages in thread From: Shawn Guo @ 2010-12-27 11:57 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug in PL011 console 2010-12-27 11:57 ` bug in PL011 console Shawn Guo @ 2010-12-27 20:37 ` Uwe Kleine-König 0 siblings, 0 replies; 10+ messages in thread From: Uwe Kleine-König @ 2010-12-27 20:37 UTC (permalink / raw) To: linux-arm-kernel 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/ | ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-12-27 20:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-12-23 15:02 bug in PL011 console Uwe Kleine-König 2010-12-23 15:08 ` Russell King - ARM Linux 2010-12-23 15:42 ` Uwe Kleine-König 2010-12-24 5:49 ` Richard Zhao 2010-12-24 6:35 ` Jassi Brar 2010-12-24 7:10 ` Richard Zhao 2010-12-27 1:00 ` Jassi Brar 2010-12-23 15:21 ` [PATCH] serial/amba-pl011: enable uart clk in setup for console Uwe Kleine-König 2010-12-27 11:57 ` bug in PL011 console Shawn Guo 2010-12-27 20:37 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).