From patchwork Thu Aug 17 16:53:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 9906691 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 2E6816038C for ; Thu, 17 Aug 2017 16:54:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1F1B228B26 for ; Thu, 17 Aug 2017 16:54:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 12B4B28B6C; Thu, 17 Aug 2017 16:54:01 +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=ham 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 3FEE328B26 for ; Thu, 17 Aug 2017 16:54:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753163AbdHQQx6 (ORCPT ); Thu, 17 Aug 2017 12:53:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:51678 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752236AbdHQQx5 (ORCPT ); Thu, 17 Aug 2017 12:53:57 -0400 Received: from localhost (unknown [69.55.156.165]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 8B56E22B4E; Thu, 17 Aug 2017 16:53:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8B56E22B4E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Thu, 17 Aug 2017 11:53:54 -0500 From: Bjorn Helgaas To: Kishon Vijay Abraham I Cc: Bjorn Helgaas , linux-pci@vger.kernel.org, Paul Burton Subject: Re: [PATCH] PCI: dra7xx: Use PCI_NUM_INTX Message-ID: <20170817165354.GI28977@bhelgaas-glaptop.roam.corp.google.com> References: <20170815213838.8231.59214.stgit@bhelgaas-glaptop.roam.corp.google.com> <20170815215225.GP32525@bhelgaas-glaptop.roam.corp.google.com> <3bb78881-48bf-cd2f-0130-f6b54e4fc762@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3bb78881-48bf-cd2f-0130-f6b54e4fc762@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Thu, Aug 17, 2017 at 09:51:29PM +0530, Kishon Vijay Abraham I wrote: > Hi Bjorn, > > On Wednesday 16 August 2017 03:22 AM, Bjorn Helgaas wrote: > > On Tue, Aug 15, 2017 at 04:38:38PM -0500, Bjorn Helgaas wrote: > >> Use the PCI_NUM_INTX macro to indicate the number of PCI INTx interrupts > >> rather than the magic number 4. This makes it clearer where the number > >> comes from & what it relates to. > >> > >> Signed-off-by: Bjorn Helgaas > >> Cc: Kishon Vijay Abraham I > >> --- > >> drivers/pci/dwc/pci-dra7xx.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c > >> index f2fc5f47064e..30131ecaadea 100644 > >> --- a/drivers/pci/dwc/pci-dra7xx.c > >> +++ b/drivers/pci/dwc/pci-dra7xx.c > >> @@ -238,7 +238,7 @@ static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp) > >> return -ENODEV; > >> } > >> > >> - dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, 4, > >> + dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX, > >> &intx_domain_ops, pp); > >> if (!dra7xx->irq_domain) { > >> dev_err(dev, "Failed to get a INTx IRQ domain\n"); > >> > > > > Oops. I think this patch is OK as far as it goes, but Kishon > > confirmed [1] that INTD was broken in dra7xx, and AFAIK it's still > > broken, so maybe we need to also use pci_irqd_intx_xlate() as in the > > following? > > > > [1] https://lkml.org/lkml/2016/9/14/241 > > > > > > > > commit 569f29aa4ff0ab4bd4d1782b3a806a7561fe9db5 > > Author: Bjorn Helgaas > > Date: Tue Aug 15 16:28:27 2017 -0500 > > > > PCI: dra7xx: Translate INTx range to hwirqs 0-3 > > > > The pci-dra7xx driver creates an IRQ domain of size 4 for legacy PCI INTx > > interrupts, which at first glance seems reasonable since there are 4 possible > > such interrupts. Unfortunately the driver then proceeds to use the range 1-4 > > as the hwirq numbers for INTA-INTD, causing warnings & broken interrupts when > > attempting to use INTD/hwirq=4 due to it being beyond the range of the IRQ > > domain: > > > > WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:342 irq_domain_associate+0x12c/0x1c4 > > error: hwirq 0x4 is too large for dummy > > Modules linked in: > > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc6-01351-gd4fcf3d-dirty #15 > > Hardware name: Generic DRA72X (Flattened Device Tree) > > (unwind_backtrace) from [] (show_stack+0x10/0x14) > > (show_stack) from [] (dump_stack+0xac/0xe0) > > (dump_stack) from [] (__warn+0xd8/0x104) > > (__warn) from [] (warn_slowpath_fmt+0x34/0x44) > > (warn_slowpath_fmt) from [] (irq_domain_associate+0x12c/0x1c4) > > (irq_domain_associate) from [] (irq_create_mapping+0x64/0xcc) > > (irq_create_mapping) from [] (irq_create_fwspec_mapping+0xac/0x2fc) > > (irq_create_fwspec_mapping) from [] (irq_create_of_mapping+0x54/0x5c) > > (irq_create_of_mapping) from [] (of_irq_parse_and_map_pci+0x24/0x2c) > > (of_irq_parse_and_map_pci) from [] (pci_fixup_irqs+0x44/0xb8) > > (pci_fixup_irqs) from [] (dw_pcie_host_init+0x234/0x3e4) > > (dw_pcie_host_init) from [] (dra7xx_pcie_probe+0x418/0x5c4) > > (dra7xx_pcie_probe) from [] (platform_drv_probe+0x4c/0xb0) > > > > Fix this by making use of the new pci_irqd_intx_xlate() helper to translate > > the INTx 1-4 range into the 0-3 range suitable for the IRQ domain of size > > 4, and stop adding 1 to the hwirq number decoded from the interrupt FIFO > > which is already in the range 0-3. > > > > Whilst we're here we switch to using PCI_NUM_INTX rather than the magic > > number 4, making it clearer what the 4 means. > > > > Based-on-similar-patches-by: Paul Burton > > Signed-off-by: Bjorn Helgaas > > Cc: Kishon Vijay Abraham I > > > > diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c > > index f2fc5f47064e..55f16fcf701d 100644 > > --- a/drivers/pci/dwc/pci-dra7xx.c > > +++ b/drivers/pci/dwc/pci-dra7xx.c > > @@ -223,6 +223,7 @@ static int dra7xx_pcie_intx_map(struct irq_domain *domain, unsigned int irq, > > > > static const struct irq_domain_ops intx_domain_ops = { > > .map = dra7xx_pcie_intx_map, > > + .xlate = pci_irqd_intx_xlate, > > }; > > > > static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp) > > @@ -238,7 +239,7 @@ static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp) > > return -ENODEV; > > } > > > > - dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, 4, > > + dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX, > > &intx_domain_ops, pp); > > if (!dra7xx->irq_domain) { > > dev_err(dev, "Failed to get a INTx IRQ domain\n"); > > With this patch, I don't see [1] mentioned anymore. > However I think there are other issues in pci-dra7xx which doesn't allow me to > use INTD. When I tried to set 0x4 in interrupt_pin of pcie endpoint and raise > legacy interrupt, I get the following dump in RC. > [ 102.348957] irq 0, desc: ee80d800, depth: 1, count: 0, unhandled: 0 > [ 102.355536] ->handle_irq(): c01a6994, > [ 102.355553] handle_bad_irq+0x0/0x268 > [ 102.363311] ->irq_data.chip(): c0d46f50, > [ 102.363325] no_irq_chip+0x0/0x88 > [ 102.370903] ->action(): (null) > [ 102.374290] IRQ_NOPROBE set > [ 102.377490] IRQ_NOREQUEST set > [ 102.380692] unexpected IRQ trap at vector 00 > > I'll look into this. Thanks for trying this out. In the meantime, I dropped the translation part of the patch, resulting in the patch below. I don't want to exchange the "error: hwirq 0x4 is too large for dummy" problem for another unknown problem. If/when you fix the INTD issue, pci_irqd_intx_xlate() might be part of the solution, but sounds like there's more to it. [1] https://lkml.org/lkml/2016/9/14/241 commit 61534d1a4c79501261bb8c534f992e8c8e1353da Author: Bjorn Helgaas Date: Tue Aug 15 16:28:27 2017 -0500 PCI: dra7xx: Use PCI_NUM_INTX Use the PCI_NUM_INTX macro to indicate the number of PCI INTx interrupts rather than the magic number 4. This makes it clearer where the number comes from & what it relates to. Based-on-similar-patches-by: Paul Burton Signed-off-by: Bjorn Helgaas Cc: Kishon Vijay Abraham I diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c index f2fc5f47064e..30131ecaadea 100644 --- a/drivers/pci/dwc/pci-dra7xx.c +++ b/drivers/pci/dwc/pci-dra7xx.c @@ -238,7 +238,7 @@ static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp) return -ENODEV; } - dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, 4, + dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX, &intx_domain_ops, pp); if (!dra7xx->irq_domain) { dev_err(dev, "Failed to get a INTx IRQ domain\n");