* [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified @ 2011-07-07 20:32 Sebastian Andrzej Siewior 2011-07-28 9:39 ` [tip:irq/urgent] irq: Always " tip-bot for Sebastian Andrzej Siewior 2011-08-18 17:01 ` [PATCH] irq: always " Pantelis Antoniou 0 siblings, 2 replies; 19+ messages in thread From: Sebastian Andrzej Siewior @ 2011-07-07 20:32 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linux-kernel, Sebastian Andrzej Siewior If no primary handler is specified then a default one is assigned which always returns IRQ_WAKE_THREAD. This handler requires the IRQF_ONESHOT flag on LEVEL / EIO typed irqs because the source of interrupt is not disabled. Since it is required for those users and there is no difference for others it makes sense to add this flag unconditionally. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- A quick grep shows that drivers/staging/iio/adc/ad7816.c is the only driver in tree doing it wrong i.e. request_threaded_irq with IRQF_TRIGGER_{LOW|HIGH} but without ONEHOST. There are 59 users in tree which request an edge typed interrupt. 24 of them specify the ONEHOST flag the others don't. Both variants are valid and identical but a consistent behavior would be nice. There is a total of 134 users and 87 specify the irq type directly. For the remaining 47 the type depends on initial configuration of the irq chip or it might be specified via the device tree, dunno. Should that patch be accepted I suggest to remove the ONESHOT flag from every request_th.* which uses NULL as the primary handler so we have a consistent behavior here. kernel/irq/manage.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 0a7840ae..3f9cd47 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1322,6 +1322,7 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, if (!thread_fn) return -EINVAL; handler = irq_default_primary_handler; + irqflags |= IRQF_ONESHOT; } action = kzalloc(sizeof(struct irqaction), GFP_KERNEL); -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:irq/urgent] irq: Always set IRQF_ONESHOT if no primary handler is specified 2011-07-07 20:32 [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified Sebastian Andrzej Siewior @ 2011-07-28 9:39 ` tip-bot for Sebastian Andrzej Siewior 2011-08-18 17:01 ` [PATCH] irq: always " Pantelis Antoniou 1 sibling, 0 replies; 19+ messages in thread From: tip-bot for Sebastian Andrzej Siewior @ 2011-07-28 9:39 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, bigeasy, tglx Commit-ID: f3637a5f2e2eb391ff5757bc83fb5de8f9726464 Gitweb: http://git.kernel.org/tip/f3637a5f2e2eb391ff5757bc83fb5de8f9726464 Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de> AuthorDate: Thu, 7 Jul 2011 22:32:17 +0200 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 28 Jul 2011 11:23:21 +0200 irq: Always set IRQF_ONESHOT if no primary handler is specified If no primary handler is specified then a default one is assigned which always returns IRQ_WAKE_THREAD. This handler requires the IRQF_ONESHOT flag on LEVEL / EIO typed irqs because the source of interrupt is not disabled. Since it is required for those users and there is no difference for others it makes sense to add this flag unconditionally. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Link: http://lkml.kernel.org/r/1310070737-18514-1-git-send-email-bigeasy@linutronix.de Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- kernel/irq/manage.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index 0a7840ae..3f9cd47 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -1322,6 +1322,7 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, if (!thread_fn) return -EINVAL; handler = irq_default_primary_handler; + irqflags |= IRQF_ONESHOT; } action = kzalloc(sizeof(struct irqaction), GFP_KERNEL); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-07-07 20:32 [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified Sebastian Andrzej Siewior 2011-07-28 9:39 ` [tip:irq/urgent] irq: Always " tip-bot for Sebastian Andrzej Siewior @ 2011-08-18 17:01 ` Pantelis Antoniou 2011-08-18 17:22 ` Sebastian Andrzej Siewior 2011-08-22 23:26 ` Paul Walmsley 1 sibling, 2 replies; 19+ messages in thread From: Pantelis Antoniou @ 2011-08-18 17:01 UTC (permalink / raw) To: Sebastian Andrzej Siewior; +Cc: Thomas Gleixner, linux-kernel, mporter Hi there, Let me report that with this change Beagle board fails to boot, hangs right on rootfs mount. Users of BB should revert this until the offending driver(s) are fixed. Regards -- Pantelis On Jul 7, 2011, at 11:32 PM, Sebastian Andrzej Siewior wrote: > If no primary handler is specified then a default one is assigned which > always returns IRQ_WAKE_THREAD. This handler requires the IRQF_ONESHOT > flag on LEVEL / EIO typed irqs because the source of interrupt is not > disabled. > Since it is required for those users and there is no difference for others > it makes sense to add this flag unconditionally. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > A quick grep shows that drivers/staging/iio/adc/ad7816.c is the only > driver in tree doing it wrong i.e. request_threaded_irq with > IRQF_TRIGGER_{LOW|HIGH} but without ONEHOST. > > There are 59 users in tree which request an edge typed interrupt. 24 of > them specify the ONEHOST flag the others don't. Both variants are valid > and identical but a consistent behavior would be nice. > > There is a total of 134 users and 87 specify the irq type directly. For > the remaining 47 the type depends on initial configuration of the irq chip > or it might be specified via the device tree, dunno. > Should that patch be accepted I suggest to remove the ONESHOT flag from > every request_th.* which uses NULL as the primary handler so we have a > consistent behavior here. > > kernel/irq/manage.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > index 0a7840ae..3f9cd47 100644 > --- a/kernel/irq/manage.c > +++ b/kernel/irq/manage.c > @@ -1322,6 +1322,7 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, > if (!thread_fn) > return -EINVAL; > handler = irq_default_primary_handler; > + irqflags |= IRQF_ONESHOT; > } > > action = kzalloc(sizeof(struct irqaction), GFP_KERNEL); > -- > 1.7.4.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-18 17:01 ` [PATCH] irq: always " Pantelis Antoniou @ 2011-08-18 17:22 ` Sebastian Andrzej Siewior 2011-08-18 21:34 ` Felipe Balbi 2011-08-22 23:26 ` Paul Walmsley 1 sibling, 1 reply; 19+ messages in thread From: Sebastian Andrzej Siewior @ 2011-08-18 17:22 UTC (permalink / raw) To: Pantelis Antoniou; +Cc: Thomas Gleixner, linux-kernel, mporter Pantelis Antoniou wrote: > Hi there, Hi, > Let me report that with this change Beagle board fails to boot, > hangs right on rootfs mount. Can you provide some more information about the kind of the failure and used drivers? > Users of BB should revert this until the offending driver(s) are fixed. Which is the offending driver? > Regards > > -- Pantelis Sebastian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-18 17:22 ` Sebastian Andrzej Siewior @ 2011-08-18 21:34 ` Felipe Balbi 2011-08-19 7:25 ` Sebastian Andrzej Siewior 2011-08-22 23:45 ` Paul Walmsley 0 siblings, 2 replies; 19+ messages in thread From: Felipe Balbi @ 2011-08-18 21:34 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Pantelis Antoniou, Thomas Gleixner, linux-kernel, mporter [-- Attachment #1: Type: text/plain, Size: 674 bytes --] Hi, On Thu, Aug 18, 2011 at 07:22:39PM +0200, Sebastian Andrzej Siewior wrote: > Pantelis Antoniou wrote: > >Hi there, > Hi, > > >Let me report that with this change Beagle board fails to boot, > >hangs right on rootfs mount. > > Can you provide some more information about the kind of the failure and > used drivers? > > >Users of BB should revert this until the offending driver(s) are fixed. > Which is the offending driver? I would guess it's the lack of threaded IRQ conversion on the twl4030 driver. I have converted it but noone actually picked the series [1]. http://marc.info/?i=1309427470-605-1-git-send-email-balbi@ti.com -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-18 21:34 ` Felipe Balbi @ 2011-08-19 7:25 ` Sebastian Andrzej Siewior 2011-08-19 10:13 ` Felipe Balbi 2011-08-22 23:45 ` Paul Walmsley 1 sibling, 1 reply; 19+ messages in thread From: Sebastian Andrzej Siewior @ 2011-08-19 7:25 UTC (permalink / raw) To: balbi; +Cc: Pantelis Antoniou, Thomas Gleixner, linux-kernel, mporter Felipe Balbi wrote: >> Pantelis Antoniou wrote: >>> Let me report that with this change Beagle board fails to boot, >>> hangs right on rootfs mount. >> Can you provide some more information about the kind of the failure and >> used drivers? >> >>> Users of BB should revert this until the offending driver(s) are fixed. >> Which is the offending driver? > > I would guess it's the lack of threaded IRQ conversion on the > twl4030 driver. I have converted it but noone actually picked the series > [1]. This makes no sense. If you don't use threaded handlers that means you specify a primary handler and therefore the patch does not change a thing for you. > http://marc.info/?i=1309427470-605-1-git-send-email-balbi@ti.com Sebastian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-19 7:25 ` Sebastian Andrzej Siewior @ 2011-08-19 10:13 ` Felipe Balbi 0 siblings, 0 replies; 19+ messages in thread From: Felipe Balbi @ 2011-08-19 10:13 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: balbi, Pantelis Antoniou, Thomas Gleixner, linux-kernel, mporter [-- Attachment #1: Type: text/plain, Size: 881 bytes --] HI, On Fri, Aug 19, 2011 at 09:25:35AM +0200, Sebastian Andrzej Siewior wrote: > Felipe Balbi wrote: > >>Pantelis Antoniou wrote: > >>>Let me report that with this change Beagle board fails to boot, > >>>hangs right on rootfs mount. > >>Can you provide some more information about the kind of the failure and > >>used drivers? > >> > >>>Users of BB should revert this until the offending driver(s) are fixed. > >>Which is the offending driver? > > > >I would guess it's the lack of threaded IRQ conversion on the > >twl4030 driver. I have converted it but noone actually picked the series > >[1]. > > This makes no sense. If you don't use threaded handlers that means > you specify a primary handler and therefore the patch does not change a > thing for you. the thing is that twl's children are using threaded IRQs, but no the core itself. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-18 21:34 ` Felipe Balbi 2011-08-19 7:25 ` Sebastian Andrzej Siewior @ 2011-08-22 23:45 ` Paul Walmsley 2011-08-23 5:10 ` [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) Paul Walmsley 2011-08-23 9:09 ` [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified Felipe Balbi 1 sibling, 2 replies; 19+ messages in thread From: Paul Walmsley @ 2011-08-22 23:45 UTC (permalink / raw) To: Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi Cc: Pantelis Antoniou, linux-kernel, linux-omap, torvalds, mporter On Fri, 19 Aug 2011, Felipe Balbi wrote: > On Thu, Aug 18, 2011 at 07:22:39PM +0200, Sebastian Andrzej Siewior wrote: > > Pantelis Antoniou wrote: > > > > >Let me report that with this change Beagle board fails to boot, > > >hangs right on rootfs mount. > > > > Can you provide some more information about the kind of the failure and > > used drivers? > > > > >Users of BB should revert this until the offending driver(s) are fixed. > > Which is the offending driver? > > I would guess it's the lack of threaded IRQ conversion on the > twl4030 driver. I have converted it but noone actually picked the series > [1]. > > http://marc.info/?i=1309427470-605-1-git-send-email-balbi@ti.com Just a data point: applying this series doesn't seem to fix the hang. Sebastian, Thomas, does commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if no primary handler is specified") fix anything, aside from a driver in staging? - Paul ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-22 23:45 ` Paul Walmsley @ 2011-08-23 5:10 ` Paul Walmsley 2011-08-23 8:14 ` Sebastian Andrzej Siewior ` (2 more replies) 2011-08-23 9:09 ` [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified Felipe Balbi 1 sibling, 3 replies; 19+ messages in thread From: Paul Walmsley @ 2011-08-23 5:10 UTC (permalink / raw) To: linux-serial, Alan Cox Cc: Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi, torvalds, mporter, Kevin Hilman, Govindraj.R Convert the omap-serial hardirq handler to a threaded IRQ handler. Without this patch, OMAP boards which use the on-board OMAP UARTs and the omap-serial driver will not boot to userspace after commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if no primary handler is specified"). Enabling CONFIG_DEBUG_SHIRQ reveals 'IRQ handler type mismatch' errors: IRQ handler type mismatch for IRQ 74 current handler: serial idle [<c001b124>] (unwind_backtrace+0x0/0xf0) from [<c009c900>] (__setup_irq+0x42c/0x464) [<c009c900>] (__setup_irq+0x42c/0x464) from [<c009ca08>] (request_threaded_irq+0xd0/0x148) [<c009ca08>] (request_threaded_irq+0xd0/0x148) from [<c029d398>] (serial_omap_startup+0x30/0x2dc) [<c029d398>] (serial_omap_startup+0x30/0x2dc) from [<c0295a38>] (uart_startup+0x5c/0x1ac) (etc.) It turns out that the omap-serial code used one threaded IRQ handler[1][2] and one non-threaded IRQ handler[3] that shared the same IRQ. During the 3.1-rc series, a patch was merged[4] that caused IRQF_ONESHOT to be set on the threaded handler, but the non-threaded handler did not have IRQF_ONESHOT set. Since interrupt handlers that share the same IRQ must also share the presence or absence of IRQF_ONESHOT[5], this new commit caused a mismatch that prevented the non-threaded IRQ from registering. Fix by converting the non-threaded IRQ handler in omap-serial.c to a threaded IRQ handler. Ideally we would not have to make this type of change during the -rc series, but the commit that caused this behavior was itself merged between v3.1-rc2 and v3.1-rc3. In the long term, it would be good to get rid of the shared IRQ handler hack in arch/arm/mach-omap2/serial.c. This change has been boot-tested on OMAP3530 BeagleBoard and OMAP4430ES2 PandaBoard, and static suspend has been lightly tested on the BeagleBoard. Pantelis Antoniou <pantelis.antoniou@gmail.com> originally reported the boot failure. 1. arch/arm/mach-omap2/serial.c line 550, as of Linux v3.1-rc3. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/mach-omap2/serial.c;h=466fc722fa0f39f03b8d93cf84e4dae4f57fd029;hb=fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c#l550 2. arch/arm/mach-omap2/serial.c line 563, as of Linux v3.1-rc3. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/mach-omap2/serial.c;h=466fc722fa0f39f03b8d93cf84e4dae4f57fd029;hb=fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c#l563 3. drivers/tty/serial/omap-serial.c line 464, as of Linux v3.1-rc3. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/tty/serial/omap-serial.c;h=c37df8d0fa2819261dffccc5bc4d0180b9531f49;hb=fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c#l464 4. Commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if no primary handler is specified") 5. Gleixner, Thomas. _[patch 2/5] genirq: Allow shared oneshot interrupts_. http://lkml.org/lkml/2011/2/23/511 Signed-off-by: Paul Walmsley <paul@pwsan.com> Cc: Pantelis Antoniou <pantelis.antoniou@gmail.com> Cc: Govindraj.R <govindraj.raja@ti.com> Cc: Kevin Hilman <khilman@ti.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- drivers/tty/serial/omap-serial.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index c37df8d..e83a769 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -461,8 +461,8 @@ static int serial_omap_startup(struct uart_port *port) /* * Allocate the IRQ */ - retval = request_irq(up->port.irq, serial_omap_irq, up->port.irqflags, - up->name, up); + retval = request_threaded_irq(up->port.irq, NULL, serial_omap_irq, + up->port.irqflags, up->name, up); if (retval) return retval; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-23 5:10 ` [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) Paul Walmsley @ 2011-08-23 8:14 ` Sebastian Andrzej Siewior 2011-08-23 8:57 ` Alan Cox 2011-08-23 9:12 ` Felipe Balbi 2 siblings, 0 replies; 19+ messages in thread From: Sebastian Andrzej Siewior @ 2011-08-23 8:14 UTC (permalink / raw) To: Paul Walmsley Cc: linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Felipe Balbi, torvalds, mporter, Kevin Hilman, Govindraj.R * Paul Walmsley | 2011-08-22 23:10:21 [-0600]: >IRQ handler type mismatch for IRQ 74 >It turns out that the omap-serial code used one threaded IRQ >handler[1][2] and one non-threaded IRQ handler[3] that shared the same >IRQ. During the 3.1-rc series, a patch was merged[4] that caused >IRQF_ONESHOT to be set on the threaded handler, but the non-threaded >handler did not have IRQF_ONESHOT set. Since interrupt handlers Is the type of IRQ74 edge or line/eoi? Since you registered the serial handler as threaded and the primary handler did _not_ disable the irq source (due to the absence of the IRQF_ONESHOT flag or a custom primary handler which does this by changing a bit in the hardware) it is more or less luck that you did not hang forever on the first serial interrupt. Well, it does not happen with edge typed interrupt lines. >that share the same IRQ must also share the presence or absence of >IRQF_ONESHOT[5], this new commit caused a mismatch that prevented the >non-threaded IRQ from registering. In theory it should be okay if the handler without ONESHOT is either primary only or primary + threaded and the primary disables the irq source. Sebastian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-23 5:10 ` [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) Paul Walmsley 2011-08-23 8:14 ` Sebastian Andrzej Siewior @ 2011-08-23 8:57 ` Alan Cox 2011-08-23 16:13 ` Linus Torvalds 2011-08-23 9:12 ` Felipe Balbi 2 siblings, 1 reply; 19+ messages in thread From: Alan Cox @ 2011-08-23 8:57 UTC (permalink / raw) To: Paul Walmsley Cc: linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi, torvalds, mporter, Kevin Hilman, Govindraj.R On Mon, 22 Aug 2011 23:10:21 -0600 (MDT) Paul Walmsley <paul@pwsan.com> wrote: > > Convert the omap-serial hardirq handler to a threaded IRQ handler. Without > this patch, OMAP boards which use the on-board OMAP UARTs and the > omap-serial driver will not boot to userspace after commit > f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if > no primary handler is specified"). Enabling CONFIG_DEBUG_SHIRQ reveals > 'IRQ handler type mismatch' errors: There are multiple other drivers reporting all these problems - the faulty irq change should be reverted at this point not the drivers fiddled with. Alan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-23 8:57 ` Alan Cox @ 2011-08-23 16:13 ` Linus Torvalds 0 siblings, 0 replies; 19+ messages in thread From: Linus Torvalds @ 2011-08-23 16:13 UTC (permalink / raw) To: Alan Cox Cc: Paul Walmsley, linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi, mporter, Kevin Hilman, Govindraj.R On Tue, Aug 23, 2011 at 1:57 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > On Mon, 22 Aug 2011 23:10:21 -0600 (MDT) > Paul Walmsley <paul@pwsan.com> wrote: > >> >> Convert the omap-serial hardirq handler to a threaded IRQ handler. Without >> this patch, OMAP boards which use the on-board OMAP UARTs and the >> omap-serial driver will not boot to userspace after commit >> f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if >> no primary handler is specified"). Enabling CONFIG_DEBUG_SHIRQ reveals >> 'IRQ handler type mismatch' errors: > > There are multiple other drivers reporting all these problems - the > faulty irq change should be reverted at this point not the drivers > fiddled with. Agreed. It's too late to try to fix random drivers. The real issue seems to be that clue: "Enabling CONFIG_DEBUG_SHIRQ reveals 'IRQ handler type mismatch' errors" iow, we have a shared irq, and forcing the IRQF_ONESHOT bit is resulting in those shared interrupts now having different values of IRQF_ONESHOT, so this test triggers: /* * Can't share interrupts unless both agree to and are * the same type (level, edge, polarity). So both flag * fields must have IRQF_SHARED set and the bits which * set the trigger type must match. Also all must * agree on ONESHOT. */ if (!((old->flags & new->flags) & IRQF_SHARED) || ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) || ((old->flags ^ new->flags) & IRQF_ONESHOT)) { old_name = old->name; goto mismatch; } and the irq isn't installed at all (setup_irq returns with EBUSY). So the commit that caused this problem was simply bogus. The commit log says "Since it is required for those users and there is no difference for others it makes sense to add this flag unconditionally." but the "there is no difference for others" seems to be total crap, exactly because it results in this IRQF mismatch. So I think that commit should just be reverted. Thomas? Linus ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) @ 2011-08-23 16:13 ` Linus Torvalds 0 siblings, 0 replies; 19+ messages in thread From: Linus Torvalds @ 2011-08-23 16:13 UTC (permalink / raw) To: Alan Cox Cc: Paul Walmsley, linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi, mporter, Kevin Hilman, Govindraj.R On Tue, Aug 23, 2011 at 1:57 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > On Mon, 22 Aug 2011 23:10:21 -0600 (MDT) > Paul Walmsley <paul@pwsan.com> wrote: > >> >> Convert the omap-serial hardirq handler to a threaded IRQ handler. Without >> this patch, OMAP boards which use the on-board OMAP UARTs and the >> omap-serial driver will not boot to userspace after commit >> f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if >> no primary handler is specified"). Enabling CONFIG_DEBUG_SHIRQ reveals >> 'IRQ handler type mismatch' errors: > > There are multiple other drivers reporting all these problems - the > faulty irq change should be reverted at this point not the drivers > fiddled with. Agreed. It's too late to try to fix random drivers. The real issue seems to be that clue: "Enabling CONFIG_DEBUG_SHIRQ reveals 'IRQ handler type mismatch' errors" iow, we have a shared irq, and forcing the IRQF_ONESHOT bit is resulting in those shared interrupts now having different values of IRQF_ONESHOT, so this test triggers: /* * Can't share interrupts unless both agree to and are * the same type (level, edge, polarity). So both flag * fields must have IRQF_SHARED set and the bits which * set the trigger type must match. Also all must * agree on ONESHOT. */ if (!((old->flags & new->flags) & IRQF_SHARED) || ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) || ((old->flags ^ new->flags) & IRQF_ONESHOT)) { old_name = old->name; goto mismatch; } and the irq isn't installed at all (setup_irq returns with EBUSY). So the commit that caused this problem was simply bogus. The commit log says "Since it is required for those users and there is no difference for others it makes sense to add this flag unconditionally." but the "there is no difference for others" seems to be total crap, exactly because it results in this IRQF mismatch. So I think that commit should just be reverted. Thomas? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-23 16:13 ` Linus Torvalds (?) @ 2011-08-23 16:46 ` Thomas Gleixner -1 siblings, 0 replies; 19+ messages in thread From: Thomas Gleixner @ 2011-08-23 16:46 UTC (permalink / raw) To: Linus Torvalds Cc: Alan Cox, Paul Walmsley, linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Sebastian Andrzej Siewior, Felipe Balbi, mporter, Kevin Hilman, Govindraj.R Linus, On Tue, 23 Aug 2011, Linus Torvalds wrote: > but the "there is no difference for others" seems to be total crap, > exactly because it results in this IRQF mismatch. > > So I think that commit should just be reverted. Thomas? Yes. I missed that detail when I applied that patch. Reverting it seems to be the only sensible solution. Thanks, tglx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-23 5:10 ` [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) Paul Walmsley 2011-08-23 8:14 ` Sebastian Andrzej Siewior 2011-08-23 8:57 ` Alan Cox @ 2011-08-23 9:12 ` Felipe Balbi 2011-08-23 17:21 ` Paul Walmsley 2 siblings, 1 reply; 19+ messages in thread From: Felipe Balbi @ 2011-08-23 9:12 UTC (permalink / raw) To: Paul Walmsley Cc: linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi, torvalds, mporter, Kevin Hilman, Govindraj.R [-- Attachment #1: Type: text/plain, Size: 4261 bytes --] Hi, On Mon, Aug 22, 2011 at 11:10:21PM -0600, Paul Walmsley wrote: > > Convert the omap-serial hardirq handler to a threaded IRQ handler. Without > this patch, OMAP boards which use the on-board OMAP UARTs and the > omap-serial driver will not boot to userspace after commit > f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if > no primary handler is specified"). Enabling CONFIG_DEBUG_SHIRQ reveals > 'IRQ handler type mismatch' errors: > > IRQ handler type mismatch for IRQ 74 > current handler: serial idle > [<c001b124>] (unwind_backtrace+0x0/0xf0) from [<c009c900>] (__setup_irq+0x42c/0x464) > [<c009c900>] (__setup_irq+0x42c/0x464) from [<c009ca08>] (request_threaded_irq+0xd0/0x148) > [<c009ca08>] (request_threaded_irq+0xd0/0x148) from [<c029d398>] (serial_omap_startup+0x30/0x2dc) > [<c029d398>] (serial_omap_startup+0x30/0x2dc) from [<c0295a38>] (uart_startup+0x5c/0x1ac) > > (etc.) > > It turns out that the omap-serial code used one threaded IRQ > handler[1][2] and one non-threaded IRQ handler[3] that shared the same > IRQ. During the 3.1-rc series, a patch was merged[4] that caused > IRQF_ONESHOT to be set on the threaded handler, but the non-threaded > handler did not have IRQF_ONESHOT set. Since interrupt handlers > that share the same IRQ must also share the presence or absence of > IRQF_ONESHOT[5], this new commit caused a mismatch that prevented the > non-threaded IRQ from registering. > > Fix by converting the non-threaded IRQ handler in omap-serial.c to a > threaded IRQ handler. Ideally we would not have to make this type of > change during the -rc series, but the commit that caused this behavior > was itself merged between v3.1-rc2 and v3.1-rc3. In the long term, it > would be good to get rid of the shared IRQ handler hack in > arch/arm/mach-omap2/serial.c. > > This change has been boot-tested on OMAP3530 BeagleBoard and > OMAP4430ES2 PandaBoard, and static suspend has been lightly tested on > the BeagleBoard. > > Pantelis Antoniou <pantelis.antoniou@gmail.com> originally reported > the boot failure. > > 1. arch/arm/mach-omap2/serial.c line 550, as of Linux v3.1-rc3. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/mach-omap2/serial.c;h=466fc722fa0f39f03b8d93cf84e4dae4f57fd029;hb=fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c#l550 > > 2. arch/arm/mach-omap2/serial.c line 563, as of Linux v3.1-rc3. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/mach-omap2/serial.c;h=466fc722fa0f39f03b8d93cf84e4dae4f57fd029;hb=fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c#l563 > > 3. drivers/tty/serial/omap-serial.c line 464, as of Linux v3.1-rc3. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/tty/serial/omap-serial.c;h=c37df8d0fa2819261dffccc5bc4d0180b9531f49;hb=fcb8ce5cfe30ca9ca5c9a79cdfe26d1993e65e0c#l464 > > 4. Commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set > IRQF_ONESHOT if no primary handler is specified") > > 5. Gleixner, Thomas. _[patch 2/5] genirq: Allow shared oneshot > interrupts_. http://lkml.org/lkml/2011/2/23/511 > > Signed-off-by: Paul Walmsley <paul@pwsan.com> > Cc: Pantelis Antoniou <pantelis.antoniou@gmail.com> > Cc: Govindraj.R <govindraj.raja@ti.com> > Cc: Kevin Hilman <khilman@ti.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > drivers/tty/serial/omap-serial.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > index c37df8d..e83a769 100644 > --- a/drivers/tty/serial/omap-serial.c > +++ b/drivers/tty/serial/omap-serial.c > @@ -461,8 +461,8 @@ static int serial_omap_startup(struct uart_port *port) > /* > * Allocate the IRQ > */ > - retval = request_irq(up->port.irq, serial_omap_irq, up->port.irqflags, > - up->name, up); > + retval = request_threaded_irq(up->port.irq, NULL, serial_omap_irq, > + up->port.irqflags, up->name, up); if you're not running on a slow bus, you should use top half to check if IRQs are really from this device. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) 2011-08-23 9:12 ` Felipe Balbi @ 2011-08-23 17:21 ` Paul Walmsley 0 siblings, 0 replies; 19+ messages in thread From: Paul Walmsley @ 2011-08-23 17:21 UTC (permalink / raw) To: Felipe Balbi Cc: linux-serial, Alan Cox, Pantelis Antoniou, linux-kernel, linux-omap, Thomas Gleixner, Sebastian Andrzej Siewior, torvalds, mporter, Kevin Hilman, Govindraj.R On Tue, 23 Aug 2011, Felipe Balbi wrote: > if you're not running on a slow bus, you should use top half to check if > IRQs are really from this device. I don't think this applies in this case, since the IRQ isn't shared between different hardware devices - it's shared for the purposes of a software hack. - Paul ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-22 23:45 ` Paul Walmsley 2011-08-23 5:10 ` [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) Paul Walmsley @ 2011-08-23 9:09 ` Felipe Balbi 1 sibling, 0 replies; 19+ messages in thread From: Felipe Balbi @ 2011-08-23 9:09 UTC (permalink / raw) To: Paul Walmsley Cc: Thomas Gleixner, Sebastian Andrzej Siewior, Felipe Balbi, Pantelis Antoniou, linux-kernel, linux-omap, torvalds, mporter [-- Attachment #1: Type: text/plain, Size: 949 bytes --] Hi, On Mon, Aug 22, 2011 at 05:45:06PM -0600, Paul Walmsley wrote: > On Fri, 19 Aug 2011, Felipe Balbi wrote: > > > On Thu, Aug 18, 2011 at 07:22:39PM +0200, Sebastian Andrzej Siewior wrote: > > > Pantelis Antoniou wrote: > > > > > > >Let me report that with this change Beagle board fails to boot, > > > >hangs right on rootfs mount. > > > > > > Can you provide some more information about the kind of the failure and > > > used drivers? > > > > > > >Users of BB should revert this until the offending driver(s) are fixed. > > > Which is the offending driver? > > > > I would guess it's the lack of threaded IRQ conversion on the > > twl4030 driver. I have converted it but noone actually picked the series > > [1]. > > > > http://marc.info/?i=1309427470-605-1-git-send-email-balbi@ti.com > > Just a data point: applying this series doesn't seem to fix the hang. a wild guess which wasn't right :-p -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified 2011-08-18 17:01 ` [PATCH] irq: always " Pantelis Antoniou @ 2011-08-22 23:26 ` Paul Walmsley 2011-08-22 23:26 ` Paul Walmsley 1 sibling, 0 replies; 19+ messages in thread From: Paul Walmsley @ 2011-08-22 23:26 UTC (permalink / raw) To: Pantelis Antoniou Cc: Sebastian Andrzej Siewior, torvalds, Thomas Gleixner, linux-kernel, linux-omap, tony, linux-arm-kernel, mporter On Thu, 18 Aug 2011, Pantelis Antoniou wrote: > Let me report that with this change Beagle board fails to boot, > hangs right on rootfs mount. Confirmed here. Commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if no primary handler is specified"), merged as part of v3.1-rc3, won't boot into userspace via MMC on either OMAP4 Panda or OMAP3530 BeagleBoard. v3.1-rc2 works. - Paul > Users of BB should revert this until the offending driver(s) are fixed. > > Regards > > -- Pantelis > > On Jul 7, 2011, at 11:32 PM, Sebastian Andrzej Siewior wrote: > > > If no primary handler is specified then a default one is assigned which > > always returns IRQ_WAKE_THREAD. This handler requires the IRQF_ONESHOT > > flag on LEVEL / EIO typed irqs because the source of interrupt is not > > disabled. > > Since it is required for those users and there is no difference for others > > it makes sense to add this flag unconditionally. > > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > --- > > A quick grep shows that drivers/staging/iio/adc/ad7816.c is the only > > driver in tree doing it wrong i.e. request_threaded_irq with > > IRQF_TRIGGER_{LOW|HIGH} but without ONEHOST. > > > > There are 59 users in tree which request an edge typed interrupt. 24 of > > them specify the ONEHOST flag the others don't. Both variants are valid > > and identical but a consistent behavior would be nice. > > > > There is a total of 134 users and 87 specify the irq type directly. For > > the remaining 47 the type depends on initial configuration of the irq chip > > or it might be specified via the device tree, dunno. > > Should that patch be accepted I suggest to remove the ONESHOT flag from > > every request_th.* which uses NULL as the primary handler so we have a > > consistent behavior here. > > > > kernel/irq/manage.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > > index 0a7840ae..3f9cd47 100644 > > --- a/kernel/irq/manage.c > > +++ b/kernel/irq/manage.c > > @@ -1322,6 +1322,7 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, > > if (!thread_fn) > > return -EINVAL; > > handler = irq_default_primary_handler; > > + irqflags |= IRQF_ONESHOT; > > } > > > > action = kzalloc(sizeof(struct irqaction), GFP_KERNEL); > > -- > > 1.7.4.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified @ 2011-08-22 23:26 ` Paul Walmsley 0 siblings, 0 replies; 19+ messages in thread From: Paul Walmsley @ 2011-08-22 23:26 UTC (permalink / raw) To: linux-arm-kernel On Thu, 18 Aug 2011, Pantelis Antoniou wrote: > Let me report that with this change Beagle board fails to boot, > hangs right on rootfs mount. Confirmed here. Commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 ("irq: Always set IRQF_ONESHOT if no primary handler is specified"), merged as part of v3.1-rc3, won't boot into userspace via MMC on either OMAP4 Panda or OMAP3530 BeagleBoard. v3.1-rc2 works. - Paul > Users of BB should revert this until the offending driver(s) are fixed. > > Regards > > -- Pantelis > > On Jul 7, 2011, at 11:32 PM, Sebastian Andrzej Siewior wrote: > > > If no primary handler is specified then a default one is assigned which > > always returns IRQ_WAKE_THREAD. This handler requires the IRQF_ONESHOT > > flag on LEVEL / EIO typed irqs because the source of interrupt is not > > disabled. > > Since it is required for those users and there is no difference for others > > it makes sense to add this flag unconditionally. > > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > --- > > A quick grep shows that drivers/staging/iio/adc/ad7816.c is the only > > driver in tree doing it wrong i.e. request_threaded_irq with > > IRQF_TRIGGER_{LOW|HIGH} but without ONEHOST. > > > > There are 59 users in tree which request an edge typed interrupt. 24 of > > them specify the ONEHOST flag the others don't. Both variants are valid > > and identical but a consistent behavior would be nice. > > > > There is a total of 134 users and 87 specify the irq type directly. For > > the remaining 47 the type depends on initial configuration of the irq chip > > or it might be specified via the device tree, dunno. > > Should that patch be accepted I suggest to remove the ONESHOT flag from > > every request_th.* which uses NULL as the primary handler so we have a > > consistent behavior here. > > > > kernel/irq/manage.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c > > index 0a7840ae..3f9cd47 100644 > > --- a/kernel/irq/manage.c > > +++ b/kernel/irq/manage.c > > @@ -1322,6 +1322,7 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler, > > if (!thread_fn) > > return -EINVAL; > > handler = irq_default_primary_handler; > > + irqflags |= IRQF_ONESHOT; > > } > > > > action = kzalloc(sizeof(struct irqaction), GFP_KERNEL); > > -- > > 1.7.4.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo at vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-08-23 17:21 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-07 20:32 [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified Sebastian Andrzej Siewior 2011-07-28 9:39 ` [tip:irq/urgent] irq: Always " tip-bot for Sebastian Andrzej Siewior 2011-08-18 17:01 ` [PATCH] irq: always " Pantelis Antoniou 2011-08-18 17:22 ` Sebastian Andrzej Siewior 2011-08-18 21:34 ` Felipe Balbi 2011-08-19 7:25 ` Sebastian Andrzej Siewior 2011-08-19 10:13 ` Felipe Balbi 2011-08-22 23:45 ` Paul Walmsley 2011-08-23 5:10 ` [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified) Paul Walmsley 2011-08-23 8:14 ` Sebastian Andrzej Siewior 2011-08-23 8:57 ` Alan Cox 2011-08-23 16:13 ` Linus Torvalds 2011-08-23 16:13 ` Linus Torvalds 2011-08-23 16:46 ` Thomas Gleixner 2011-08-23 9:12 ` Felipe Balbi 2011-08-23 17:21 ` Paul Walmsley 2011-08-23 9:09 ` [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified Felipe Balbi 2011-08-22 23:26 ` Paul Walmsley 2011-08-22 23:26 ` Paul Walmsley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.