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)

[Linus Torvalds <torvalds@linux-foundation.org>]

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