Re: [PATCH v2] pci: fix unavailable irq number 255 reported by BIOS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 01/28/2016 06:32 AM, Bjorn Helgaas wrote:
On Wed, Jan 27, 2016 at 10:13:36AM +0100, Thomas Gleixner wrote:
On Tue, 26 Jan 2016, Bjorn Helgaas wrote:
On Tue, Jan 26, 2016 at 04:48:25PM +0100, Thomas Gleixner wrote:
Right. So we could certainly do something like this INVALID_IRQ thingy, but
that looks a bit weird. What would request_irq() return?

If it returns success, then drivers might make the wrong decision. If it
returns an error code, then the i801 one works, but we might have to fix
others anyway.
I was thinking request_irq() could return -EINVAL if the caller passed
INVALID_IRQ.  That should tell drivers that this interrupt won't work.

We'd be making request_irq() return -EINVAL in some cases where it
currently returns success.  But even though it returns success today,
I don't think the driver is getting interrupts, because the wire isn't
connected.
Correct. What I meant is that the i801 driver can handle the -EINVAL return
from request_irq() today, but other drivers don't. I agree that we need to fix
them anyway and a failure to request the interrupt is better than a silent 'no
interrupts delivered' issue.
Though instead of returning -EINVAL I prefer an explicit error code for this
case. That way a driver can distinguish between the 'not connected' case and
other failure modes. Something like the patch below should work.
This patch looks great to me, thanks for all your help!

Chen, do you want to put all this together as formal patches with
changelogs and post to the mailing lists?
With pleasure. I will test it on my environment first and send it out.
thank all of you for this problem.

Chen


Bjorn

8<------------------
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -387,6 +387,22 @@ static inline int acpi_isa_register_gsi(
  }
  #endif
+static inline bool acpi_pci_irq_valid(struc pci_dev *dev)
+{
+#ifdef CONFIG_X86
+	/*
+	 * On x86 irq line 0xff means "unknown" or "no connection" (PCI 3.0,
+	 * Section 6.2.4, footnote on page 223).
+	 */
+	if (dev->irq == 0xff) {
+		dev->irq = IRQ_NOTCONNECTED;
+		dev_warn(&dev->dev, "PCI INT not connected\n");
+		return false;
+	}
+#endif
+	return true;
+}
+
  int acpi_pci_irq_enable(struct pci_dev *dev)
  {
  	struct acpi_prt_entry *entry;
@@ -409,6 +425,9 @@ int acpi_pci_irq_enable(struct pci_dev *
  	if (pci_has_managed_irq(dev))
  		return 0;
+ if (!acpi_pci_irq_valid(dev))
+		return 0;
+
  	entry = acpi_pci_irq_lookup(dev, pin);
  	if (!entry) {
  		/*
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -125,6 +125,16 @@ struct irqaction {
extern irqreturn_t no_action(int cpl, void *dev_id); +/*
+ * If a (PCI) device interrupt is not connected we set dev->irq to
+ * IRQ_NOTCONNECTED. This causes request_irq() to fail with -ENOTCONN, so we
+ * can distingiush that case from other error returns.
+ *
+ * 0x80000000 is guaranteed to be outside the available range of interrupts
+ * and easy to distinguish from other possible incorrect values.
+ */
+#define IRQ_NOTCONNECTED	(1U << 31)
+
  extern int __must_check
  request_threaded_irq(unsigned int irq, irq_handler_t handler,
  		     irq_handler_t thread_fn,
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1609,6 +1609,9 @@ int request_threaded_irq(unsigned int ir
  	struct irq_desc *desc;
  	int retval;
+ if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
  	/*
  	 * Sanity-check: shared interrupts must pass in a real dev-ID,
  	 * otherwise we'll have trouble later trying to figure out
@@ -1699,9 +1702,13 @@ EXPORT_SYMBOL(request_threaded_irq);
  int request_any_context_irq(unsigned int irq, irq_handler_t handler,
  			    unsigned long flags, const char *name, void *dev_id)
  {
-	struct irq_desc *desc = irq_to_desc(irq);
+	struct irq_desc *desc;
  	int ret;
+ if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
+	desc = irq_to_desc(irq);
  	if (!desc)
  		return -EINVAL;


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

.




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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux