[PATCH] pnpacpi: Call acpi_register_gsi for possible resources too
STALE5384d
From: Petr Vandrovec <hidden>
Date: 2012-01-03 08:07:16
Subsystem:
acpi, pnp support, the rest · Maintainers:
"Rafael J. Wysocki", Linus Torvalds
Hello, while extending our platform to allow for non-ISA interrupts on serial ports I've found that after boot-up device is properly enabled at I/O base 0x500, and assigned IRQ 16, but IRQ 16 is not listed in /proc/interrupts, and attempts to open serial port (/dev/ttyS4) fail with EIO error. After system is soft-rebooted, kernel finds that device is already enabled (because we do not reprogram device configuration on soft reboot), reuses base 0x500 and IRQ 16, and /dev/ttyS4 works. Inspection of pnpacpi revealed that nobody will call acpi_register_gsi if device is disabled when discovered by the kernel, but kernel will then happilly try to assign IRQ 16-23 to the device - and if interrupt happens to be not used by any other (PCI) device, nobody will call acpi_register_gsi, and register_irq(16) from serial port driver will fail :-( Diff below fixes issue - with patch serial ports work even immediately after hard reboot. Resources for the port as reported by the kernel are in the attachment - allocated resources were decided by the kernel, possible are reported by the ACPI's _PRS. I have no system which would use non 1:1 mapping for GSI<->IRQ (except 0/2 which is already blacklisted in pnpacpi), so I have no idea whether adding it to non-extended IRQ descriptor handling breaks something somewhere or not. But given that translation happens in _CRS it seems more correct to me to do acpi_register_gsi always. There are two more problems I've discovered during testing: 1. I had to restrict I/O base to 0x500-0xBF8 (from original 0x100-0xFFFF), as otherwise kernel happilly enables one of serial ports at 0x170, although IDE already occupied that port - it seems that resources occupied by IDE are reported only after libata loads, and as libata loads after serial port driver, pnpacpi does not seem to notice that 0x170 is in use, rather than available for allocation, enables device there, and then system gets very confused. 2. pnpacpi lacks support for both hot-add and hot-remove. So serial ports cannot be neither added nor removed while system runs. I guess that's the next task. Thanks, Petr Vandrovec From: Petr Vandrovec <redacted> Convert interrupt numbers from GSI to IRQ for _PRS pnpacpi calls acpi_register_gsi only when interrupt resource is read via _CRS method - either when device is discovered (if it was already enabled when pnpacpi initializes), or when someone calls 'get' method on the device. What is missing from this list is invoking acpi_register_gsi when someone issues _SRS method to enable device. And if nobody issues acpi_register_gsi, then device may not work if it is only device using GSI selected by the kernel. As whole kernel thinks in terms of IRQs, rather than GSIs, it seems that correct place where to call acpi_register_gsi is when possible resources are retrieved from _PRS, rather than just before _SRS invocation, so that is what I did. I've enabled checking to make sure we are not selecting mismatching level/edge configuration (with dev_warn, same way _CRS warns). But it may be too noisy, as on almost all systems I could check we get warnings that IRQ 9 is used by ACPI as level/low, while resource descriptors list it as edge/high (together with other IRQs in 3-15 range). Signed-off-by: Petr Vandrovec <redacted>
diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index 5be4a39..3243bf4 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c@@ -518,6 +518,47 @@ static __init void pnpacpi_parse_dma_option(struct pnp_dev *dev, pnp_register_dma_resource(dev, option_flags, map, flags); } +static __init void pnpacpi_gsi_option_to_irq(struct pnp_dev *dev, + pnp_irq_mask_t *map, + u32 gsi, + int triggering, + int polarity) +{ + int p, t; + int irq; + + /* GSI 0 is ignored. */ + if (!gsi) + return; + + /* + * In IO-APIC mode, accept interrupt only if its trigger/polarity + * matches with one already selected. + */ + if (!acpi_get_override_irq(gsi, &t, &p)) { + t = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE; + p = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; + if (triggering != t || polarity != p) { + dev_warn(&dev->dev, "IRQ %d status %s, %s does not match override %s, %s\n", + gsi, triggering ? "edge" : "level", + polarity ? "low" : "high", + t ? "edge":"level", p ? "low":"high"); + return; + } + } + irq = acpi_register_gsi(&dev->dev, gsi, triggering, polarity); + if (irq < 0) + dev_err(&dev->dev, "ignoring IRQ %d option " + "(cannot be mapped to platform IRQ)\n", + gsi); + else if (irq < PNP_IRQ_NR) + __set_bit(irq, map->bits); + else + dev_err(&dev->dev, "ignoring IRQ %d (GSI %d) option " + "(too large for %d entry bitmap)\n", + irq, gsi, PNP_IRQ_NR); +} + static __init void pnpacpi_parse_irq_option(struct pnp_dev *dev, unsigned int option_flags, struct acpi_resource_irq *p)
@@ -528,8 +569,8 @@ static __init void pnpacpi_parse_irq_option(struct pnp_dev *dev, bitmap_zero(map.bits, PNP_IRQ_NR); for (i = 0; i < p->interrupt_count; i++) - if (p->interrupts[i]) - __set_bit(p->interrupts[i], map.bits); + pnpacpi_gsi_option_to_irq(dev, &map, p->interrupts[i], + p->triggering, p->polarity); flags = irq_flags(p->triggering, p->polarity, p->sharable); pnp_register_irq_resource(dev, option_flags, &map, flags);
@@ -544,16 +585,9 @@ static __init void pnpacpi_parse_ext_irq_option(struct pnp_dev *dev, unsigned char flags; bitmap_zero(map.bits, PNP_IRQ_NR); - for (i = 0; i < p->interrupt_count; i++) { - if (p->interrupts[i]) { - if (p->interrupts[i] < PNP_IRQ_NR) - __set_bit(p->interrupts[i], map.bits); - else - dev_err(&dev->dev, "ignoring IRQ %d option " - "(too large for %d entry bitmap)\n", - p->interrupts[i], PNP_IRQ_NR); - } - } + for (i = 0; i < p->interrupt_count; i++) + pnpacpi_gsi_option_to_irq(dev, &map, p->interrupts[i], + p->triggering, p->polarity); flags = irq_flags(p->triggering, p->polarity, p->sharable); pnp_register_irq_resource(dev, option_flags, &map, flags);
Attachments
- comport.txt [text/plain] 1009 bytes · preview
- dmesg.txt [text/plain] 9614 bytes · preview
- acpiregistergsi.txt [text/plain] 2831 bytes · preview