Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA

From: Mathias Nyman
Date: Wed Apr 23 2014 - 07:24:21 EST



Urgent fix and the maintainers did not react in a week? Well maybe they need
to be on the To: line...

Mathias: can you send a patch adding yourself as maintainer of this
driver in the MAINTAINERS file so stuff like this does not fall to the
floor (me)?


Hi,

Sorry about the delay. I'm taking over Sarah's role as usb3 xhci maintainer and got my hands full over there. I can look at these
patches now and then but you might need to kick me.

In general, I'd agree with Mika, Andy and Heikki (and Rafael obviously) opinion regarding baytrail gpio / acpi related matters.

Second: this fix is ugly like hell, is it really the best we can think
of, plus in the commit message I'd very much like to know the
real issue behind this as people in the x86 camp seem to be
using some strange static IRQ line assignments that I cannot
really understand so I don't know what the proper fix for this is :-(


This patch went out a bit early, a new version (which is not mangled) can be found at:

http://marc.info/?l=linux-kernel&m=139765010717522&w=2

But that one still produces some compiler warning which should be fixed.

There might be some touchscreen related issues recently found related to this patch that need to be investigated ( see bug link in commit message)

This is a hack to get T100TA working. This is one of many bad ways to workaround the real issue instead of fixing it, and I hope it can be reverted later, but this is the best we can do with our (my) limited io-apic and interrupt knowledge. But in the end, with this the Asus T100TA gpio works somehow, without it, it doesn't.

The real issue to my understanding is that the x86 io-apic code is not really capable of living together with other interrupt controllers at the same time. There are some assumptions that interrupts 0-15 belong to the legacy ISA world, from there on we got io-apic PCI interrupts
( I think io-apic interrupt controller handles something like 24 - 48 interrupts?) we want to be outside that range until this is fixed.

The x86 io-apic code does nasty things, for example the function
that allocates the irq and the private chip data assumes that if the irq descriptor is taken, (returns -EEXISTS) then io-apic just must have reserved it earlier and can anyway use it, and access the chip data in a format only io-apic (struct irq_cfg) uses. This is of course not the case if a gpio interrupt controller like pinctrl-baytrail reserved the interrupt descriptor earler. -> oops

here's the io_apic.c function:

static struct irq_cfg *alloc_irq_and_cfg_at(unsigned int at, int node)
{
int res = irq_alloc_desc_at(at, node);
struct irq_cfg *cfg;

if (res < 0) {
if (res != -EEXIST)
return NULL;
cfg = irq_get_chip_data(at);
if (cfg)
return cfg;
}

cfg = alloc_irq_cfg(at, node);
if (cfg)
irq_set_chip_data(at, cfg);
else
irq_free_desc(at);
return cfg;
}

We tried to fix this particular issue (make sure the interrupt belongs to a io_apic controller before letting io_apic touch it), but we just opened a can of worms of other issues I don't know how to deal with.

-Mathias

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/