Thread (2 messages) 2 messages, 2 authors, 2012-01-06

[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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help