From patchwork Thu Aug 25 11:18:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Zyngier X-Patchwork-Id: 9299231 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 99EC7608A7 for ; Thu, 25 Aug 2016 11:18:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8AC5B26CF9 for ; Thu, 25 Aug 2016 11:18:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7F40B29289; Thu, 25 Aug 2016 11:18:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 184FC26CF9 for ; Thu, 25 Aug 2016 11:18:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753421AbcHYLSb (ORCPT ); Thu, 25 Aug 2016 07:18:31 -0400 Received: from foss.arm.com ([217.140.101.70]:56917 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752567AbcHYLSa (ORCPT ); Thu, 25 Aug 2016 07:18:30 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2C73326B; Thu, 25 Aug 2016 04:20:10 -0700 (PDT) Received: from localhost (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id ABDE73F220; Thu, 25 Aug 2016 04:18:29 -0700 (PDT) Received: from localhost ([::1]) by localhost with esmtp (Exim 4.87) (envelope-from ) id 1bcsfz-0000xe-Je; Thu, 25 Aug 2016 12:18:27 +0100 Date: Thu, 25 Aug 2016 12:18:25 +0100 From: Marc Zyngier To: Duc Dang Cc: Bjorn Helgaas , Lorenzo Pieralisi , Rafael Wysocki , , , patches , Bjorn Helgaas , Subject: Re: Defining polarity and trigger mode for static interrupts in _PRT Message-ID: <20160825121825.322d8450@arm.com> In-Reply-To: References: <20160824142723.GA25843@red-moon> <20160824193000.GE23914@localhost> <20160824213005.1a9300ef@arm.com> Organization: ARM Ltd X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.30; arm-unknown-linux-gnueabihf) MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 24 Aug 2016 15:19:21 -0700 Duc Dang wrote: > On Wed, Aug 24, 2016 at 1:30 PM, Marc Zyngier wrote: > > On Wed, 24 Aug 2016 14:30:00 -0500 > > Bjorn Helgaas wrote: > > > >> On Wed, Aug 24, 2016 at 03:27:23PM +0100, Lorenzo Pieralisi wrote: > >> > [ +Bjorn, Punit] > >> > > >> > On Wed, Aug 24, 2016 at 04:06:13AM -0700, Duc Dang wrote: > >> > > [Resend in plain text mode] > >> > > > >> > > Hi Lorenzo, Rafael, > >> > > > >> > > ACPI 6.1 spec does not specify how to set interrupt polarity and > >> > > trigger mode in _PRT when the interrupts are static (hardwired to > >> > > specific interrupt inputs in interrupt controller). In current > >> > > acpi_pci_irq_enable (drivers/acpi/pci_irq.c) implementation, by > >> > > default the trigger mode is set to LEVEL_SENSITIVE, polarity is set to > >> > > ACTIVE_LOW. This default setting won't work for ARM64 GICv2, GICv2m, > >> > > GICv3 controllers and will cause failures in PCIe AER, PME services > >> > > (on X-Gene platforms). > >> > >> PCI (not PCIe) r3.0, sec 2.2.6, says "Interrupts on PCI are optional > >> and defined as 'level sensitive,' asserted low." > >> > >> I've heard before that ARM64 does this differently, but I still don't > >> understand the difference. Obviously if you plug a legacy PCI card > >> into an ARM64 system, it's still going to pull INTA# low to assert an > >> interrupt. So is there something special about ARM64 that inverts > >> that, or what? > > > > There is certainly an inverter somewhere on the interrupt path, because > > the GIC triggers on level high, not level low. But I don't think that's > > the issue Duc is trying to outline here, because that's not something > > SW can fix. I'm worried that in his system, the interrupt is edge > > triggered instead. > > Yes, there is an inverter in the interrupt path to deliver interrupt to the GIC > as level-high. X-Gene GIC uses level high for PCI INTx. I myself has been > lucky when using trigger-rising for PCI INTx in DT boot mode. > > > > >> > >> > > Is there any way to specify polarity and trigger mode for static > >> > > interrupts in _PRT? > >> > >> There is no way I'm aware of in _PRT to specify polarity and trigger > >> mode. I don't know the history, but my guess is that it would be seen > >> as superfluous given that the PCI spec requires level, active low. > > The device still pulls the INTx pin low to trigger interrupt, but the > interrupt delivered > to interrupt controller (GIC in this case) is not necessarily to be > level-low. Current code > assume level-low mode to program to the interrupt controller for INTx, > and fails for > GIC, GICv2m and GICv3. Well, there's nothing that can't be fixed. The GIC doesn't have a programmatic notion of what is high or low. It only knows about level interrupts. But the HW only knows about level_high. Obviously, for things to work, the integrator has to put an inverter on the line to cope with level_low. If the driver code insist on using level_low, we can address this pretty easily, and just warn about the oddity: Does this work for you? Thanks, M. diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 6fc56c3..b3755a3 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -306,9 +306,16 @@ static int gic_set_type(struct irq_data *d, unsigned int type) return -EINVAL; /* SPIs have restrictions on the supported types */ - if (irq >= 32 && type != IRQ_TYPE_LEVEL_HIGH && - type != IRQ_TYPE_EDGE_RISING) - return -EINVAL; + if (irq >= 32) { + unsigned int tmp = type; + if (type == IRQ_TYPE_LEVEL_LOW) + type = IRQ_TYPE_LEVEL_HIGH; + if (type == IRQ_TYPE_EDGE_FALLING) + type = IRQ_TYPE_EDGE_RISING; + if (tmp != type) + pr_warn("Overriding IRQ%d type from %d to %d\n", + d->irq, tmp, type); + } if (gic_irq_in_rdist(d)) { base = gic_data_rdist_sgi_base(); diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index c2cab57..0d187dc 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -280,9 +280,16 @@ static int gic_set_type(struct irq_data *d, unsigned int type) return -EINVAL; /* SPIs have restrictions on the supported types */ - if (gicirq >= 32 && type != IRQ_TYPE_LEVEL_HIGH && - type != IRQ_TYPE_EDGE_RISING) - return -EINVAL; + if (gicirq >= 32) { + unsigned int tmp = type; + if (type == IRQ_TYPE_LEVEL_LOW) + type = IRQ_TYPE_LEVEL_HIGH; + if (type == IRQ_TYPE_EDGE_FALLING) + type = IRQ_TYPE_EDGE_RISING; + if (tmp != type) + pr_warn("Overriding IRQ%d type from %d to %d\n", + d->irq, tmp, type); + } return gic_configure_irq(gicirq, type, base, NULL); }