On Sat, Feb 06, 2016 at 04:43:14PM +0100, Heiner Kallweit wrote:
> Am 06.02.2016 um 16:19 schrieb Bjorn Helgaas:
> > On Sat, Feb 06, 2016 at 09:09:47AM +0100, Heiner Kallweit wrote:
> >> Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:
> >>> On Saturday, February 06, 2016 12:00:32 AM Heiner Kallweit wrote:
> >>>> Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255
> >>>> means "not connected"" I get several "PCI INT not connected" warnings on
> >>>> a Zotac CI321 and EHCI failes to load:
> >>>
> >>> That doesn't follow clearly from your report, but I'm assuming that it works
> >>> correctly without that commit, right?
> >>>
> >> Right, w/o this commit it looks like this:
> >>
> >> dmesg
> >> ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000
> >>
> >> /proc/interrupts
> >> IO-APIC 23-fasteoi ehci_hcd:usb3
> >
> > Thanks a lot for your report! This is a bit of a minefield, and I was
> > worried that we'd trip over something with a44c386a361e.
> >
> > Oops, I think I see a problem with a44c386a361e. We're checking for
> > Interrupt Line == 255 even before we try to look it up in the _PRT. I
> > think we should only check Interrupt Line *after* everything else has
> > failed. Can you try the patch below instead of a44c386a361e?
> >
> With the attached patch everything is fine. No warnings and EHCI loads properly.
Great, thanks a lot for testing this!
Chen, will you update and repost your patch? I know Rafael already
had it on a branch, but I think he dropped it, so we need to get the
update merged somehow.
Bjorn
> > If the patch below doesn't work, would you mind collecting the
> > complete output of "lspci -vvv" and the complete dmesg logs from
> > kernels with and without a44c386a361e, and putting them somewhere
> > (maybe a bugzilla.kernel.org report)?
> >
> > Bjorn
> >
> >
> > diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> > index d30184c..807a0a8 100644
> > --- a/drivers/acpi/pci_irq.c
> > +++ b/drivers/acpi/pci_irq.c
> > @@ -33,6 +33,7 @@
> > #include <linux/pci.h>
> > #include <linux/acpi.h>
> > #include <linux/slab.h>
> > +#include <linux/interrupt.h>
> >
> > #define PREFIX "ACPI: "
> >
> > @@ -387,6 +388,23 @@ static inline int acpi_isa_register_gsi(struct pci_dev *dev)
> > }
> > #endif
> >
> > +static inline bool acpi_pci_irq_valid(struct pci_dev *dev, u8 pin)
> > +{
> > +#ifdef CONFIG_X86
> > + /*
> > + * On x86, Interrupt 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 %c: not connected\n",
> > + pin_name(pin));
> > + return false;
> > + }
> > +#endif
> > + return true;
> > +}
> > +
> > int acpi_pci_irq_enable(struct pci_dev *dev)
> > {
> > struct acpi_prt_entry *entry;
> > @@ -431,11 +449,14 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
> > } else
> > gsi = -1;
> >
> > - /*
> > - * No IRQ known to the ACPI subsystem - maybe the BIOS /
> > - * driver reported one, then use it. Exit in any case.
> > - */
> > if (gsi < 0) {
> > + /*
> > + * No IRQ known to the ACPI subsystem - maybe the BIOS /
> > + * driver reported one, then use it. Exit in any case.
> > + */
> > + if (!acpi_pci_irq_valid(dev, pin))
> > + return 0;
> > +
> > if (acpi_isa_register_gsi(dev))
> > dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
> > pin_name(pin));
> > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> > index 0e95fcc..358076e 100644
> > --- 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,
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 8411872..e79e60f 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -1609,6 +1609,9 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
> > 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
