Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Thu, Apr 24, 2014 at 04:40:25PM +0200, Thomas Gleixner wrote: > On Thu, 24 Apr 2014, Linus Walleij wrote: > > On Thu, Apr 24, 2014 at 9:25 AM, Thomas Gleixner wrote: > > > > > I'm traveling until friday, so please wait before you commit that > > > fugly hack. I'll have a closer look how we can handle that at the core > > > level. > > > > Thanks a *lot* Thomas, I'll stand by for action. > > Find an untested patch below. It should cure the issue. > > I went through all code which can be affected by this and except for > some other places, which might erroneously allocate inside the > hardwired space, I can't see any possible fallout. > > Thanks, > > tglx > > > > > Subject: genirq: x86: Ensure that dynamic irq allocation does not conflict > From: Thomas Gleixner > Date: Thu, 24 Apr 2014 09:50:53 +0200 > > On x86 the allocation of irq descriptors may allocate interrupts which > are in the range of the GSI interrupts. That's wrong as those > interrupts are hardwired and we don't have the irq domain translation > like PPC. So one of these interrupts can be hooked up later to one of > the devices which are hard wired to it and the io_apic init code for > that particular interrupt line happily reuses that descriptor with a > completely different configuration so hell breaks lose. > > Inside x86 we allocate dynamic interrupts from above nr_gsi_irqs, > except for a few usage sites which have not yet blown up in our face > for whatever reason. But for drivers which need an irq range, like the > GPIO drivers, we have no limit in place and we don't want to expose > such a detail to a driver. > > To cure this introduce a function which an architecture can implement > to impose a lower bound on the dynamic interrupt allocations. > > Implement it for x86 and set the lower bound to nr_gsi_irqs, which is > the end of the hardwired interrupt space, so all dynamic allocations > happen above. > > That not only allows the GPIO driver to work sanely, it also protects > the bogus callsites of create_irq_nr() in hpet, uv, irq_remapping and > htirq code. They need to be cleaned up as well, but that's a separate > issue. > > Signed-off-by: Thomas Gleixner Works here on my T100, Tested-by: Mika Westerberg -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Thu, 24 Apr 2014, Linus Walleij wrote: > On Thu, Apr 24, 2014 at 9:25 AM, Thomas Gleixner wrote: > > > I'm traveling until friday, so please wait before you commit that > > fugly hack. I'll have a closer look how we can handle that at the core > > level. > > Thanks a *lot* Thomas, I'll stand by for action. Find an untested patch below. It should cure the issue. I went through all code which can be affected by this and except for some other places, which might erroneously allocate inside the hardwired space, I can't see any possible fallout. Thanks, tglx > Subject: genirq: x86: Ensure that dynamic irq allocation does not conflict From: Thomas Gleixner Date: Thu, 24 Apr 2014 09:50:53 +0200 On x86 the allocation of irq descriptors may allocate interrupts which are in the range of the GSI interrupts. That's wrong as those interrupts are hardwired and we don't have the irq domain translation like PPC. So one of these interrupts can be hooked up later to one of the devices which are hard wired to it and the io_apic init code for that particular interrupt line happily reuses that descriptor with a completely different configuration so hell breaks lose. Inside x86 we allocate dynamic interrupts from above nr_gsi_irqs, except for a few usage sites which have not yet blown up in our face for whatever reason. But for drivers which need an irq range, like the GPIO drivers, we have no limit in place and we don't want to expose such a detail to a driver. To cure this introduce a function which an architecture can implement to impose a lower bound on the dynamic interrupt allocations. Implement it for x86 and set the lower bound to nr_gsi_irqs, which is the end of the hardwired interrupt space, so all dynamic allocations happen above. That not only allows the GPIO driver to work sanely, it also protects the bogus callsites of create_irq_nr() in hpet, uv, irq_remapping and htirq code. They need to be cleaned up as well, but that's a separate issue. Signed-off-by: Thomas Gleixner --- arch/x86/kernel/apic/io_apic.c |5 + include/linux/irq.h|2 ++ kernel/irq/irqdesc.c |2 ++ kernel/softirq.c |5 + 4 files changed, 14 insertions(+) Index: linux-2.6/arch/x86/kernel/apic/io_apic.c === --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c +++ linux-2.6/arch/x86/kernel/apic/io_apic.c @@ -3425,6 +3425,11 @@ int get_nr_irqs_gsi(void) return nr_irqs_gsi; } +unsigned int arch_dynirq_lower_bound(unsigned int from) +{ + return from < nr_irqs_gsi ? nr_irqs_gsi : from; +} + int __init arch_probe_nr_irqs(void) { int nr; Index: linux-2.6/include/linux/irq.h === --- linux-2.6.orig/include/linux/irq.h +++ linux-2.6/include/linux/irq.h @@ -602,6 +602,8 @@ static inline u32 irq_get_trigger_type(u return d ? irqd_get_trigger_type(d) : 0; } +unsigned int arch_dynirq_lower_bound(unsigned int from); + int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, struct module *owner); Index: linux-2.6/kernel/irq/irqdesc.c === --- linux-2.6.orig/kernel/irq/irqdesc.c +++ linux-2.6/kernel/irq/irqdesc.c @@ -363,6 +363,8 @@ __irq_alloc_descs(int irq, unsigned int if (from > irq) return -EINVAL; from = irq; + } else { + from = arch_dynirq_lower_bound(from); } mutex_lock(&sparse_irq_lock); Index: linux-2.6/kernel/softirq.c === --- linux-2.6.orig/kernel/softirq.c +++ linux-2.6/kernel/softirq.c @@ -779,3 +779,8 @@ int __init __weak arch_early_irq_init(vo { return 0; } + +unsigned int __weak arch_dynirq_lower_bound(unsigned int from) +{ + return from; +} -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Thu, Apr 24, 2014 at 9:25 AM, Thomas Gleixner wrote: > I'm traveling until friday, so please wait before you commit that > fugly hack. I'll have a closer look how we can handle that at the core > level. Thanks a *lot* Thomas, I'll stand by for action. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Wed, 23 Apr 2014, Linus Walleij wrote: > However this is a first time for an embedded irqchip (coupled > with GPIO ACPI) creeping into the x86 world, so it needs some > attention I think, do we have a direction forward for peaceful > coexistence of several irq controllers picking some IRQ > numbers/descriptors dynamically as they probe, also on > x86 systems? The issue on x86 is, that we have basically two classes of interrupts. - The hardwired ones (in the chipset and we have no way to change that except with MSI) - The dynamic ones, i.e. MSI[X] So for allocating the MSI interrupts we use the range above the possible hardwired interrupts. Unfortunaly the information where the reserved/hardwired range ends is not available to drivers, but it should be simple to do so. I'm traveling until friday, so please wait before you commit that fugly hack. I'll have a closer look how we can handle that at the core level. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Wed, 2014-04-23 at 14:35 +0300, Mathias Nyman wrote: > > > > 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. This patch and discussion reminds me an old one where I was trying to solve quite similar issue on Medfield [1], in particular [2, and 3]. For my opinion irq mapping framework should be redesigned to solve the issue regarding to devices which require fixed interrupt lines. [1] https://lkml.org/lkml/2012/7/5/152 [2] https://lkml.org/lkml/2012/7/5/155 [3] https://lkml.org/lkml/2012/7/16/173 > > > 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 > -- Andy Shevchenko Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Wed, Apr 23, 2014 at 1:35 PM, Mathias Nyman wrote: > 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. This looks pretty scary. io_apic.c is maintained by every single kernel bigshot and I sure as hell would like to not under any circumstance step on its toes so whatever solution we can think of until the next merge window will be applied. (AFICT this affects all baytrails, no desktops etc). However this is a first time for an embedded irqchip (coupled with GPIO ACPI) creeping into the x86 world, so it needs some attention I think, do we have a direction forward for peaceful coexistence of several irq controllers picking some IRQ numbers/descriptors dynamically as they probe, also on x86 systems? Other archs doesn't seem to have this problem, but they also do not have the same legacy. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
yt_gpio_irq_ops, vg); >> if (!vg->domain) >> return -ENXIO; >> @@ -550,6 +564,12 @@ static int byt_gpio_probe(struct platform_device *pdev) >> irq_set_chained_handler(hwirq, byt_gpio_irq_handler); >> } >> >> + ret = gpiochip_add(gc); >> + if (ret) { >> + dev_err(&pdev->dev, "failed adding byt-gpio chip\n"); >> + return ret; >> + } >> + >> pm_runtime_enable(dev); >> >> return 0; >> @@ -572,6 +592,7 @@ static const struct dev_pm_ops byt_gpio_pm_ops = { >> >> static const struct acpi_device_id byt_gpio_acpi_match[] = { >> { "INT33B2", 0 }, >> + { "INT33FC", 0 }, >> { } >> }; >> MODULE_DEVICE_TABLE(acpi, byt_gpio_acpi_match); > > Urgent fix and the maintainers did not react in a week? Well maybe they need > to be on the To: line... > Thanks for reminding. I will keep in mind for next time. > 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)? > > 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 :-( GPIO driver is loaded before graphics. It's no problem for it to get the first free irq, which number is 16. After a while, graphics driver is loaded. The IRQ number (16) it needs is hardwired by BIOS. Here is part of DSDT table: Name (AR00, Package (0x11) { Package (0x04) { 0x0002, Zero, Zero, 0x10/* Jin Yao: IRQ number 16 */ }, .. } The problem happens at __irq_alloc_descs(). __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, struct module *owner) { .. start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS, from, cnt, 0); ret = -EEXIST; /* Jin Yao: irq = 16, start = 17 */ /* A */ if (irq >=0 && start != irq) goto err; ...... err: } When graphics driver probing, the irq is 16 and bitmap_find_next_zero_area() returns 17. That's correct, because 16 has been allocated to GPIO pin. The problem is at the line A, the checking is failed and goto "err:" directly. The next io_apic processing code (not list here) assumes all irq descriptors belongs to a io_apic chip, and that all chip_data is of type irq_cfg. But unfortunately in this case, the chip_data in irq_desc of IRQ16 is pointing to byt_gpio, then crash happens. So this needs to be fixed in two places: 1. make sure io_apic code does not access data in interrupt descriptors that belong to other "interrupt controllers", eg gpio than io_apic. 2. fix graphics driver / bios to not request the not needed irq 16 Probably the above fixes need long time, so I decide to use a simple and direct way by just shifting the irq for GPIO pins to avoid the conflict. This patch "[PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA" has format issue. I post it by forwarding via Thunderbird, but lead to format issue, I'm so sorry for that. I post another patch "[PATCH] pinctrl-baytrail: fix for irq descriptor conflict on ASUS T100TA" to solve the format issue. Thanks Jin Yao > > Yours, > Linus Walleij > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
On Mon, Apr 14, 2014 at 4:48 AM, Jin, Yao wrote: > A crash is triggered on the ASUS T100TA Baytrail-T because of a IRQ > descriptor conflict. There are two gpio triggered acpi events in this > device, GPIO 6 and 18. These gpios are translated to irqs by calling > gpio_to_irq which in turn will call irq_create_mapping(vg->domain, offset). > irq_create_mapping will take care of allocating the irq descriptor, taking > the first available number starting from the given value (6 in our case). > The 0-15 are already reserved by legacy ISA code, so it gets the first > free irq descriptor which is number 16. The i915 driver also uses irq 16, > it loads later than gpio and crashes in probe. > > The bug is reported here: > https://bugzilla.kernel.org/show_bug.cgi?id=68291 > > The rootcause we know now is a low level irq issue. It needs a long term > solution to fix the issue in irq system. > > This patch is a workaround which changes the Baytrail GPIO driver to avoid > the IRQ conflict. It still uses the irq domain to allocate irq descriptor > but start from a predefined irq base number (256). > > Signed-off-by: Jin Yao > --- > drivers/pinctrl/pinctrl-baytrail.c | 37 > + > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-baytrail.c > b/drivers/pinctrl/pinctrl-baytrail.c > index 6e8301f..45b2d81 100644 > --- a/drivers/pinctrl/pinctrl-baytrail.c > +++ b/drivers/pinctrl/pinctrl-baytrail.c > @@ -124,6 +124,18 @@ static struct pinctrl_gpio_range byt_ranges[] = { > }, > }; > > +/* > + * Start from an irq base number above x86 ioapic range to work around some > + * nasty, which is still in 3.14 unresolved irq descriptor conflicts. > + */ > +#define BYT_GPIO_PIN_IRQBASE 256 > + > +static int byt_pin_irqbase[] = { > + BYT_GPIO_PIN_IRQBASE, > + BYT_GPIO_PIN_IRQBASE + BYT_NGPIO_SCORE, > + BYT_GPIO_PIN_IRQBASE + BYT_NGPIO_SCORE + BYT_NGPIO_NCORE, > +}; > + > struct byt_gpio { > struct gpio_chipchip; > struct irq_domain *domain; > @@ -131,6 +143,7 @@ struct byt_gpio { > spinlock_t lock; > void __iomem*reg_base; > struct pinctrl_gpio_range *range; > + int pin_irqbase; > }; > > #define to_byt_gpio(c) container_of(c, struct byt_gpio, chip) > @@ -481,7 +494,7 @@ static int byt_gpio_probe(struct platform_device *pdev) > struct pinctrl_gpio_range *range; > acpi_handle handle = ACPI_HANDLE(dev); > unsigned hwirq; > - int ret; > + int ret, i; > > if (acpi_bus_get_device(handle, &acpi_dev)) > return -ENODEV; > @@ -496,6 +509,12 @@ static int byt_gpio_probe(struct platform_device *pdev) > if (!strcmp(acpi_dev->pnp.unique_id, range->name)) { > vg->chip.ngpio = range->npins; > vg->range = range; > + ret = kstrtol(range->name, 10, &i); > + if (ret != 0) > + return ret; > + > + i--; > + vg->pin_irqbase = byt_pin_irqbase[i]; > break; > } > } > @@ -527,19 +546,14 @@ static int byt_gpio_probe(struct platform_device > *pdev) > gc->can_sleep = false; > gc->dev = dev; > > - ret = gpiochip_add(gc); > - if (ret) { > - dev_err(&pdev->dev, "failed adding byt-gpio chip\n"); > - return ret; > - } > - > /* set up interrupts */ > irq_rc = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > if (irq_rc && irq_rc->start) { > hwirq = irq_rc->start; > gc->to_irq = byt_gpio_to_irq; > > - vg->domain = irq_domain_add_linear(NULL, gc->ngpio, > + vg->domain = irq_domain_add_simple(NULL, gc->ngpio, > + vg->pin_irqbase, >&byt_gpio_irq_ops, vg); > if (!vg->domain) > return -ENXIO; > @@ -550,6 +564,12 @@ static int byt_gpio_probe(struct platform_device *pdev) > irq_set_chained_handler(hwirq, byt_gpio_irq_handler); > } > > + ret = gpiochip_add(gc); > + if (ret) { > + dev_err(&pdev->dev, "failed adding byt-gpio chip\n"); > + return ret; > + } > + > pm_runtime_enable(dev); > > return 0; > @@ -572,6 +592,7 @@ static const struct dev_pm_ops byt_gpio_pm_ops = { > > static const struct acpi_device_id byt_gpio_acpi_match[] = { > { "INT33B2", 0 }, > + { "INT33FC", 0 }, > { } > }; > MODULE_DEVICE_TABLE(acpi, byt_gpio_acpi_match); Urgent fix and the maintainers did not react in a week? Well maybe they need to be on the To: line... Mathias: can y
[PATCH] pinctrl-baytrail: workaround for irq descriptor conflict on ASUS T100TA
A crash is triggered on the ASUS T100TA Baytrail-T because of a IRQ descriptor conflict. There are two gpio triggered acpi events in this device, GPIO 6 and 18. These gpios are translated to irqs by calling gpio_to_irq which in turn will call irq_create_mapping(vg->domain, offset). irq_create_mapping will take care of allocating the irq descriptor, taking the first available number starting from the given value (6 in our case). The 0-15 are already reserved by legacy ISA code, so it gets the first free irq descriptor which is number 16. The i915 driver also uses irq 16, it loads later than gpio and crashes in probe. The bug is reported here: https://bugzilla.kernel.org/show_bug.cgi?id=68291 The rootcause we know now is a low level irq issue. It needs a long term solution to fix the issue in irq system. This patch is a workaround which changes the Baytrail GPIO driver to avoid the IRQ conflict. It still uses the irq domain to allocate irq descriptor but start from a predefined irq base number (256). Signed-off-by: Jin Yao --- drivers/pinctrl/pinctrl-baytrail.c | 37 + 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/drivers/pinctrl/pinctrl-baytrail.c b/drivers/pinctrl/pinctrl-baytrail.c index 6e8301f..45b2d81 100644 --- a/drivers/pinctrl/pinctrl-baytrail.c +++ b/drivers/pinctrl/pinctrl-baytrail.c @@ -124,6 +124,18 @@ static struct pinctrl_gpio_range byt_ranges[] = { }, }; +/* + * Start from an irq base number above x86 ioapic range to work around some + * nasty, which is still in 3.14 unresolved irq descriptor conflicts. + */ +#define BYT_GPIO_PIN_IRQBASE 256 + +static int byt_pin_irqbase[] = { + BYT_GPIO_PIN_IRQBASE, + BYT_GPIO_PIN_IRQBASE + BYT_NGPIO_SCORE, + BYT_GPIO_PIN_IRQBASE + BYT_NGPIO_SCORE + BYT_NGPIO_NCORE, +}; + struct byt_gpio { struct gpio_chipchip; struct irq_domain *domain; @@ -131,6 +143,7 @@ struct byt_gpio { spinlock_t lock; void __iomem*reg_base; struct pinctrl_gpio_range *range; + int pin_irqbase; }; #define to_byt_gpio(c) container_of(c, struct byt_gpio, chip) @@ -481,7 +494,7 @@ static int byt_gpio_probe(struct platform_device *pdev) struct pinctrl_gpio_range *range; acpi_handle handle = ACPI_HANDLE(dev); unsigned hwirq; - int ret; + int ret, i; if (acpi_bus_get_device(handle, &acpi_dev)) return -ENODEV; @@ -496,6 +509,12 @@ static int byt_gpio_probe(struct platform_device *pdev) if (!strcmp(acpi_dev->pnp.unique_id, range->name)) { vg->chip.ngpio = range->npins; vg->range = range; + ret = kstrtol(range->name, 10, &i); + if (ret != 0) + return ret; + + i--; + vg->pin_irqbase = byt_pin_irqbase[i]; break; } } @@ -527,19 +546,14 @@ static int byt_gpio_probe(struct platform_device *pdev) gc->can_sleep = false; gc->dev = dev; - ret = gpiochip_add(gc); - if (ret) { - dev_err(&pdev->dev, "failed adding byt-gpio chip\n"); - return ret; - } - /* set up interrupts */ irq_rc = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (irq_rc && irq_rc->start) { hwirq = irq_rc->start; gc->to_irq = byt_gpio_to_irq; - vg->domain = irq_domain_add_linear(NULL, gc->ngpio, + vg->domain = irq_domain_add_simple(NULL, gc->ngpio, + vg->pin_irqbase, &byt_gpio_irq_ops, vg); if (!vg->domain) return -ENXIO; @@ -550,6 +564,12 @@ static int byt_gpio_probe(struct platform_device *pdev) irq_set_chained_handler(hwirq, byt_gpio_irq_handler); } + ret = gpiochip_add(gc); + if (ret) { + dev_err(&pdev->dev, "failed adding byt-gpio chip\n"); + return ret; + } + pm_runtime_enable(dev); return 0; @@ -572,6 +592,7 @@ static const struct dev_pm_ops byt_gpio_pm_ops = { static const struct acpi_device_id byt_gpio_acpi_match[] = { { "INT33B2", 0 }, + { "INT33FC", 0 }, { } }; MODULE_DEVICE_TABLE(acpi, byt_gpio_acpi_match); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/