From: Bjorn Helgaas <helgaas@kernel.org> To: Paul Burton <paul.burton@imgtec.com> Cc: linux-mips@linux-mips.org, "Sören Brinkmann" <soren.brinkmann@xilinx.com>, "Michal Simek" <michal.simek@xilinx.com>, "Jiang Liu" <jiang.liu@linux.intel.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Rob Herring" <robh@kernel.org>, "Bjorn Helgaas" <bhelgaas@google.com>, linux-pci@vger.kernel.org, "Russell Joyce" <russell.joyce@york.ac.uk>, "Arnd Bergmann" <arnd@arndb.de>, linux-kernel@vger.kernel.org, "Jingoo Han" <jingoohan1@gmail.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 05/28] PCI: xilinx: keep references to both IRQ domains Date: Fri, 4 Dec 2015 12:17:49 -0600 [thread overview] Message-ID: <20151204181749.GB20125@localhost> (raw) In-Reply-To: <1448900513-20856-6-git-send-email-paul.burton@imgtec.com> Hi Paul, Please capitalize the first word of your PCI changelog subjects. On Mon, Nov 30, 2015 at 04:21:30PM +0000, Paul Burton wrote: > pcie-xilinx creates 2 IRQ domains when built with MSI support: one for > MSI interrupts & one for legacy INTx interrupts. However, it only kept a > reference to the MSI IRQ domain. This means that any INTx interrupts > that may occur would be mapped using the wrong domain, and that only the > MSI IRQ domain would be removed along with the driver. Track both IRQ > domains & clean up both as appropriate. It looks like this fixes a problem in the original commit 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver"), which appeared in v3.18. Does this need a stable backport tag? It sounds like any device using INTx just won't work? From later patches, I surmise that this series might be related to using Xilinx in a new MIPS Boston board. If that's the case, and pre-v4.5 kernels don't support that board anyway, a stable backport might not be needed. It *does* fix a leak even if you don't need INTx, but that seems minor and probably not worth a stable backport all by itself. I probably *would* add a 'Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")' line to leave breadcrumbs for people backporting things. This seems to be part of a larger series -- can these PCI patches go through my tree, or do they all need to go together because of dependencies on the rest of the series? They all need acks from Michal, of course. > Signed-off-by: Paul Burton <paul.burton@imgtec.com> > --- > > drivers/pci/host/pcie-xilinx.c | 58 ++++++++++++++++++++---------------------- > 1 file changed, 28 insertions(+), 30 deletions(-) > > diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c > index 3c7a0d5..c412a37 100644 > --- a/drivers/pci/host/pcie-xilinx.c > +++ b/drivers/pci/host/pcie-xilinx.c > @@ -105,6 +105,7 @@ > * @root_busno: Root Bus number > * @dev: Device pointer > * @irq_domain: IRQ domain pointer > + * @msi_irq_domain: MSI IRQ domain pointer > * @bus_range: Bus range > * @resources: Bus Resources > */ > @@ -115,6 +116,7 @@ struct xilinx_pcie_port { > u8 root_busno; > struct device *dev; > struct irq_domain *irq_domain; > + struct irq_domain *msi_irq_domain; > struct resource bus_range; > struct list_head resources; > }; > @@ -291,7 +293,7 @@ static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip, > if (hwirq < 0) > return hwirq; > > - irq = irq_create_mapping(port->irq_domain, hwirq); > + irq = irq_create_mapping(port->msi_irq_domain, hwirq); > if (!irq) > return -EINVAL; > > @@ -517,31 +519,21 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data) > > /** > * xilinx_pcie_free_irq_domain - Free IRQ domain > - * @port: PCIe port information > + * @domain: the IRQ domain to free > + * @nr: the number of IRQs in the domain > */ > -static void xilinx_pcie_free_irq_domain(struct xilinx_pcie_port *port) > +static void xilinx_pcie_free_irq_domain(struct irq_domain *domain, int nr) > { > int i; > - u32 irq, num_irqs; > - > - /* Free IRQ Domain */ > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > - > - free_pages(port->msi_pages, 0); > - > - num_irqs = XILINX_NUM_MSI_IRQS; > - } else { > - /* INTx */ > - num_irqs = 4; > - } > + u32 irq; > > - for (i = 0; i < num_irqs; i++) { > - irq = irq_find_mapping(port->irq_domain, i); > + for (i = 0; i < nr; i++) { > + irq = irq_find_mapping(domain, i); > if (irq > 0) > irq_dispose_mapping(irq); > } > > - irq_domain_remove(port->irq_domain); > + irq_domain_remove(domain); > } > > /** > @@ -571,20 +563,20 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port) > return PTR_ERR(port->irq_domain); > } > > - /* Setup MSI */ > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > - port->irq_domain = irq_domain_add_linear(node, > - XILINX_NUM_MSI_IRQS, > - &msi_domain_ops, > - &xilinx_pcie_msi_chip); > - if (!port->irq_domain) { > - dev_err(dev, "Failed to get a MSI IRQ domain\n"); > - return PTR_ERR(port->irq_domain); > - } > + if (!IS_ENABLED(CONFIG_PCI_MSI)) > + return 0; > > - xilinx_pcie_enable_msi(port); > + /* Setup MSI */ > + port->msi_irq_domain = irq_domain_add_linear(node, > + XILINX_NUM_MSI_IRQS, > + &msi_domain_ops, > + &xilinx_pcie_msi_chip); > + if (!port->msi_irq_domain) { > + dev_err(dev, "Failed to get a MSI IRQ domain\n"); > + return PTR_ERR(port->msi_irq_domain); > } > > + xilinx_pcie_enable_msi(port); > return 0; > } > > @@ -868,7 +860,13 @@ static int xilinx_pcie_remove(struct platform_device *pdev) > { > struct xilinx_pcie_port *port = platform_get_drvdata(pdev); > > - xilinx_pcie_free_irq_domain(port); > + xilinx_pcie_free_irq_domain(port->irq_domain, 4); > + > + if (config_enabled(CONFIG_MSI)) { > + free_pages(port->msi_pages, 0); > + xilinx_pcie_free_irq_domain(port->msi_irq_domain, > + XILINX_NUM_MSI_IRQS); > + } > > return 0; > } > -- > 2.6.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
WARNING: multiple messages have this Message-ID (diff)
From: helgaas@kernel.org (Bjorn Helgaas) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 05/28] PCI: xilinx: keep references to both IRQ domains Date: Fri, 4 Dec 2015 12:17:49 -0600 [thread overview] Message-ID: <20151204181749.GB20125@localhost> (raw) In-Reply-To: <1448900513-20856-6-git-send-email-paul.burton@imgtec.com> Hi Paul, Please capitalize the first word of your PCI changelog subjects. On Mon, Nov 30, 2015 at 04:21:30PM +0000, Paul Burton wrote: > pcie-xilinx creates 2 IRQ domains when built with MSI support: one for > MSI interrupts & one for legacy INTx interrupts. However, it only kept a > reference to the MSI IRQ domain. This means that any INTx interrupts > that may occur would be mapped using the wrong domain, and that only the > MSI IRQ domain would be removed along with the driver. Track both IRQ > domains & clean up both as appropriate. It looks like this fixes a problem in the original commit 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver"), which appeared in v3.18. Does this need a stable backport tag? It sounds like any device using INTx just won't work? From later patches, I surmise that this series might be related to using Xilinx in a new MIPS Boston board. If that's the case, and pre-v4.5 kernels don't support that board anyway, a stable backport might not be needed. It *does* fix a leak even if you don't need INTx, but that seems minor and probably not worth a stable backport all by itself. I probably *would* add a 'Fixes: 8961def56845 ("PCI: xilinx: Add Xilinx AXI PCIe Host Bridge IP driver")' line to leave breadcrumbs for people backporting things. This seems to be part of a larger series -- can these PCI patches go through my tree, or do they all need to go together because of dependencies on the rest of the series? They all need acks from Michal, of course. > Signed-off-by: Paul Burton <paul.burton@imgtec.com> > --- > > drivers/pci/host/pcie-xilinx.c | 58 ++++++++++++++++++++---------------------- > 1 file changed, 28 insertions(+), 30 deletions(-) > > diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c > index 3c7a0d5..c412a37 100644 > --- a/drivers/pci/host/pcie-xilinx.c > +++ b/drivers/pci/host/pcie-xilinx.c > @@ -105,6 +105,7 @@ > * @root_busno: Root Bus number > * @dev: Device pointer > * @irq_domain: IRQ domain pointer > + * @msi_irq_domain: MSI IRQ domain pointer > * @bus_range: Bus range > * @resources: Bus Resources > */ > @@ -115,6 +116,7 @@ struct xilinx_pcie_port { > u8 root_busno; > struct device *dev; > struct irq_domain *irq_domain; > + struct irq_domain *msi_irq_domain; > struct resource bus_range; > struct list_head resources; > }; > @@ -291,7 +293,7 @@ static int xilinx_pcie_msi_setup_irq(struct msi_controller *chip, > if (hwirq < 0) > return hwirq; > > - irq = irq_create_mapping(port->irq_domain, hwirq); > + irq = irq_create_mapping(port->msi_irq_domain, hwirq); > if (!irq) > return -EINVAL; > > @@ -517,31 +519,21 @@ static irqreturn_t xilinx_pcie_intr_handler(int irq, void *data) > > /** > * xilinx_pcie_free_irq_domain - Free IRQ domain > - * @port: PCIe port information > + * @domain: the IRQ domain to free > + * @nr: the number of IRQs in the domain > */ > -static void xilinx_pcie_free_irq_domain(struct xilinx_pcie_port *port) > +static void xilinx_pcie_free_irq_domain(struct irq_domain *domain, int nr) > { > int i; > - u32 irq, num_irqs; > - > - /* Free IRQ Domain */ > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > - > - free_pages(port->msi_pages, 0); > - > - num_irqs = XILINX_NUM_MSI_IRQS; > - } else { > - /* INTx */ > - num_irqs = 4; > - } > + u32 irq; > > - for (i = 0; i < num_irqs; i++) { > - irq = irq_find_mapping(port->irq_domain, i); > + for (i = 0; i < nr; i++) { > + irq = irq_find_mapping(domain, i); > if (irq > 0) > irq_dispose_mapping(irq); > } > > - irq_domain_remove(port->irq_domain); > + irq_domain_remove(domain); > } > > /** > @@ -571,20 +563,20 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port) > return PTR_ERR(port->irq_domain); > } > > - /* Setup MSI */ > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > - port->irq_domain = irq_domain_add_linear(node, > - XILINX_NUM_MSI_IRQS, > - &msi_domain_ops, > - &xilinx_pcie_msi_chip); > - if (!port->irq_domain) { > - dev_err(dev, "Failed to get a MSI IRQ domain\n"); > - return PTR_ERR(port->irq_domain); > - } > + if (!IS_ENABLED(CONFIG_PCI_MSI)) > + return 0; > > - xilinx_pcie_enable_msi(port); > + /* Setup MSI */ > + port->msi_irq_domain = irq_domain_add_linear(node, > + XILINX_NUM_MSI_IRQS, > + &msi_domain_ops, > + &xilinx_pcie_msi_chip); > + if (!port->msi_irq_domain) { > + dev_err(dev, "Failed to get a MSI IRQ domain\n"); > + return PTR_ERR(port->msi_irq_domain); > } > > + xilinx_pcie_enable_msi(port); > return 0; > } > > @@ -868,7 +860,13 @@ static int xilinx_pcie_remove(struct platform_device *pdev) > { > struct xilinx_pcie_port *port = platform_get_drvdata(pdev); > > - xilinx_pcie_free_irq_domain(port); > + xilinx_pcie_free_irq_domain(port->irq_domain, 4); > + > + if (config_enabled(CONFIG_MSI)) { > + free_pages(port->msi_pages, 0); > + xilinx_pcie_free_irq_domain(port->msi_irq_domain, > + XILINX_NUM_MSI_IRQS); > + } > > return 0; > } > -- > 2.6.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
next prev parent reply other threads:[~2015-12-04 18:17 UTC|newest] Thread overview: 94+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-11-30 16:21 [PATCH 00/28] MIPS Boston board support Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [rtc-linux] " Paul Burton 2015-11-30 16:21 ` [PATCH 01/28] serial: earlycon: allow MEM32 I/O for DT earlycon Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 22:52 ` Rob Herring 2015-11-30 22:52 ` Rob Herring 2015-12-02 23:38 ` Peter Hurley 2015-11-30 16:21 ` [PATCH 02/28] dt-bindings: ascii-lcd: Document a binding for simple ASCII LCDs Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 18:57 ` Rob Herring 2015-11-30 18:57 ` Rob Herring 2015-11-30 16:21 ` [PATCH 03/28] auxdisplay: driver for simple memory mapped ASCII LCD displays Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 04/28] MIPS: PCI: compatibility with ARM-like PCI host drivers Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 05/28] PCI: xilinx: keep references to both IRQ domains Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-04 18:17 ` Bjorn Helgaas [this message] 2015-12-04 18:17 ` Bjorn Helgaas 2015-11-30 16:21 ` [PATCH 06/28] PCI: xilinx: unify INTx & MSI interrupt FIFO decode Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 07/28] PCI: xilinx: always clear interrupt decode register Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 08/28] PCI: xilinx: fix INTX irq dispatch Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 09/28] PCI: xilinx: allow build on MIPS platforms Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 10/28] misc: pch_phub: " Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 11/28] dmaengine: pch_dma: " Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-10 3:49 ` Vinod Koul 2015-11-30 16:21 ` [PATCH 12/28] gpio: pch: " Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-10 16:23 ` Linus Walleij 2015-11-30 16:21 ` [PATCH 13/28] gpio: pch: allow use from device tree Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-10 16:25 ` Linus Walleij 2015-11-30 16:21 ` [PATCH 14/28] i2c: eg20t: allow build on MIPS platforms Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-03 21:05 ` Wolfram Sang 2015-11-30 16:21 ` [PATCH 15/28] i2c: eg20t: set i2c_adapter->dev.of_node Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-03 21:06 ` Wolfram Sang 2015-11-30 16:21 ` [PATCH 16/28] rtc: m41t80: add devicetree probe support Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [rtc-linux] " Paul Burton 2015-11-30 16:56 ` Alexandre Belloni 2015-11-30 16:56 ` [rtc-linux] " Alexandre Belloni 2015-11-30 16:21 ` [PATCH 17/28] spi: topcliff-pch: allow build for MIPS platforms Paul Burton 2015-11-30 16:21 ` Paul Burton [not found] ` <1448900513-20856-18-git-send-email-paul.burton-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> 2015-11-30 16:39 ` Applied "spi: topcliff-pch: allow build for MIPS platforms" to the spi tree Mark Brown 2015-11-30 16:21 ` [PATCH 18/28] ptp: pch: allow build on MIPS platforms Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-01 8:32 ` Richard Cochran 2015-11-30 16:21 ` [PATCH 19/28] net: pch_gbe: " Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 20/28] net: pch_gbe: clear interrupt FIFO during probe Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-01 1:48 ` Florian Fainelli 2015-12-01 1:48 ` Florian Fainelli 2015-11-30 16:21 ` [PATCH 21/28] net: pch_gbe: mark Minnow PHY reset GPIO active low Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 22/28] net: pch_gbe: pull PHY GPIO handling out of Minnow code Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 23/28] net: pch_gbe: always reset PHY along with MAC Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 24/28] net: pch_gbe: add device tree support Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-12-13 1:22 ` Andy Shevchenko 2015-11-30 16:21 ` [PATCH 25/28] net: pch_gbe: allow longer for resets Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 26/28] MIPS: support for generating FIT (.itb) images Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:21 ` [PATCH 27/28] dt-bindings: mips: img,boston: Document img,boston binding Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 18:34 ` Rob Herring 2015-11-30 16:21 ` [PATCH 28/28] MIPS: Boston board support Paul Burton 2015-11-30 16:21 ` Paul Burton 2015-11-30 16:34 ` [PATCH 00/28] MIPS " Mark Brown 2015-11-30 16:34 ` Mark Brown 2015-11-30 16:34 ` [rtc-linux] " Mark Brown 2015-12-10 16:26 ` Linus Walleij 2015-12-10 16:26 ` Linus Walleij 2015-12-10 16:26 ` [rtc-linux] " Linus Walleij
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20151204181749.GB20125@localhost \ --to=helgaas@kernel.org \ --cc=arnd@arndb.de \ --cc=bhelgaas@google.com \ --cc=jiang.liu@linux.intel.com \ --cc=jingoohan1@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@linux-mips.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=michal.simek@xilinx.com \ --cc=paul.burton@imgtec.com \ --cc=robh@kernel.org \ --cc=russell.joyce@york.ac.uk \ --cc=soren.brinkmann@xilinx.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.