From patchwork Wed Jan 27 09:13:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 8131321 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0AC509F1C0 for ; Wed, 27 Jan 2016 09:14:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 19CC9202F8 for ; Wed, 27 Jan 2016 09:14:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1036120274 for ; Wed, 27 Jan 2016 09:14:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753624AbcA0JOw (ORCPT ); Wed, 27 Jan 2016 04:14:52 -0500 Received: from www.linutronix.de ([62.245.132.108]:41522 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751583AbcA0JOs (ORCPT ); Wed, 27 Jan 2016 04:14:48 -0500 Received: from localhost ([127.0.0.1]) by Galois.linutronix.de with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1aOMBS-0006t1-Fy; Wed, 27 Jan 2016 10:14:38 +0100 Date: Wed, 27 Jan 2016 10:13:36 +0100 (CET) From: Thomas Gleixner To: Bjorn Helgaas cc: Chen Fan , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, rjw@rjwysocki.net, lenb@kernel.org, izumi.taku@jp.fujitsu.com, wency@cn.fujitsu.com, caoj.fnst@cn.fujitsu.com, ddaney.cavm@gmail.com, okaya@codeaurora.org, bhelgaas@google.com, jiang.liu@linux.intel.com, linux-pci@vger.kernel.org Subject: Re: [PATCH v2] pci: fix unavailable irq number 255 reported by BIOS In-Reply-To: <20160127002505.GA3329@localhost> Message-ID: References: <1453705178-27389-1-git-send-email-chen.fan.fnst@cn.fujitsu.com> <20160125205803.GA10272@localhost> <20160126152221.GB9279@localhost> <20160127002505.GA3329@localhost> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1, SHORTCIRCUIT=-0.0001 Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. Thanks, tglx 8<------------------ --- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- 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;