* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping @ 2014-02-28 1:19 Tim Harvey 2014-02-28 1:19 ` [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping Tim Harvey ` (3 more replies) 0 siblings, 4 replies; 29+ messages in thread From: Tim Harvey @ 2014-02-28 1:19 UTC (permalink / raw) To: linux-arm-kernel An add-in card used on the Ventana IMX6 SoC based family of boards has a TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings between the bridge and the four mini-PCI slots are swapped (INTD/C/B/A). This series adds legacy PCI interrupt mapping to support PCI bridges on IMX6 PCIe host controllers, adds the ability to use a host-specific custom swizzle function, then uses that to fix the IRQ mapping. I'm hoping this is the right way to perform such a workaround - please provide feedback if not. Patch1 will likely collide with the owrk Lucas is doing with device-tree bindings and MSI support. I'm happy to re-base if/when needed. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Zhu <r65037@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Sean Cross <xobs@kosagi.com> Cc: Jingoo Han <jg1.han@samsung.com> --- Tim Harvey (3): PCI: designware: add legacy PCI interrupt mapping PCI: designware: add ability for custom swizzle PCI: imx6: ventana: fixup for IRQ mismapping arch/arm/boot/dts/imx6qdl.dtsi | 2 +- drivers/pci/host/pci-exynos.c | 7 ++++-- drivers/pci/host/pci-imx6.c | 47 ++++++++++++++++++++++++++++++++++---- drivers/pci/host/pcie-designware.c | 11 ++++++++- drivers/pci/host/pcie-designware.h | 3 ++- include/linux/pci_ids.h | 1 + 6 files changed, 62 insertions(+), 9 deletions(-) -- 1.8.3.2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 1:19 [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping Tim Harvey @ 2014-02-28 1:19 ` Tim Harvey 2014-02-28 2:00 ` Jingoo Han 2014-02-28 2:10 ` Shawn Guo 2014-02-28 1:19 ` [PATCH 2/3] PCI: designware: add ability for custom swizzle Tim Harvey ` (2 subsequent siblings) 3 siblings, 2 replies; 29+ messages in thread From: Tim Harvey @ 2014-02-28 1:19 UTC (permalink / raw) To: linux-arm-kernel The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. This allows a PCIe-to-PCI bridge to function properly. The irq field of the pcie_host struct is expanded to 4 interrupts to allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them all from devicetree. I'm not clear if the Exynos driver has this capability so it places the same interrupt in all 4 slots. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Zhu <r65037@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Sean Cross <xobs@kosagi.com> Cc: Jingoo Han <jg1.han@samsung.com> --- arch/arm/boot/dts/imx6qdl.dtsi | 2 +- drivers/pci/host/pci-exynos.c | 7 +++++-- drivers/pci/host/pci-imx6.c | 11 +++++++---- drivers/pci/host/pcie-designware.c | 8 +++++++- drivers/pci/host/pcie-designware.h | 2 +- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index eed6897..fad8d90 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -126,7 +126,7 @@ 0x81000000 0 0 0x01f80000 0 0x00010000 /* downstream I/O */ 0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable memory */ num-lanes = <1>; - interrupts = <0 123 0x04>; + interrupts = <0 123 0x04>, <0 122 0x04>, <0 121 0x04>, <0 120 0x04>; clocks = <&clks 189>, <&clks 187>, <&clks 206>, <&clks 144>; clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_gate", "pcie_axi"; status = "disabled"; diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c index 3de6bfb..d85dcb0 100644 --- a/drivers/pci/host/pci-exynos.c +++ b/drivers/pci/host/pci-exynos.c @@ -515,11 +515,14 @@ static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev) { int ret; - pp->irq = platform_get_irq(pdev, 1); - if (!pp->irq) { + pp->irq[0] = platform_get_irq(pdev, 1); + if (!pp->irq[0]) { dev_err(&pdev->dev, "failed to get irq\n"); return -ENODEV; } + pp->irq[1] = pp->irq[0]; + pp->irq[2] = pp->irq[0]; + pp->irq[3] = pp->irq[0]; ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler, IRQF_SHARED, "exynos-pcie", pp); if (ret) { diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index e8663a8..aaa05c8 100644 --- a/drivers/pci/host/pci-imx6.c +++ b/drivers/pci/host/pci-imx6.c @@ -470,11 +470,14 @@ static int imx6_add_pcie_port(struct pcie_port *pp, struct platform_device *pdev) { int ret; + int i; - pp->irq = platform_get_irq(pdev, 0); - if (!pp->irq) { - dev_err(&pdev->dev, "failed to get irq\n"); - return -ENODEV; + for (i = 0; i < 4; i++) { + pp->irq[i] = platform_get_irq(pdev, i); + if (!pp->irq[i]) { + dev_err(&pdev->dev, "failed to get irq%d\n", i); + return -ENODEV; + } } pp->root_bus_nr = -1; diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 17ce88f..5808177 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -738,8 +738,14 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys) static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) { struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); + int irq = -1; - return pp->irq; + if (pin > 0 && pin < 5) + irq = pp->irq[pin - 1]; + dev_info(&dev->dev, "map_irq: %04x:%04x slot%d pin%d irq%d\n", + dev->vendor, dev->device, slot, pin, irq); + + return irq; } static void dw_pcie_add_bus(struct pci_bus *bus) diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h index 3063b35..5c596c0 100644 --- a/drivers/pci/host/pcie-designware.h +++ b/drivers/pci/host/pcie-designware.h @@ -46,7 +46,7 @@ struct pcie_port { struct resource io; struct resource mem; struct pcie_port_info config; - int irq; + int irq[4]; u32 lanes; struct pcie_host_ops *ops; int msi_irq; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 1:19 ` [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping Tim Harvey @ 2014-02-28 2:00 ` Jingoo Han 2014-02-28 4:24 ` Tim Harvey 2014-02-28 2:10 ` Shawn Guo 1 sibling, 1 reply; 29+ messages in thread From: Jingoo Han @ 2014-02-28 2:00 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. > This allows a PCIe-to-PCI bridge to function properly. > > The irq field of the pcie_host struct is expanded to 4 interrupts to > allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them > all from devicetree. I'm not clear if the Exynos driver has this > capability so it places the same interrupt in all 4 slots. (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) In the case of Exynos, 'INTA/B/C/D' are mapped to only one interrupt (<0 20 0>). Thus, the current code works properly on Exynos platform. There are three interrupts for Exynos PCIe; INTx, MSI, PHY Link, respectively as below. ./arch/arm/boot/dts/exynos5440.dtsi interrupts = <0 20 0>, <0 21 0>, <0 22 0>; <0 20 0>: PCIe RC0 pulse interrupt, INTA, INTB, INTC and INTD, etc <0 21 0>: PCIe RC0 level interrupt, MSI, etc <0 22 0>: PCIe RC0 special interrupt, PHY Link related interrupts, etc Of course, legacy INTx is handled as message only. Mohit, Kishon, How about the other SoCs? INTx is mapped to single interrupt such as Exynos, or separate interrupts such as i.MX6? Best regards, Jingoo Han > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Zhu <r65037@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Sean Cross <xobs@kosagi.com> > Cc: Jingoo Han <jg1.han@samsung.com> > --- > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- > drivers/pci/host/pci-exynos.c | 7 +++++-- > drivers/pci/host/pci-imx6.c | 11 +++++++---- > drivers/pci/host/pcie-designware.c | 8 +++++++- > drivers/pci/host/pcie-designware.h | 2 +- > 5 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > index eed6897..fad8d90 100644 > --- a/arch/arm/boot/dts/imx6qdl.dtsi > +++ b/arch/arm/boot/dts/imx6qdl.dtsi > @@ -126,7 +126,7 @@ > 0x81000000 0 0 0x01f80000 0 0x00010000 /* downstream I/O */ > 0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable > memory */ > num-lanes = <1>; > - interrupts = <0 123 0x04>; > + interrupts = <0 123 0x04>, <0 122 0x04>, <0 121 0x04>, <0 120 0x04>; > clocks = <&clks 189>, <&clks 187>, <&clks 206>, <&clks 144>; > clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_gate", "pcie_axi"; > status = "disabled"; > diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c > index 3de6bfb..d85dcb0 100644 > --- a/drivers/pci/host/pci-exynos.c > +++ b/drivers/pci/host/pci-exynos.c > @@ -515,11 +515,14 @@ static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev) > { > int ret; > > - pp->irq = platform_get_irq(pdev, 1); > - if (!pp->irq) { > + pp->irq[0] = platform_get_irq(pdev, 1); > + if (!pp->irq[0]) { > dev_err(&pdev->dev, "failed to get irq\n"); > return -ENODEV; > } > + pp->irq[1] = pp->irq[0]; > + pp->irq[2] = pp->irq[0]; > + pp->irq[3] = pp->irq[0]; > ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler, > IRQF_SHARED, "exynos-pcie", pp); > if (ret) { > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index e8663a8..aaa05c8 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -470,11 +470,14 @@ static int imx6_add_pcie_port(struct pcie_port *pp, > struct platform_device *pdev) > { > int ret; > + int i; > > - pp->irq = platform_get_irq(pdev, 0); > - if (!pp->irq) { > - dev_err(&pdev->dev, "failed to get irq\n"); > - return -ENODEV; > + for (i = 0; i < 4; i++) { > + pp->irq[i] = platform_get_irq(pdev, i); > + if (!pp->irq[i]) { > + dev_err(&pdev->dev, "failed to get irq%d\n", i); > + return -ENODEV; > + } > } > > pp->root_bus_nr = -1; > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > index 17ce88f..5808177 100644 > --- a/drivers/pci/host/pcie-designware.c > +++ b/drivers/pci/host/pcie-designware.c > @@ -738,8 +738,14 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys) > static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) > { > struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); > + int irq = -1; > > - return pp->irq; > + if (pin > 0 && pin < 5) > + irq = pp->irq[pin - 1]; > + dev_info(&dev->dev, "map_irq: %04x:%04x slot%d pin%d irq%d\n", > + dev->vendor, dev->device, slot, pin, irq); > + > + return irq; > } > > static void dw_pcie_add_bus(struct pci_bus *bus) > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > index 3063b35..5c596c0 100644 > --- a/drivers/pci/host/pcie-designware.h > +++ b/drivers/pci/host/pcie-designware.h > @@ -46,7 +46,7 @@ struct pcie_port { > struct resource io; > struct resource mem; > struct pcie_port_info config; > - int irq; > + int irq[4]; > u32 lanes; > struct pcie_host_ops *ops; > int msi_irq; > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 2:00 ` Jingoo Han @ 2014-02-28 4:24 ` Tim Harvey 2014-02-28 7:01 ` Jingoo Han 0 siblings, 1 reply; 29+ messages in thread From: Tim Harvey @ 2014-02-28 4:24 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 27, 2014 at 6:00 PM, Jingoo Han <jg1.han@samsung.com> wrote: > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: >> >> The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. >> This allows a PCIe-to-PCI bridge to function properly. >> >> The irq field of the pcie_host struct is expanded to 4 interrupts to >> allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them >> all from devicetree. I'm not clear if the Exynos driver has this >> capability so it places the same interrupt in all 4 slots. > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) > > In the case of Exynos, > 'INTA/B/C/D' are mapped to only one interrupt (<0 20 0>). > Thus, the current code works properly on Exynos platform. > > There are three interrupts for Exynos PCIe; INTx, MSI, PHY Link, > respectively as below. > > ./arch/arm/boot/dts/exynos5440.dtsi > interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > <0 20 0>: PCIe RC0 pulse interrupt, > INTA, INTB, INTC and INTD, etc > <0 21 0>: PCIe RC0 level interrupt, > MSI, etc > <0 22 0>: PCIe RC0 special interrupt, > PHY Link related interrupts, etc > > Of course, legacy INTx is handled as message only. > > Mohit, Kishon, > How about the other SoCs? INTx is mapped to single interrupt > such as Exynos, or separate interrupts such as i.MX6? Jingoo, Ok - so at least the I.MX6 and Exynos, which both use the designware IP need different IRQ mappings. It seems to me then that the map_irq should be moved out of drivers/pci/host/pcie-designware.c and into the SoC specific host controller drivers (pci-imx.c and pci-exynos.c). If that becomes the consensus I can submit a patch that does that. Thanks, Tim > > Best regards, > Jingoo Han > >> >> Signed-off-by: Tim Harvey <tharvey@gateworks.com> >> Cc: Bjorn Helgaas <bhelgaas@google.com> >> Cc: Richard Zhu <r65037@freescale.com> >> Cc: Shawn Guo <shawn.guo@linaro.org> >> Cc: Lucas Stach <l.stach@pengutronix.de> >> Cc: Sean Cross <xobs@kosagi.com> >> Cc: Jingoo Han <jg1.han@samsung.com> >> --- >> arch/arm/boot/dts/imx6qdl.dtsi | 2 +- >> drivers/pci/host/pci-exynos.c | 7 +++++-- >> drivers/pci/host/pci-imx6.c | 11 +++++++---- >> drivers/pci/host/pcie-designware.c | 8 +++++++- >> drivers/pci/host/pcie-designware.h | 2 +- >> 5 files changed, 21 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi >> index eed6897..fad8d90 100644 >> --- a/arch/arm/boot/dts/imx6qdl.dtsi >> +++ b/arch/arm/boot/dts/imx6qdl.dtsi >> @@ -126,7 +126,7 @@ >> 0x81000000 0 0 0x01f80000 0 0x00010000 /* downstream I/O */ >> 0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable >> memory */ >> num-lanes = <1>; >> - interrupts = <0 123 0x04>; >> + interrupts = <0 123 0x04>, <0 122 0x04>, <0 121 0x04>, <0 120 0x04>; >> clocks = <&clks 189>, <&clks 187>, <&clks 206>, <&clks 144>; >> clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_gate", "pcie_axi"; >> status = "disabled"; >> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c >> index 3de6bfb..d85dcb0 100644 >> --- a/drivers/pci/host/pci-exynos.c >> +++ b/drivers/pci/host/pci-exynos.c >> @@ -515,11 +515,14 @@ static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev) >> { >> int ret; >> >> - pp->irq = platform_get_irq(pdev, 1); >> - if (!pp->irq) { >> + pp->irq[0] = platform_get_irq(pdev, 1); >> + if (!pp->irq[0]) { >> dev_err(&pdev->dev, "failed to get irq\n"); >> return -ENODEV; >> } >> + pp->irq[1] = pp->irq[0]; >> + pp->irq[2] = pp->irq[0]; >> + pp->irq[3] = pp->irq[0]; >> ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler, >> IRQF_SHARED, "exynos-pcie", pp); >> if (ret) { >> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c >> index e8663a8..aaa05c8 100644 >> --- a/drivers/pci/host/pci-imx6.c >> +++ b/drivers/pci/host/pci-imx6.c >> @@ -470,11 +470,14 @@ static int imx6_add_pcie_port(struct pcie_port *pp, >> struct platform_device *pdev) >> { >> int ret; >> + int i; >> >> - pp->irq = platform_get_irq(pdev, 0); >> - if (!pp->irq) { >> - dev_err(&pdev->dev, "failed to get irq\n"); >> - return -ENODEV; >> + for (i = 0; i < 4; i++) { >> + pp->irq[i] = platform_get_irq(pdev, i); >> + if (!pp->irq[i]) { >> + dev_err(&pdev->dev, "failed to get irq%d\n", i); >> + return -ENODEV; >> + } >> } >> >> pp->root_bus_nr = -1; >> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c >> index 17ce88f..5808177 100644 >> --- a/drivers/pci/host/pcie-designware.c >> +++ b/drivers/pci/host/pcie-designware.c >> @@ -738,8 +738,14 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys) >> static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) >> { >> struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); >> + int irq = -1; >> >> - return pp->irq; >> + if (pin > 0 && pin < 5) >> + irq = pp->irq[pin - 1]; >> + dev_info(&dev->dev, "map_irq: %04x:%04x slot%d pin%d irq%d\n", >> + dev->vendor, dev->device, slot, pin, irq); >> + >> + return irq; >> } >> >> static void dw_pcie_add_bus(struct pci_bus *bus) >> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h >> index 3063b35..5c596c0 100644 >> --- a/drivers/pci/host/pcie-designware.h >> +++ b/drivers/pci/host/pcie-designware.h >> @@ -46,7 +46,7 @@ struct pcie_port { >> struct resource io; >> struct resource mem; >> struct pcie_port_info config; >> - int irq; >> + int irq[4]; >> u32 lanes; >> struct pcie_host_ops *ops; >> int msi_irq; >> -- >> 1.8.3.2 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 4:24 ` Tim Harvey @ 2014-02-28 7:01 ` Jingoo Han 2014-02-28 10:12 ` Marek Vasut 2014-02-28 11:27 ` Lucas Stach 0 siblings, 2 replies; 29+ messages in thread From: Jingoo Han @ 2014-02-28 7:01 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 1:25 PM, Tim Harvey wrote: > On Thu, Feb 27, 2014 at 6:00 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > >> > >> The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. > >> This allows a PCIe-to-PCI bridge to function properly. > >> > >> The irq field of the pcie_host struct is expanded to 4 interrupts to > >> allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them > >> all from devicetree. I'm not clear if the Exynos driver has this > >> capability so it places the same interrupt in all 4 slots. > > > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) > > > > In the case of Exynos, > > 'INTA/B/C/D' are mapped to only one interrupt (<0 20 0>). > > Thus, the current code works properly on Exynos platform. > > > > There are three interrupts for Exynos PCIe; INTx, MSI, PHY Link, > > respectively as below. > > > > ./arch/arm/boot/dts/exynos5440.dtsi > > interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > > > <0 20 0>: PCIe RC0 pulse interrupt, > > INTA, INTB, INTC and INTD, etc > > <0 21 0>: PCIe RC0 level interrupt, > > MSI, etc > > <0 22 0>: PCIe RC0 special interrupt, > > PHY Link related interrupts, etc > > > > Of course, legacy INTx is handled as message only. > > > > Mohit, Kishon, > > How about the other SoCs? INTx is mapped to single interrupt > > such as Exynos, or separate interrupts such as i.MX6? > > Jingoo, > > Ok - so at least the I.MX6 and Exynos, which both use the designware > IP need different IRQ mappings. It seems to me then that the map_irq > should be moved out of drivers/pci/host/pcie-designware.c and into the > SoC specific host controller drivers (pci-imx.c and pci-exynos.c). If > that becomes the consensus I can submit a patch that does that. (+CC Arnd Bergmann) If you want to split dw_pcie_map_irq(), the following would be better. ./drivers/pci/host/pcie-designware.c static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) { struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); if (pp->ops->writel_rc) return pp->ops->map_irq(pp, pin); else return pp->irq; } ./drivers/pci/host/pci-imx6.c static int imx6_pcie_map_irq(struct pcie_port *pp, u8 pin) { ..... } Also, please add additional 'irq[4]' variable to 'struct imx6_pcie', instead of 'struct pcie_port'. Other SoCs does not use four separate INTx, as far as I know. Best regards, Jingoo Han > > Thanks, > > Tim > > > > > Best regards, > > Jingoo Han > > > >> > >> Signed-off-by: Tim Harvey <tharvey@gateworks.com> > >> Cc: Bjorn Helgaas <bhelgaas@google.com> > >> Cc: Richard Zhu <r65037@freescale.com> > >> Cc: Shawn Guo <shawn.guo@linaro.org> > >> Cc: Lucas Stach <l.stach@pengutronix.de> > >> Cc: Sean Cross <xobs@kosagi.com> > >> Cc: Jingoo Han <jg1.han@samsung.com> > >> --- > >> arch/arm/boot/dts/imx6qdl.dtsi | 2 +- > >> drivers/pci/host/pci-exynos.c | 7 +++++-- > >> drivers/pci/host/pci-imx6.c | 11 +++++++---- > >> drivers/pci/host/pcie-designware.c | 8 +++++++- > >> drivers/pci/host/pcie-designware.h | 2 +- > >> 5 files changed, 21 insertions(+), 9 deletions(-) > >> > >> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > >> index eed6897..fad8d90 100644 > >> --- a/arch/arm/boot/dts/imx6qdl.dtsi > >> +++ b/arch/arm/boot/dts/imx6qdl.dtsi > >> @@ -126,7 +126,7 @@ > >> 0x81000000 0 0 0x01f80000 0 0x00010000 /* downstream I/O */ > >> 0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable > >> memory */ > >> num-lanes = <1>; > >> - interrupts = <0 123 0x04>; > >> + interrupts = <0 123 0x04>, <0 122 0x04>, <0 121 0x04>, <0 120 0x04>; > >> clocks = <&clks 189>, <&clks 187>, <&clks 206>, <&clks 144>; > >> clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_gate", "pcie_axi"; > >> status = "disabled"; > >> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c > >> index 3de6bfb..d85dcb0 100644 > >> --- a/drivers/pci/host/pci-exynos.c > >> +++ b/drivers/pci/host/pci-exynos.c > >> @@ -515,11 +515,14 @@ static int add_pcie_port(struct pcie_port *pp, struct platform_device *pdev) > >> { > >> int ret; > >> > >> - pp->irq = platform_get_irq(pdev, 1); > >> - if (!pp->irq) { > >> + pp->irq[0] = platform_get_irq(pdev, 1); > >> + if (!pp->irq[0]) { > >> dev_err(&pdev->dev, "failed to get irq\n"); > >> return -ENODEV; > >> } > >> + pp->irq[1] = pp->irq[0]; > >> + pp->irq[2] = pp->irq[0]; > >> + pp->irq[3] = pp->irq[0]; > >> ret = devm_request_irq(&pdev->dev, pp->irq, exynos_pcie_irq_handler, > >> IRQF_SHARED, "exynos-pcie", pp); > >> if (ret) { > >> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > >> index e8663a8..aaa05c8 100644 > >> --- a/drivers/pci/host/pci-imx6.c > >> +++ b/drivers/pci/host/pci-imx6.c > >> @@ -470,11 +470,14 @@ static int imx6_add_pcie_port(struct pcie_port *pp, > >> struct platform_device *pdev) > >> { > >> int ret; > >> + int i; > >> > >> - pp->irq = platform_get_irq(pdev, 0); > >> - if (!pp->irq) { > >> - dev_err(&pdev->dev, "failed to get irq\n"); > >> - return -ENODEV; > >> + for (i = 0; i < 4; i++) { > >> + pp->irq[i] = platform_get_irq(pdev, i); > >> + if (!pp->irq[i]) { > >> + dev_err(&pdev->dev, "failed to get irq%d\n", i); > >> + return -ENODEV; > >> + } > >> } > >> > >> pp->root_bus_nr = -1; > >> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c > >> index 17ce88f..5808177 100644 > >> --- a/drivers/pci/host/pcie-designware.c > >> +++ b/drivers/pci/host/pcie-designware.c > >> @@ -738,8 +738,14 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys) > >> static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) > >> { > >> struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); > >> + int irq = -1; > >> > >> - return pp->irq; > >> + if (pin > 0 && pin < 5) > >> + irq = pp->irq[pin - 1]; > >> + dev_info(&dev->dev, "map_irq: %04x:%04x slot%d pin%d irq%d\n", > >> + dev->vendor, dev->device, slot, pin, irq); > >> + > >> + return irq; > >> } > >> > >> static void dw_pcie_add_bus(struct pci_bus *bus) > >> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h > >> index 3063b35..5c596c0 100644 > >> --- a/drivers/pci/host/pcie-designware.h > >> +++ b/drivers/pci/host/pcie-designware.h > >> @@ -46,7 +46,7 @@ struct pcie_port { > >> struct resource io; > >> struct resource mem; > >> struct pcie_port_info config; > >> - int irq; > >> + int irq[4]; > >> u32 lanes; > >> struct pcie_host_ops *ops; > >> int msi_irq; > >> -- > >> 1.8.3.2 > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 7:01 ` Jingoo Han @ 2014-02-28 10:12 ` Marek Vasut 2014-02-28 11:27 ` Lucas Stach 1 sibling, 0 replies; 29+ messages in thread From: Marek Vasut @ 2014-02-28 10:12 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 at 08:01:46 AM, Jingoo Han wrote: > On Friday, February 28, 2014 1:25 PM, Tim Harvey wrote: > > On Thu, Feb 27, 2014 at 6:00 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > >> The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. > > >> This allows a PCIe-to-PCI bridge to function properly. > > >> > > >> The irq field of the pcie_host struct is expanded to 4 interrupts to > > >> allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them > > >> all from devicetree. I'm not clear if the Exynos driver has this > > >> capability so it places the same interrupt in all 4 slots. > > > > > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR > > > DCG) > > > > > > In the case of Exynos, > > > 'INTA/B/C/D' are mapped to only one interrupt (<0 20 0>). > > > Thus, the current code works properly on Exynos platform. > > > > > > There are three interrupts for Exynos PCIe; INTx, MSI, PHY Link, > > > respectively as below. > > > > > > ./arch/arm/boot/dts/exynos5440.dtsi > > > > > > interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > > > > > <0 20 0>: PCIe RC0 pulse interrupt, > > > > > > INTA, INTB, INTC and INTD, etc > > > > > > <0 21 0>: PCIe RC0 level interrupt, > > > > > > MSI, etc > > > > > > <0 22 0>: PCIe RC0 special interrupt, > > > > > > PHY Link related interrupts, etc > > > > > > Of course, legacy INTx is handled as message only. > > > > > > Mohit, Kishon, > > > How about the other SoCs? INTx is mapped to single interrupt > > > such as Exynos, or separate interrupts such as i.MX6? > > > > Jingoo, > > > > Ok - so at least the I.MX6 and Exynos, which both use the designware > > IP need different IRQ mappings. It seems to me then that the map_irq > > should be moved out of drivers/pci/host/pcie-designware.c and into the > > SoC specific host controller drivers (pci-imx.c and pci-exynos.c). If > > that becomes the consensus I can submit a patch that does that. > > (+CC Arnd Bergmann) > > If you want to split dw_pcie_map_irq(), the following would be better. > > ./drivers/pci/host/pcie-designware.c > static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) > { > struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); > > if (pp->ops->writel_rc) > return pp->ops->map_irq(pp, pin); > else > return pp->irq; > } > > ./drivers/pci/host/pci-imx6.c > static int imx6_pcie_map_irq(struct pcie_port *pp, u8 pin) > { > ..... > } > > Also, please add additional 'irq[4]' variable to 'struct imx6_pcie', > instead of 'struct pcie_port'. Other SoCs does not use four separate > INTx, as far as I know. I agree with this. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 7:01 ` Jingoo Han 2014-02-28 10:12 ` Marek Vasut @ 2014-02-28 11:27 ` Lucas Stach 2014-02-28 11:37 ` Arnd Bergmann 1 sibling, 1 reply; 29+ messages in thread From: Lucas Stach @ 2014-02-28 11:27 UTC (permalink / raw) To: linux-arm-kernel Am Freitag, den 28.02.2014, 16:01 +0900 schrieb Jingoo Han: > On Friday, February 28, 2014 1:25 PM, Tim Harvey wrote: > > On Thu, Feb 27, 2014 at 6:00 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > >> > > >> The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. > > >> This allows a PCIe-to-PCI bridge to function properly. > > >> > > >> The irq field of the pcie_host struct is expanded to 4 interrupts to > > >> allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them > > >> all from devicetree. I'm not clear if the Exynos driver has this > > >> capability so it places the same interrupt in all 4 slots. > > > > > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) > > > > > > In the case of Exynos, > > > 'INTA/B/C/D' are mapped to only one interrupt (<0 20 0>). > > > Thus, the current code works properly on Exynos platform. > > > > > > There are three interrupts for Exynos PCIe; INTx, MSI, PHY Link, > > > respectively as below. > > > > > > ./arch/arm/boot/dts/exynos5440.dtsi > > > interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > > > > > <0 20 0>: PCIe RC0 pulse interrupt, > > > INTA, INTB, INTC and INTD, etc > > > <0 21 0>: PCIe RC0 level interrupt, > > > MSI, etc > > > <0 22 0>: PCIe RC0 special interrupt, > > > PHY Link related interrupts, etc > > > > > > Of course, legacy INTx is handled as message only. > > > > > > Mohit, Kishon, > > > How about the other SoCs? INTx is mapped to single interrupt > > > such as Exynos, or separate interrupts such as i.MX6? > > > > Jingoo, > > > > Ok - so at least the I.MX6 and Exynos, which both use the designware > > IP need different IRQ mappings. It seems to me then that the map_irq > > should be moved out of drivers/pci/host/pcie-designware.c and into the > > SoC specific host controller drivers (pci-imx.c and pci-exynos.c). If > > that becomes the consensus I can submit a patch that does that. > > (+CC Arnd Bergmann) > > If you want to split dw_pcie_map_irq(), the following would be better. > > ./drivers/pci/host/pcie-designware.c > static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) > { > struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata); > > if (pp->ops->writel_rc) > return pp->ops->map_irq(pp, pin); > else > return pp->irq; > } > > ./drivers/pci/host/pci-imx6.c > static int imx6_pcie_map_irq(struct pcie_port *pp, u8 pin) > { > ..... > } > > Also, please add additional 'irq[4]' variable to 'struct imx6_pcie', > instead of 'struct pcie_port'. Other SoCs does not use four separate > INTx, as far as I know. > It seems that dw_pcie_map_irq shouldn't even exist and should be replaced with of_irq_parse_and_map_pci(). As I understand the Exynos hardware is mapping all legacy PCI IRQs to one single GIC IRQ. Now I wonder why you have two different IRQs in your DT? See this snippet: interrupts = <0 20 0>, <0 21 0>, <0 22 0>; #interrupt-cells = <1>; interrupt-map-mask = <0 0 0 0>; interrupt-map = <0x0 0 &gic 53>; The driver uses the second device IRQ to map all PCI IRQs to, which is GIC IRQ 21 in this example. Your interrupt-map property in contrary indicates that all PCI IRQs should be mapped to GIC IRQ 53. What is the right thing to do here? Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 11:27 ` Lucas Stach @ 2014-02-28 11:37 ` Arnd Bergmann 2014-02-28 12:09 ` Lucas Stach 0 siblings, 1 reply; 29+ messages in thread From: Arnd Bergmann @ 2014-02-28 11:37 UTC (permalink / raw) To: linux-arm-kernel On Friday 28 February 2014 12:27:36 Lucas Stach wrote: > > It seems that dw_pcie_map_irq shouldn't even exist and should be > replaced with of_irq_parse_and_map_pci(). Right. > As I understand the Exynos hardware is mapping all legacy PCI IRQs to > one single GIC IRQ. Now I wonder why you have two different IRQs in your > DT? See this snippet: > > interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > #interrupt-cells = <1>; > interrupt-map-mask = <0 0 0 0>; > interrupt-map = <0x0 0 &gic 53>; > > The driver uses the second device IRQ to map all PCI IRQs to, which is > GIC IRQ 21 in this example. Your interrupt-map property in contrary > indicates that all PCI IRQs should be mapped to GIC IRQ 53. > What is the right thing to do here? The interrupt-map doesn't even look well-formed, since it refers to a gic that has #interrupt-cells=<3>, but only has one cell with the value. This clearly cannot work at all. It should probably be interrupt-map = <0x0 0 &gic GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; Note that '53' is the IRQ number that is commonly assigned to SPI interrupt 21, since SPI starts at number 32 on GIC. Arnd ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 11:37 ` Arnd Bergmann @ 2014-02-28 12:09 ` Lucas Stach 2014-02-28 12:27 ` Arnd Bergmann 0 siblings, 1 reply; 29+ messages in thread From: Lucas Stach @ 2014-02-28 12:09 UTC (permalink / raw) To: linux-arm-kernel Am Freitag, den 28.02.2014, 12:37 +0100 schrieb Arnd Bergmann: > On Friday 28 February 2014 12:27:36 Lucas Stach wrote: > > > > It seems that dw_pcie_map_irq shouldn't even exist and should be > > replaced with of_irq_parse_and_map_pci(). > > Right. > > > As I understand the Exynos hardware is mapping all legacy PCI IRQs to > > one single GIC IRQ. Now I wonder why you have two different IRQs in your > > DT? See this snippet: > > > > interrupts = <0 20 0>, <0 21 0>, <0 22 0>; > > #interrupt-cells = <1>; > > interrupt-map-mask = <0 0 0 0>; > > interrupt-map = <0x0 0 &gic 53>; > > > > The driver uses the second device IRQ to map all PCI IRQs to, which is > > GIC IRQ 21 in this example. Your interrupt-map property in contrary > > indicates that all PCI IRQs should be mapped to GIC IRQ 53. > > What is the right thing to do here? > > The interrupt-map doesn't even look well-formed, since it refers to > a gic that has #interrupt-cells=<3>, but only has one cell with the > value. This clearly cannot work at all. It should probably be > > interrupt-map = <0x0 0 &gic GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; > > Note that '53' is the IRQ number that is commonly assigned to > SPI interrupt 21, since SPI starts at number 32 on GIC. > Ah, I see. Thanks for the explanation. So we have a bogus Exynos DT, which will break existing systems once the kernel starts to do the right thing. This is starting to look great. ;) Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 12:09 ` Lucas Stach @ 2014-02-28 12:27 ` Arnd Bergmann 2014-02-28 13:53 ` Lucas Stach 0 siblings, 1 reply; 29+ messages in thread From: Arnd Bergmann @ 2014-02-28 12:27 UTC (permalink / raw) To: linux-arm-kernel On Friday 28 February 2014 13:09:34 Lucas Stach wrote: > Am Freitag, den 28.02.2014, 12:37 +0100 schrieb Arnd Bergmann: > > On Friday 28 February 2014 12:27:36 Lucas Stach wrote: > > The interrupt-map doesn't even look well-formed, since it refers to > > a gic that has #interrupt-cells=<3>, but only has one cell with the > > value. This clearly cannot work at all. It should probably be > > > > interrupt-map = <0x0 0 &gic GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; > > > > Note that '53' is the IRQ number that is commonly assigned to > > SPI interrupt 21, since SPI starts at number 32 on GIC. > > > Ah, I see. Thanks for the explanation. So we have a bogus Exynos DT, > which will break existing systems once the kernel starts to do the right > thing. This is starting to look great. It seems tegra and r-car have the same bug. We can probably fix it in a backwards-compatible way by changing the function to call the generic helper first and if that fails call printk_once() to emit a warning and return the hardwired IRQ number. Arnd ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 12:27 ` Arnd Bergmann @ 2014-02-28 13:53 ` Lucas Stach 0 siblings, 0 replies; 29+ messages in thread From: Lucas Stach @ 2014-02-28 13:53 UTC (permalink / raw) To: linux-arm-kernel Am Freitag, den 28.02.2014, 13:27 +0100 schrieb Arnd Bergmann: > On Friday 28 February 2014 13:09:34 Lucas Stach wrote: > > Am Freitag, den 28.02.2014, 12:37 +0100 schrieb Arnd Bergmann: > > > On Friday 28 February 2014 12:27:36 Lucas Stach wrote: > > > The interrupt-map doesn't even look well-formed, since it refers to > > > a gic that has #interrupt-cells=<3>, but only has one cell with the > > > value. This clearly cannot work at all. It should probably be > > > > > > interrupt-map = <0x0 0 &gic GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; > > > > > > Note that '53' is the IRQ number that is commonly assigned to > > > SPI interrupt 21, since SPI starts at number 32 on GIC. > > > > > Ah, I see. Thanks for the explanation. So we have a bogus Exynos DT, > > which will break existing systems once the kernel starts to do the right > > thing. This is starting to look great. > > It seems tegra and r-car have the same bug. We can probably fix it > in a backwards-compatible way by changing the function to call > the generic helper first and if that fails call printk_once() > to emit a warning and return the hardwired IRQ number. > Yes, seems like the way to go. I'll prepare some patches for that. It seems of_irq_parse_and_map_pci() already emits an error message if it can't find a valid mapping. This should be enough to get people to fix their DTs. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping 2014-02-28 1:19 ` [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping Tim Harvey 2014-02-28 2:00 ` Jingoo Han @ 2014-02-28 2:10 ` Shawn Guo 1 sibling, 0 replies; 29+ messages in thread From: Shawn Guo @ 2014-02-28 2:10 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 27, 2014 at 05:19:52PM -0800, Tim Harvey wrote: > The IMX6 maps INTA/B/C/D to ARM GIC IRQ 155/154/153/152 respectively. > This allows a PCIe-to-PCI bridge to function properly. > > The irq field of the pcie_host struct is expanded to 4 interrupts to > allow for INTA/B/C/D and the IMX6 PCIe host driver will populate them > all from devicetree. I'm not clear if the Exynos driver has this > capability so it places the same interrupt in all 4 slots. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Zhu <r65037@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Sean Cross <xobs@kosagi.com> > Cc: Jingoo Han <jg1.han@samsung.com> > --- > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- This change should be in a separate patch that goes to IMX tree. And please be aware of Arnd's comment [1] there. Shawn [1] http://www.spinics.net/lists/arm-kernel/msg311615.html ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/3] PCI: designware: add ability for custom swizzle 2014-02-28 1:19 [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping Tim Harvey 2014-02-28 1:19 ` [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping Tim Harvey @ 2014-02-28 1:19 ` Tim Harvey 2014-02-28 1:19 ` [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping Tim Harvey 2014-02-28 1:50 ` [RFC PATCH 0/3] PCI: imx6: fixup for add-in card " Jingoo Han 3 siblings, 0 replies; 29+ messages in thread From: Tim Harvey @ 2014-02-28 1:19 UTC (permalink / raw) To: linux-arm-kernel Add the ability for a platform driver to provide a host-specific swizzle function. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Zhu <r65037@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Sean Cross <xobs@kosagi.com> Cc: Jingoo Han <jg1.han@samsung.com> --- drivers/pci/host/pcie-designware.c | 3 +++ drivers/pci/host/pcie-designware.h | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 5808177..a143f2a 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -480,6 +480,9 @@ int __init dw_pcie_host_init(struct pcie_port *pp) if (pp->ops->host_init) pp->ops->host_init(pp); + if (pp->swizzle) + dw_pci.swizzle = pp->swizzle; + dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); /* program correct class for RC */ diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h index 5c596c0..8631d56 100644 --- a/drivers/pci/host/pcie-designware.h +++ b/drivers/pci/host/pcie-designware.h @@ -53,6 +53,7 @@ struct pcie_port { struct irq_domain *irq_domain; unsigned long msi_data; DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); + u8 (*swizzle)(struct pci_dev *, u8 *); }; struct pcie_host_ops { -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-02-28 1:19 [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping Tim Harvey 2014-02-28 1:19 ` [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping Tim Harvey 2014-02-28 1:19 ` [PATCH 2/3] PCI: designware: add ability for custom swizzle Tim Harvey @ 2014-02-28 1:19 ` Tim Harvey 2014-02-28 2:10 ` Jingoo Han 2014-02-28 9:27 ` Arnd Bergmann 2014-02-28 1:50 ` [RFC PATCH 0/3] PCI: imx6: fixup for add-in card " Jingoo Han 3 siblings, 2 replies; 29+ messages in thread From: Tim Harvey @ 2014-02-28 1:19 UTC (permalink / raw) To: linux-arm-kernel The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards has its slot-to-bridge IRQ mapping reversed from the PCI specification: INTA->INTD INTB->INTC INTC->INTB INTD->INTA Implement a custom swizzle function that does a fixup on the interrupt for devices on the first TI XIO2001 bridge in the tree. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Zhu <r65037@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Sean Cross <xobs@kosagi.com> Cc: Jingoo Han <jg1.han@samsung.com> --- drivers/pci/host/pci-imx6.c | 36 ++++++++++++++++++++++++++++++++++++ include/linux/pci_ids.h | 1 + 2 files changed, 37 insertions(+) diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index aaa05c8..b171a21 100644 --- a/drivers/pci/host/pci-imx6.c +++ b/drivers/pci/host/pci-imx6.c @@ -493,6 +493,39 @@ static int imx6_add_pcie_port(struct pcie_port *pp, return 0; } +/* TI XIO2001 PCIe-to-PCI bridge on GW16082 exp card has IRQs reversed */ +u8 ventana_swizzle(struct pci_dev *dev, u8 *pin) +{ + u8 i = 0; + struct pci_dev *pdev = dev; + + /* count number of TI XIO2001 bridges on bus */ + while (!pci_is_root_bus(pdev->bus)) { + if (pdev->bus && pdev->bus->self && + (pdev->bus->self->vendor == PCI_VENDOR_ID_TI) && + (pdev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { + i++; + } + pdev = pdev->bus->self; + } + while (!pci_is_root_bus(dev->bus)) { + /* if we are directly downstream from 1st TI XIO2001 bridge */ + if (dev->bus && dev->bus->self && + (dev->bus->self->vendor == PCI_VENDOR_ID_TI) && + (dev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { + if (--i == 0) { + /* swap IRQs and swizzle backwards */ + *pin = (15 - PCI_SLOT(dev->devfn)) + 1; + dev = dev->bus->self; + continue; + } + } + *pin = pci_swizzle_interrupt_pin(dev, *pin); + dev = dev->bus->self; + } + return PCI_SLOT(dev->devfn); +} + static int __init imx6_pcie_probe(struct platform_device *pdev) { struct imx6_pcie *imx6_pcie; @@ -601,6 +634,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) return PTR_ERR(imx6_pcie->iomuxc_gpr); } + if (of_machine_is_compatible("gw,ventana")) + pp->swizzle = ventana_swizzle; + ret = imx6_add_pcie_port(pp, pdev); if (ret < 0) return ret; diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 97fbecd..4ca334f 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -822,6 +822,7 @@ #define PCI_DEVICE_ID_TI_XX12 0x8039 #define PCI_DEVICE_ID_TI_XX12_FM 0x803b #define PCI_DEVICE_ID_TI_XIO2000A 0x8231 +#define PCI_DEVICE_ID_TI_XIO2001 0x8240 #define PCI_DEVICE_ID_TI_1130 0xac12 #define PCI_DEVICE_ID_TI_1031 0xac13 #define PCI_DEVICE_ID_TI_1131 0xac15 -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-02-28 1:19 ` [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping Tim Harvey @ 2014-02-28 2:10 ` Jingoo Han 2014-02-28 9:27 ` Arnd Bergmann 1 sibling, 0 replies; 29+ messages in thread From: Jingoo Han @ 2014-02-28 2:10 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards > has its slot-to-bridge IRQ mapping reversed from the PCI specification: > > INTA->INTD > INTB->INTC > INTC->INTB > INTD->INTA (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) As I mention earlier, 'i.MX6 PCIe IP' is not a culprit. The 'TI XIO2001 PCIe-to-PCI bridge' is the culprit, right? Then, './drivers/pci/host/pci-imx6.c' is not a good place to fix the board problems. Look at other i.MX6 boards. They don't have the 'TI XIO2001 PCIe-to-PCI bridge'. Please don't add this board specific code to SoC specific driver code. Best regards, Jingoo Han > > Implement a custom swizzle function that does a fixup on the interrupt for > devices on the first TI XIO2001 bridge in the tree. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Zhu <r65037@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Sean Cross <xobs@kosagi.com> > Cc: Jingoo Han <jg1.han@samsung.com> > --- > drivers/pci/host/pci-imx6.c | 36 ++++++++++++++++++++++++++++++++++++ > include/linux/pci_ids.h | 1 + > 2 files changed, 37 insertions(+) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index aaa05c8..b171a21 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -493,6 +493,39 @@ static int imx6_add_pcie_port(struct pcie_port *pp, > return 0; > } > > +/* TI XIO2001 PCIe-to-PCI bridge on GW16082 exp card has IRQs reversed */ > +u8 ventana_swizzle(struct pci_dev *dev, u8 *pin) > +{ > + u8 i = 0; > + struct pci_dev *pdev = dev; > + > + /* count number of TI XIO2001 bridges on bus */ > + while (!pci_is_root_bus(pdev->bus)) { > + if (pdev->bus && pdev->bus->self && > + (pdev->bus->self->vendor == PCI_VENDOR_ID_TI) && > + (pdev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { > + i++; > + } > + pdev = pdev->bus->self; > + } > + while (!pci_is_root_bus(dev->bus)) { > + /* if we are directly downstream from 1st TI XIO2001 bridge */ > + if (dev->bus && dev->bus->self && > + (dev->bus->self->vendor == PCI_VENDOR_ID_TI) && > + (dev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { > + if (--i == 0) { > + /* swap IRQs and swizzle backwards */ > + *pin = (15 - PCI_SLOT(dev->devfn)) + 1; > + dev = dev->bus->self; > + continue; > + } > + } > + *pin = pci_swizzle_interrupt_pin(dev, *pin); > + dev = dev->bus->self; > + } > + return PCI_SLOT(dev->devfn); > +} > + > static int __init imx6_pcie_probe(struct platform_device *pdev) > { > struct imx6_pcie *imx6_pcie; > @@ -601,6 +634,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) > return PTR_ERR(imx6_pcie->iomuxc_gpr); > } > > + if (of_machine_is_compatible("gw,ventana")) > + pp->swizzle = ventana_swizzle; > + > ret = imx6_add_pcie_port(pp, pdev); > if (ret < 0) > return ret; > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 97fbecd..4ca334f 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -822,6 +822,7 @@ > #define PCI_DEVICE_ID_TI_XX12 0x8039 > #define PCI_DEVICE_ID_TI_XX12_FM 0x803b > #define PCI_DEVICE_ID_TI_XIO2000A 0x8231 > +#define PCI_DEVICE_ID_TI_XIO2001 0x8240 > #define PCI_DEVICE_ID_TI_1130 0xac12 > #define PCI_DEVICE_ID_TI_1031 0xac13 > #define PCI_DEVICE_ID_TI_1131 0xac15 > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-02-28 1:19 ` [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping Tim Harvey 2014-02-28 2:10 ` Jingoo Han @ 2014-02-28 9:27 ` Arnd Bergmann 2014-02-28 17:39 ` Jason Gunthorpe 1 sibling, 1 reply; 29+ messages in thread From: Arnd Bergmann @ 2014-02-28 9:27 UTC (permalink / raw) To: linux-arm-kernel On Thursday 27 February 2014 17:19:54 Tim Harvey wrote: > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards > has its slot-to-bridge IRQ mapping reversed from the PCI specification: > > INTA->INTD > INTB->INTC > INTC->INTB > INTD->INTA > > Implement a custom swizzle function that does a fixup on the interrupt for > devices on the first TI XIO2001 bridge in the tree. > I'm pretty sure you can express that by using a more specific 'interrupt-map' property that defines the correct mapping for the PCIe-to-PCI bridge in the board file. Arnd ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-02-28 9:27 ` Arnd Bergmann @ 2014-02-28 17:39 ` Jason Gunthorpe 2014-03-01 0:52 ` Tim Harvey 0 siblings, 1 reply; 29+ messages in thread From: Jason Gunthorpe @ 2014-02-28 17:39 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 28, 2014 at 10:27:13AM +0100, Arnd Bergmann wrote: > On Thursday 27 February 2014 17:19:54 Tim Harvey wrote: > > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards > > has its slot-to-bridge IRQ mapping reversed from the PCI specification: > > > > INTA->INTD > > INTB->INTC > > INTC->INTB > > INTD->INTA > > > > Implement a custom swizzle function that does a fixup on the interrupt for > > devices on the first TI XIO2001 bridge in the tree. > > > > I'm pretty sure you can express that by using a more specific > 'interrupt-map' property that defines the correct mapping for > the PCIe-to-PCI bridge in the board file. Yes, the correct way to handle this is to define a stanza for the PCIe-PCI bridge and then place an interrupt map in that stanza that describes the downstream translation. Nobody should be messing with IRQ numbers in code on DT platforms. DTs using the mvebu driver have an example of this: pcie-controller { compatible = "marvell,kirkwood-pcie"; status = "disabled"; device_type = "pci"; pcie at 1,0 { device_type = "pci"; reg = <0x0800 0 0 0 0>; ^^^^^^^ This is the PCI device ID of the bridge interrupt-map-mask = <0 0 0 0>; interrupt-map = <0 0 0 0 &intc 9>; Which says 'INTA/B/C/D of devices downstream of bridge 00:01.0 map to &intc input 9' In particular, this is probably not a TI XIO2001 problem, but a board design problem - the swizzle rules were not properly followed when wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. Jason ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-02-28 17:39 ` Jason Gunthorpe @ 2014-03-01 0:52 ` Tim Harvey 2014-03-01 1:22 ` Jason Gunthorpe 0 siblings, 1 reply; 29+ messages in thread From: Tim Harvey @ 2014-03-01 0:52 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 28, 2014 at 9:39 AM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Fri, Feb 28, 2014 at 10:27:13AM +0100, Arnd Bergmann wrote: >> On Thursday 27 February 2014 17:19:54 Tim Harvey wrote: >> > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards >> > has its slot-to-bridge IRQ mapping reversed from the PCI specification: >> > >> > INTA->INTD >> > INTB->INTC >> > INTC->INTB >> > INTD->INTA >> > >> > Implement a custom swizzle function that does a fixup on the interrupt for >> > devices on the first TI XIO2001 bridge in the tree. >> > >> >> I'm pretty sure you can express that by using a more specific >> 'interrupt-map' property that defines the correct mapping for >> the PCIe-to-PCI bridge in the board file. > > Yes, the correct way to handle this is to define a stanza for the > PCIe-PCI bridge and then place an interrupt map in that stanza that > describes the downstream translation. > > Nobody should be messing with IRQ numbers in code on DT platforms. > > DTs using the mvebu driver have an example of this: > > pcie-controller { > compatible = "marvell,kirkwood-pcie"; > status = "disabled"; > device_type = "pci"; > > pcie at 1,0 { > device_type = "pci"; > reg = <0x0800 0 0 0 0>; > ^^^^^^^ This is the PCI device > ID of the bridge > interrupt-map-mask = <0 0 0 0>; > interrupt-map = <0 0 0 0 &intc 9>; > > Which says 'INTA/B/C/D of devices downstream of bridge 00:01.0 map to > &intc input 9' > Jason, In our case, the IMX6 is on a basebaord and we have optional expansion boards. One such board has a PEX 8609 9-port PCIe switch, the other has a TI XIO2001 PCIe-to-PCI bridge. So, depending on how you connect these expansion boards the bus numbers change. For example, one use case of the baseboard+XIO2001-expansion is: IMX6 -> XIO2001 PCIe-to-PCI bridge: The following shows one endpoint off the bridge (bus2 slot12): $ lspci -n 00:00.0 0604: 16c3:abcd (rev 01) 01:00.0 0604: 104c:8240 02:0c.0 0280: 168c:0027 (rev 01) $ lspci -tnv -[0000:00]---00.0-[01-02]----00.0-[02]----0c.0 168c:0027 Another use case of the same baseboard + PEX8609-expansion + XIO2001-expansion is: $ lspci -n 00:00.0 0604: 16c3:abcd (rev 01) 01:00.0 0604: 10b5:8609 (rev ba) 01:00.1 0880: 10b5:8609 (rev ba) 02:01.0 0604: 10b5:8609 (rev ba) 02:04.0 0604: 10b5:8609 (rev ba) 02:05.0 0604: 10b5:8609 (rev ba) 02:06.0 0604: 10b5:8609 (rev ba) 02:07.0 0604: 10b5:8609 (rev ba) 02:08.0 0604: 10b5:8609 (rev ba) 02:09.0 0604: 10b5:8609 (rev ba) 04:00.0 0604: 104c:8240 05:0c.0 0280: 168c:0027 (rev 01) $ lspci -tnv -[0000:00]---00.0-[01-0a]--+-00.0-[02-0a]--+-01.0-[03]-- | +-04.0-[04-05]----00.0-[05]----0c.0 168c:0027 | +-05.0-[06]-- | +-06.0-[07]-- | +-07.0-[08]-- | +-08.0-[09]-- | \-09.0-[0a]-- \-00.1 10b5:8609 > In particular, this is probably not a TI XIO2001 problem, but a board > design problem - the swizzle rules were not properly followed when > wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. Correct - its not a TI XIO2001 problem, its that the interrupts from the slots to the XIO2001 don't follow the interrupt mapping called out in the spec correctly (board problem on the 'add-in' board, not the 'baseboard'). Regardless of the issue of not knowing the bus topology before-hand, I'm still trying to understand how to describe the bridge in devicetree using your example above. If I were able to define a DT node for the XIO2001 bridge and apply an interrupt-map there, how does that map get encorporated into the swizzle in the case the the bridge is in the middle of the chain of devices? In looking at the of_irq_parse_map_pci() function that will likely be called from map_irq() it would end up calling of_irq_parse_raw to map the irq and I'm not understanding how that will take into account the fact that a bridge possibly in the middle of the bus-tree had invalid mapping. Thanks, Tim > > Jason ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-03-01 0:52 ` Tim Harvey @ 2014-03-01 1:22 ` Jason Gunthorpe 2014-03-03 19:59 ` Tim Harvey 0 siblings, 1 reply; 29+ messages in thread From: Jason Gunthorpe @ 2014-03-01 1:22 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 28, 2014 at 04:52:05PM -0800, Tim Harvey wrote: > > In particular, this is probably not a TI XIO2001 problem, but a board > > design problem - the swizzle rules were not properly followed when > > wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. > > Correct - its not a TI XIO2001 problem, its that the interrupts from > the slots to the XIO2001 don't follow the interrupt mapping called out > in the spec correctly (board problem on the 'add-in' board, not the > 'baseboard'). So broken hardware requires explicit DT representation, the auto-probing mechanism only work for compliant hardware. > Regardless of the issue of not knowing the bus topology before-hand, To solve this problem, you reall need to know the bus topology before hand, so any systems that include a borken TI XIO2001 board need a special DT. Including stuff like this in code means you hobble every one who might use the TI chip correctly. It is certainly inappropriate to include a host driver, or generic PCI fixup. > I'm still trying to understand how to describe the bridge in > devicetree using your example above. If I were able to define a DT > node for the XIO2001 bridge and apply an interrupt-map there, how does > that map get encorporated into the swizzle in the case the the bridge > is in the middle of the chain of devices? You keep nesting the PCI-PCI bridges until you get to the bottom, basically following along the lspci -t output, cast into DT notation: Your first example: pcie-controller { compatible = "marvell,kirkwood-pcie"; status = "disabled"; device_type = "pci"; pcie at 0,0 { device_type = "pci"; // Presumably this is the root port bridge? // 00:00.0 0604: 16c3:abcd reg = <PCI_ID(0,0,0) 0 0 0 0>; pcie at 0,0 { // This is the broken TI board: // 01:00.0 0604: 104c:8240 reg = <PCI_ID(1,0,0) 0 0 0 0>; Second example: pcie-controller { compatible = "marvell,kirkwood-pcie"; status = "disabled"; device_type = "pci"; pcie at 0,0 { device_type = "pci"; // Presumably this is the root port bridge? // 00:00.0 0604: 16c3:abcd reg = <PCI_ID(0,0,0) 0 0 0 0>; pcie at 0,0 { // This is the PEX switch bridge to internal // 01:00.0 0604: 10b5:8609 (rev ba) reg = <PCI_ID(1,0,0) 0 0 0 0>; pcie at 4,0 { // This is the PEX port bridge to // To the TI part // 02:04.0 0604: 10b5:8609 (rev ba) reg = <PCI_ID(2,4,0) 0 0 0 0>; pcie at 0,0 { // This is the broken TI board: // 04:00.0 0604: 104c:8240 reg = <PCI_ID(4,0,0) 0 0 0 0>; Also, bear in mind that every single explicitly declared stanza requires a correct interrupt-map. > of_irq_parse_map_pci() function that will likely be called from > map_irq() it would end up calling of_irq_parse_raw to map the irq and > I'm not understanding how that will take into account the fact that a > bridge possibly in the middle of the bus-tree had invalid mapping. First the PCI core matches the DT nodes to the discovered topology. Then the interrupt mapper starts from a probed PCI node and traces up the tree to the root. Each time it goes up it swizzles. When it finds a node with a DT mapping it immediately switches to DT to resolve the interrupt, which uses the first interrupt map found by traversing up from the match'd DT node. Once it switches to DT mode there is no swizzling, the DT must exactly describe the interconnection. Jason ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-03-01 1:22 ` Jason Gunthorpe @ 2014-03-03 19:59 ` Tim Harvey 2014-03-03 23:37 ` Jason Gunthorpe 0 siblings, 1 reply; 29+ messages in thread From: Tim Harvey @ 2014-03-03 19:59 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 28, 2014 at 5:22 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Fri, Feb 28, 2014 at 04:52:05PM -0800, Tim Harvey wrote: > >> > In particular, this is probably not a TI XIO2001 problem, but a board >> > design problem - the swizzle rules were not properly followed when >> > wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. >> >> Correct - its not a TI XIO2001 problem, its that the interrupts from >> the slots to the XIO2001 don't follow the interrupt mapping called out >> in the spec correctly (board problem on the 'add-in' board, not the >> 'baseboard'). > > So broken hardware requires explicit DT representation, the > auto-probing mechanism only work for compliant hardware. ok - makes sense > >> Regardless of the issue of not knowing the bus topology before-hand, > > To solve this problem, you reall need to know the bus topology before > hand, so any systems that include a borken TI XIO2001 board need a > special DT. I suppose I could go about this from the bootloader as well. The bootloader could enumerate the bus and build the DT with the nodes required to handle the broken TI XIO2001 add-in cards. > > Including stuff like this in code means you hobble every one who might > use the TI chip correctly. It is certainly inappropriate to include a > host driver, or generic PCI fixup. Understood - trust me... I'm not happy about this situation. A pci fixup for the XIO2001 could at least be placed in arch/arm/mach-imx/mach-imx6q.c with a check for of_machine_is_compatible("gw, ventana"). This is currently done in order to handle GPIO on the PEX890x bridge which is used as PERST# for downstream slots (see http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). This of course, assumes that I can come up with a way for a pci fixup to replace the swizzle function of the host controller before enumeration. > >> I'm still trying to understand how to describe the bridge in >> devicetree using your example above. If I were able to define a DT >> node for the XIO2001 bridge and apply an interrupt-map there, how does >> that map get encorporated into the swizzle in the case the the bridge >> is in the middle of the chain of devices? > > You keep nesting the PCI-PCI bridges until you get to the bottom, > basically following along the lspci -t output, cast into DT notation: > > Your first example: > pcie-controller { > compatible = "marvell,kirkwood-pcie"; > status = "disabled"; > device_type = "pci"; > > pcie at 0,0 { > device_type = "pci"; > // Presumably this is the root port bridge? > // 00:00.0 0604: 16c3:abcd > reg = <PCI_ID(0,0,0) 0 0 0 0>; I'm still not understanding what needs to go in the 'reg' property for a DT node representing a PCI dev (what you are using PCI_ID() for). I've looked at the PCI Bus Binding to Open Firmware doc and still don't understand (its very terse, and lacks examples). By looking through the drivers/of code it seemed that this should be devfn but after some digging through examples in Documentation/devicetree/bindings/pci it seems that it really needs to be devfn<<8. I'm not clear why the <<8 is needed. I also determined that for fsl.imx6q-pcie I needed to define #address-cells = <3> and #size-cells = <2> as the enw pcie dev nodes kept defaulting to something else - I didn't realize those were not inherited from the parent DT node. In all, your examples here were extremely helpful to me. I now know how to properly describe my GigE PCI dev node so that the driver can have a hope of getting its MAC from DT. Thank you! > pcie at 0,0 { > // This is the broken TI board: > // 01:00.0 0604: 104c:8240 > reg = <PCI_ID(1,0,0) 0 0 0 0>; > <snip> > > Also, bear in mind that every single explicitly declared stanza requires > a correct interrupt-map. if not specified it wouldn't default to standard bridge mapping such that I only need to provide interrupt-map for the node with the the TI bridge? > >> of_irq_parse_map_pci() function that will likely be called from >> map_irq() it would end up calling of_irq_parse_raw to map the irq and >> I'm not understanding how that will take into account the fact that a >> bridge possibly in the middle of the bus-tree had invalid mapping. > > First the PCI core matches the DT nodes to the discovered topology. > > Then the interrupt mapper starts from a probed PCI node and traces up > the tree to the root. Each time it goes up it swizzles. > > When it finds a node with a DT mapping it immediately switches to DT > to resolve the interrupt, which uses the first interrupt map found by > traversing up from the match'd DT node. Once it switches to DT mode > there is no swizzling, the DT must exactly describe the > interconnection. Ok - so this assumes that the host controller driver _is_ calling something like of_irq_parse_and_map_pci() instead of returning based on some static mapping (like pp->irq) right? What confuses me is that swizzling is done to the pin before the call to map_irq(dev, slot, pin). It looks like that this common swizzling is ignored when using of_irq_parse_and_map_pci() which instead would perform what you describe above correct? Thanks for all the help here! Tim > > Jason ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-03-03 19:59 ` Tim Harvey @ 2014-03-03 23:37 ` Jason Gunthorpe 2014-03-04 0:38 ` Tim Harvey 0 siblings, 1 reply; 29+ messages in thread From: Jason Gunthorpe @ 2014-03-03 23:37 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 03, 2014 at 11:59:51AM -0800, Tim Harvey wrote: > A pci fixup for the XIO2001 could at least be placed in > arch/arm/mach-imx/mach-imx6q.c with a check for > of_machine_is_compatible("gw, ventana"). This is currently done in > order to handle GPIO on the PEX890x bridge which is used as PERST# for > downstream slots (see > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). That is an odd looking thing .. PERST# of a subordinate port should be tied to the PCI_BRIDGE_CTL_BUS_RESET bit in the port's bridge configuration space - is more configuration of the PLX chip required? Also, be aware that PCI specifies a minimum time after reset-deassertion before accessing the bus, and I don't see a sleep... > I'm still not understanding what needs to go in the 'reg' property for > a DT node representing a PCI dev (what you are using PCI_ID() for). It is a 3 DW address in this format: phys.hi cell: npt000ss bbbbbbbb dddddfff rrrrrrrr phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh phys.low cell: llllllll llllllll llllllll llllllll And in this context you use 'ss' = 0 for configuration: * npt000ss bbbbbbbb dddddfff rrrrrrrr = 00000000 00000000 dddddfff 00000000 So devfn << 8 sounds right to me. I *think* you can use 0 for the bus in this context. > #address-cells = <3> and #size-cells = <2> as the enw pcie dev nodes > kept defaulting to something else - I didn't realize those were not > inherited from the parent DT node. Right, sorry, I was making it terse.. > In all, your examples here were extremely helpful to me. I now know > how to properly describe my GigE PCI dev node so that the driver can > have a hope of getting its MAC from DT. Thank you! Right! > > Also, bear in mind that every single explicitly declared stanza requires > > a correct interrupt-map. > > if not specified it wouldn't default to standard bridge mapping such > that I only need to provide interrupt-map for the node with the the TI > bridge? Nope, there are two distinct mechanisms - finding the closes DT node to the actual PCI device (of_irq_parse_pci) - this swizzles as it searches. The rational here is to support hot pluggable devices that were not discovered during initialization time. The second is the completely generic interrupt-map mechanism (of_irq_parse_raw), which takes the interrupt specification from the first phase and transates it into an actual interrupt. This isn't PCI aware and doesn't change the description in any way. So if you specify a node, you have to specify an interrupt-map that covers that node. I recommend placing the map in the node itself so the bus number is not required. > Ok - so this assumes that the host controller driver _is_ calling > something like of_irq_parse_and_map_pci() instead of returning based > on some static mapping (like pp->irq) right? Yes, absoultely. All DT based PCI drivers need to use this mechanism to get the expected interrupt-map behavior. > What confuses me is that swizzling is done to the pin before the call > to map_irq(dev, slot, pin). It looks like that this common swizzling > is ignored when using of_irq_parse_and_map_pci() which instead would > perform what you describe above correct? Right, the pin # from the PCI core isn't used in the OF code: * of_irq_parse_and_map_pci() - Decode a PCI irq from the device tree and map to a virq * @slot and @pin are unused, but included in the function so that this * function can be used directly as the map_irq callback to pci_fixup_irqs(). It re-reads the device's pin number directly and feeds that through the standard swizzle and OF's UIS system. Jason ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-03-03 23:37 ` Jason Gunthorpe @ 2014-03-04 0:38 ` Tim Harvey 2014-03-04 1:01 ` Jason Gunthorpe 0 siblings, 1 reply; 29+ messages in thread From: Tim Harvey @ 2014-03-04 0:38 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 3, 2014 at 3:37 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Mon, Mar 03, 2014 at 11:59:51AM -0800, Tim Harvey wrote: > >> A pci fixup for the XIO2001 could at least be placed in >> arch/arm/mach-imx/mach-imx6q.c with a check for >> of_machine_is_compatible("gw, ventana"). This is currently done in >> order to handle GPIO on the PEX890x bridge which is used as PERST# for >> downstream slots (see >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). > > That is an odd looking thing .. PERST# of a subordinate port should be > tied to the PCI_BRIDGE_CTL_BUS_RESET bit in the port's bridge > configuration space - is more configuration of the PLX chip required? PCI_BRIDGE_CTL_BUS_RESET does cause a hot reset on the corresponding downstream link however the minPCIe card slot has a PERST# pin that is for a functional reset to the card. The GPIO's I mention on the PLX bridge are routed to that signal for the various slots. Regardless of what a device may do when it gets a hot-reset some devices still may require a hard reset. Think of MiniPCIe form-factor USB cellular modems for example which don't even use the PCIe bus. > > Also, be aware that PCI specifies a minimum time after > reset-deassertion before accessing the bus, and I don't see a sleep... there is an msleep(100) there after the de-assertion. > >> I'm still not understanding what needs to go in the 'reg' property for >> a DT node representing a PCI dev (what you are using PCI_ID() for). > > It is a 3 DW address in this format: > > phys.hi cell: npt000ss bbbbbbbb dddddfff rrrrrrrr > phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh > phys.low cell: llllllll llllllll llllllll llllllll > > And in this context you use 'ss' = 0 for configuration: > > * npt000ss bbbbbbbb dddddfff rrrrrrrr > = 00000000 00000000 dddddfff 00000000 > > So devfn << 8 sounds right to me. > > I *think* you can use 0 for the bus in this context. yes, it would appear bus can be 0 from my findings. Thanks for clarifying that verbiage! > >> #address-cells = <3> and #size-cells = <2> as the enw pcie dev nodes >> kept defaulting to something else - I didn't realize those were not >> inherited from the parent DT node. > > Right, sorry, I was making it terse.. > >> In all, your examples here were extremely helpful to me. I now know >> how to properly describe my GigE PCI dev node so that the driver can >> have a hope of getting its MAC from DT. Thank you! > > Right! > >> > Also, bear in mind that every single explicitly declared stanza requires >> > a correct interrupt-map. >> >> if not specified it wouldn't default to standard bridge mapping such >> that I only need to provide interrupt-map for the node with the the TI >> bridge? > > Nope, there are two distinct mechanisms - finding the closes DT node > to the actual PCI device (of_irq_parse_pci) - this swizzles as it > searches. The rational here is to support hot pluggable devices that > were not discovered during initialization time. > > The second is the completely generic interrupt-map mechanism > (of_irq_parse_raw), which takes the interrupt specification from the > first phase and transates it into an actual interrupt. This isn't PCI > aware and doesn't change the description in any way. > > So if you specify a node, you have to specify an interrupt-map that > covers that node. I recommend placing the map in the node itself so > the bus number is not required. > >> Ok - so this assumes that the host controller driver _is_ calling >> something like of_irq_parse_and_map_pci() instead of returning based >> on some static mapping (like pp->irq) right? > > Yes, absoultely. All DT based PCI drivers need to use this mechanism > to get the expected interrupt-map behavior. > >> What confuses me is that swizzling is done to the pin before the call >> to map_irq(dev, slot, pin). It looks like that this common swizzling >> is ignored when using of_irq_parse_and_map_pci() which instead would >> perform what you describe above correct? > > Right, the pin # from the PCI core isn't used in the OF code: > > * of_irq_parse_and_map_pci() - Decode a PCI irq from the device tree and map to a virq > * @slot and @pin are unused, but included in the function so that this > * function can be used directly as the map_irq callback to pci_fixup_irqs(). > > It re-reads the device's pin number directly and feeds that through > the standard swizzle and OF's UIS system. > > Jason Ok - that all makes sense and hopefully when I get the legacy PCI interrupt mapping via DT working for IMX6 I can prove it out. What did you think of my two ideas above to resolve this issue on our add-in card where the bus-topology isn't known ahead of time: 1. bootloader builds out DT with proper interrupt-map's to handle the mis-map after enumerating the bus to understand the toplogy. or 2. a pci fixup for xio2001 that further qualifies the DT machine and the 'depth' of the xio2001 (ie make sure its the first xio2001 bridge on the bus) that replaces the swizzle function. This would be in arch/arm/mach-imx/mach-imx6q.c Thanks, Tim ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping 2014-03-04 0:38 ` Tim Harvey @ 2014-03-04 1:01 ` Jason Gunthorpe 0 siblings, 0 replies; 29+ messages in thread From: Jason Gunthorpe @ 2014-03-04 1:01 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 03, 2014 at 04:38:10PM -0800, Tim Harvey wrote: > On Mon, Mar 3, 2014 at 3:37 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > On Mon, Mar 03, 2014 at 11:59:51AM -0800, Tim Harvey wrote: > > > >> A pci fixup for the XIO2001 could at least be placed in > >> arch/arm/mach-imx/mach-imx6q.c with a check for > >> of_machine_is_compatible("gw, ventana"). This is currently done in > >> order to handle GPIO on the PEX890x bridge which is used as PERST# for > >> downstream slots (see > >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). > > > > That is an odd looking thing .. PERST# of a subordinate port should be > > tied to the PCI_BRIDGE_CTL_BUS_RESET bit in the port's bridge > > configuration space - is more configuration of the PLX chip required? > > PCI_BRIDGE_CTL_BUS_RESET does cause a hot reset on the corresponding > downstream link however the minPCIe card slot has a PERST# pin that is > for a functional reset to the card. Ah, I'm getting my PCI versions mixed up, that all sounds right to me then.. > > Also, be aware that PCI specifies a minimum time after > > reset-deassertion before accessing the bus, and I don't see a sleep... > > there is an msleep(100) there after the de-assertion. .. and I'm blind, looks good. > Ok - that all makes sense and hopefully when I get the legacy PCI > interrupt mapping via DT working for IMX6 I can prove it out. > > What did you think of my two ideas above to resolve this issue on our > add-in card where the bus-topology isn't known ahead of time: > > 1. bootloader builds out DT with proper interrupt-map's to handle the > mis-map after enumerating the bus to understand the toplogy. This is how the OF stuff for PCI has historically been used, so it is a nice OS-agnostic way to implement this work around, particularly if you have some kind of daughter card specific information you can rely on (eg a SPD that could indicate the problematic PCB revision) If you do that you could also hoist the required fixup for your switch into firmware too. > 2. a pci fixup for xio2001 that further qualifies the DT machine and > the 'depth' of the xio2001 (ie make sure its the first xio2001 bridge > on the bus) that replaces the swizzle function. This would be in > arch/arm/mach-imx/mach-imx6q.c You can probably make this work, but given there is already a DT based fix I'm not sure if people will like a .c version, especially if it requires more single-use 'general purpose' interfaces from the core code... Jason ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping 2014-02-28 1:19 [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping Tim Harvey ` (2 preceding siblings ...) 2014-02-28 1:19 ` [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping Tim Harvey @ 2014-02-28 1:50 ` Jingoo Han 2014-02-28 4:16 ` Tim Harvey 3 siblings, 1 reply; 29+ messages in thread From: Jingoo Han @ 2014-02-28 1:50 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > An add-in card used on the Ventana IMX6 SoC based family of boards has a > TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings between the > bridge and the four mini-PCI slots are swapped (INTD/C/B/A). (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) This problem happens from the 'Board', not a 'SoC'. 'TI XIO2001 PCIe-to-PCI bridge' is not a 'SoC'. 'pci-imx6.c' is the driver for 'IMX6 PCI IP', not for 'IMX6 SoC based board'. Isn't it? Then, the workaround code for board problem should NOT be included to './drivers/pci/host/' side. Also, please add the following members to CC list. They will give important comments. : Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG Best regards, Jingoo Han > > This series adds legacy PCI interrupt mapping to support PCI bridges on > IMX6 PCIe host controllers, adds the ability to use a host-specific > custom swizzle function, then uses that to fix the IRQ mapping. > > I'm hoping this is the right way to perform such a workaround - please provide > feedback if not. Patch1 will likely collide with the owrk Lucas is doing > with device-tree bindings and MSI support. I'm happy to re-base if/when > needed. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Zhu <r65037@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Sean Cross <xobs@kosagi.com> > Cc: Jingoo Han <jg1.han@samsung.com> > --- > Tim Harvey (3): > PCI: designware: add legacy PCI interrupt mapping > PCI: designware: add ability for custom swizzle > PCI: imx6: ventana: fixup for IRQ mismapping > > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- > drivers/pci/host/pci-exynos.c | 7 ++++-- > drivers/pci/host/pci-imx6.c | 47 ++++++++++++++++++++++++++++++++++---- > drivers/pci/host/pcie-designware.c | 11 ++++++++- > drivers/pci/host/pcie-designware.h | 3 ++- > include/linux/pci_ids.h | 1 + > 6 files changed, 62 insertions(+), 9 deletions(-) > > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping 2014-02-28 1:50 ` [RFC PATCH 0/3] PCI: imx6: fixup for add-in card " Jingoo Han @ 2014-02-28 4:16 ` Tim Harvey 2014-02-28 6:22 ` Jingoo Han 0 siblings, 1 reply; 29+ messages in thread From: Tim Harvey @ 2014-02-28 4:16 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 27, 2014 at 5:50 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > > > An add-in card used on the Ventana IMX6 SoC based family of boards has a > > TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings between the > > bridge and the four mini-PCI slots are swapped (INTD/C/B/A). > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) > > This problem happens from the 'Board', not a 'SoC'. > 'TI XIO2001 PCIe-to-PCI bridge' is not a 'SoC'. > 'pci-imx6.c' is the driver for 'IMX6 PCI IP', not for 'IMX6 SoC based > board'. Isn't it? Jingoo, Correct, this is an issue in the way the XIO2001 was hooked up to the PCI slots, so it should be viewed as a board issue. > > Then, the workaround code for board problem should NOT be > included to './drivers/pci/host/' side. I would agree, but to overcome this sort of interrupt mapping issue one would need to either implement a custom swizzle or perhaps a custom map_irq and both of those are hooked into the pcie driver core. Do you have any suggestions on where/how I would better hook into those? > > Also, please add the following members to CC list. They will give > important comments. > > : Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG > > Best regards, > Jingoo Han Thanks, Tim > > > > > This series adds legacy PCI interrupt mapping to support PCI bridges on > > IMX6 PCIe host controllers, adds the ability to use a host-specific > > custom swizzle function, then uses that to fix the IRQ mapping. > > > > I'm hoping this is the right way to perform such a workaround - please provide > > feedback if not. Patch1 will likely collide with the owrk Lucas is doing > > with device-tree bindings and MSI support. I'm happy to re-base if/when > > needed. > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Richard Zhu <r65037@freescale.com> > > Cc: Shawn Guo <shawn.guo@linaro.org> > > Cc: Lucas Stach <l.stach@pengutronix.de> > > Cc: Sean Cross <xobs@kosagi.com> > > Cc: Jingoo Han <jg1.han@samsung.com> > > --- > > Tim Harvey (3): > > PCI: designware: add legacy PCI interrupt mapping > > PCI: designware: add ability for custom swizzle > > PCI: imx6: ventana: fixup for IRQ mismapping > > > > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- > > drivers/pci/host/pci-exynos.c | 7 ++++-- > > drivers/pci/host/pci-imx6.c | 47 ++++++++++++++++++++++++++++++++++---- > > drivers/pci/host/pcie-designware.c | 11 ++++++++- > > drivers/pci/host/pcie-designware.h | 3 ++- > > include/linux/pci_ids.h | 1 + > > 6 files changed, 62 insertions(+), 9 deletions(-) > > > > -- > > 1.8.3.2 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping 2014-02-28 4:16 ` Tim Harvey @ 2014-02-28 6:22 ` Jingoo Han 2014-02-28 10:15 ` Marek Vasut 2014-02-28 16:52 ` Tim Harvey 0 siblings, 2 replies; 29+ messages in thread From: Jingoo Han @ 2014-02-28 6:22 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 1:16 PM, Tim Harvey wrote: > On Thu, Feb 27, 2014 at 5:50 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > > > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > > > > > An add-in card used on the Ventana IMX6 SoC based family of boards has a > > > TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings between the > > > bridge and the four mini-PCI slots are swapped (INTD/C/B/A). > > > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) > > > > This problem happens from the 'Board', not a 'SoC'. > > 'TI XIO2001 PCIe-to-PCI bridge' is not a 'SoC'. > > 'pci-imx6.c' is the driver for 'IMX6 PCI IP', not for 'IMX6 SoC based > > board'. Isn't it? > > Jingoo, > > Correct, this is an issue in the way the XIO2001 was hooked up to the > PCI slots, so it should be viewed as a board issue. (+CC Arnd Bergmann) Then, this board fixup code should NOT be placed in './drivers/pci/host/' side. > > > > > Then, the workaround code for board problem should NOT be > > included to './drivers/pci/host/' side. > > I would agree, but to overcome this sort of interrupt mapping issue > one would need to either implement a custom swizzle or perhaps a > custom map_irq and both of those are hooked into the pcie driver core. > Do you have any suggestions on where/how I would better hook into > those? Anyway, 'TI XIO2001 PCIe-to-PCI bridge' chip on the board is the culprit. So, the board specific side is a good place. For instance, ./arch/arm/mach-imx/ I don't know how to handle this problem. But, there is no reason that 'pcie-designware.c' should take a care of the board specific issue. Arnd Bergmann, Would you give your opinions? Best regards, Jingoo Han > > > > > Also, please add the following members to CC list. They will give > > important comments. > > > > : Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG > > > > Best regards, > > Jingoo Han > > Thanks, > > Tim > > > > > > > > > This series adds legacy PCI interrupt mapping to support PCI bridges on > > > IMX6 PCIe host controllers, adds the ability to use a host-specific > > > custom swizzle function, then uses that to fix the IRQ mapping. > > > > > > I'm hoping this is the right way to perform such a workaround - please provide > > > feedback if not. Patch1 will likely collide with the owrk Lucas is doing > > > with device-tree bindings and MSI support. I'm happy to re-base if/when > > > needed. > > > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > > Cc: Richard Zhu <r65037@freescale.com> > > > Cc: Shawn Guo <shawn.guo@linaro.org> > > > Cc: Lucas Stach <l.stach@pengutronix.de> > > > Cc: Sean Cross <xobs@kosagi.com> > > > Cc: Jingoo Han <jg1.han@samsung.com> > > > --- > > > Tim Harvey (3): > > > PCI: designware: add legacy PCI interrupt mapping > > > PCI: designware: add ability for custom swizzle > > > PCI: imx6: ventana: fixup for IRQ mismapping > > > > > > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- > > > drivers/pci/host/pci-exynos.c | 7 ++++-- > > > drivers/pci/host/pci-imx6.c | 47 ++++++++++++++++++++++++++++++++++---- > > > drivers/pci/host/pcie-designware.c | 11 ++++++++- > > > drivers/pci/host/pcie-designware.h | 3 ++- > > > include/linux/pci_ids.h | 1 + > > > 6 files changed, 62 insertions(+), 9 deletions(-) > > > > > > -- > > > 1.8.3.2 > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping 2014-02-28 6:22 ` Jingoo Han @ 2014-02-28 10:15 ` Marek Vasut 2014-02-28 16:52 ` Tim Harvey 1 sibling, 0 replies; 29+ messages in thread From: Marek Vasut @ 2014-02-28 10:15 UTC (permalink / raw) To: linux-arm-kernel On Friday, February 28, 2014 at 07:22:52 AM, Jingoo Han wrote: > On Friday, February 28, 2014 1:16 PM, Tim Harvey wrote: > > On Thu, Feb 27, 2014 at 5:50 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > > > An add-in card used on the Ventana IMX6 SoC based family of boards > > > > has a TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings > > > > between the bridge and the four mini-PCI slots are swapped > > > > (INTD/C/B/A). > > > > > > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR > > > DCG) > > > > > > This problem happens from the 'Board', not a 'SoC'. > > > 'TI XIO2001 PCIe-to-PCI bridge' is not a 'SoC'. > > > 'pci-imx6.c' is the driver for 'IMX6 PCI IP', not for 'IMX6 SoC based > > > board'. Isn't it? > > > > Jingoo, > > > > Correct, this is an issue in the way the XIO2001 was hooked up to the > > PCI slots, so it should be viewed as a board issue. > > (+CC Arnd Bergmann) > > Then, this board fixup code should NOT be placed in './drivers/pci/host/' > side. > > > > Then, the workaround code for board problem should NOT be > > > included to './drivers/pci/host/' side. > > > > I would agree, but to overcome this sort of interrupt mapping issue > > one would need to either implement a custom swizzle or perhaps a > > custom map_irq and both of those are hooked into the pcie driver core. > > > > Do you have any suggestions on where/how I would better hook into > > > > those? > > Anyway, 'TI XIO2001 PCIe-to-PCI bridge' chip on the board is the > culprit. So, the board specific side is a good place. > For instance, ./arch/arm/mach-imx/ > > I don't know how to handle this problem. > But, there is no reason that 'pcie-designware.c' should take a care > of the board specific issue. Can you not just supply the INTA...INTD IRQ numbers in reverse order via DT for this board ? ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping 2014-02-28 6:22 ` Jingoo Han 2014-02-28 10:15 ` Marek Vasut @ 2014-02-28 16:52 ` Tim Harvey 2014-02-28 16:55 ` Lucas Stach 1 sibling, 1 reply; 29+ messages in thread From: Tim Harvey @ 2014-02-28 16:52 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 27, 2014 at 10:22 PM, Jingoo Han <jg1.han@samsung.com> wrote: > On Friday, February 28, 2014 1:16 PM, Tim Harvey wrote: >> On Thu, Feb 27, 2014 at 5:50 PM, Jingoo Han <jg1.han@samsung.com> wrote: >> > >> > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: >> > > >> > > An add-in card used on the Ventana IMX6 SoC based family of boards has a >> > > TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings between the >> > > bridge and the four mini-PCI slots are swapped (INTD/C/B/A). >> > >> > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) >> > >> > This problem happens from the 'Board', not a 'SoC'. >> > 'TI XIO2001 PCIe-to-PCI bridge' is not a 'SoC'. >> > 'pci-imx6.c' is the driver for 'IMX6 PCI IP', not for 'IMX6 SoC based >> > board'. Isn't it? >> >> Jingoo, >> >> Correct, this is an issue in the way the XIO2001 was hooked up to the >> PCI slots, so it should be viewed as a board issue. > > (+CC Arnd Bergmann) > > Then, this board fixup code should NOT be placed in './drivers/pci/host/' > side. > >> >> > >> > Then, the workaround code for board problem should NOT be >> > included to './drivers/pci/host/' side. >> >> I would agree, but to overcome this sort of interrupt mapping issue >> one would need to either implement a custom swizzle or perhaps a >> custom map_irq and both of those are hooked into the pcie driver core. >> Do you have any suggestions on where/how I would better hook into >> those? > > Anyway, 'TI XIO2001 PCIe-to-PCI bridge' chip on the board is the > culprit. So, the board specific side is a good place. > For instance, ./arch/arm/mach-imx/ > > I don't know how to handle this problem. > But, there is no reason that 'pcie-designware.c' should take a care > of the board specific issue. > > Arnd Bergmann, > Would you give your opinions? I agree with what your saying, but I don't see a way to put a custom map_irq or swizzle in arch/arm/mach-imx when the imx6 pcie driver is in drivers/pci/host. Many machines sitll have their pcie host controller drivers in arch/arm/mach-* but the imx6 was placed in drivers/host/pci because it shares the designware core with other boards (exynos if not more). If you look at the PATCH 3/3, I did take care to work around this in pci-imx6.c 'not' pcie-designware.c (I had to add support for assigning a custom swizzle - see PATH 2/3). I'm looking for suggestions as to if/how to get it into someplace more 'board-specific' (arch/arm/mach-imx) or 'device-specific' (ie pci fixup for xio2001). As far as I can tell this has to be resolved with either a custom swizzle or map_irq which are function pointers in struct hw_pci which is passed in to pci_common_init from pci host controller drivers. Perhaps there is a way to re-assign the swizzle function after pcie-designware.c has called pci_common_init? I see that pcibios_init_hw assignes the map_irq/swizzle passed in from the struct hw_pci to a struct pci_sys_data yet I'm not sure if I can get to this structure within a pci fixup. Thanks, Tim > > Best regards, > Jingoo Han > >> >> > >> > Also, please add the following members to CC list. They will give >> > important comments. >> > >> > : Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG >> > >> > Best regards, >> > Jingoo Han >> >> Thanks, >> >> Tim >> >> > >> > > >> > > This series adds legacy PCI interrupt mapping to support PCI bridges on >> > > IMX6 PCIe host controllers, adds the ability to use a host-specific >> > > custom swizzle function, then uses that to fix the IRQ mapping. >> > > >> > > I'm hoping this is the right way to perform such a workaround - please provide >> > > feedback if not. Patch1 will likely collide with the owrk Lucas is doing >> > > with device-tree bindings and MSI support. I'm happy to re-base if/when >> > > needed. >> > > >> > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> >> > > Cc: Bjorn Helgaas <bhelgaas@google.com> >> > > Cc: Richard Zhu <r65037@freescale.com> >> > > Cc: Shawn Guo <shawn.guo@linaro.org> >> > > Cc: Lucas Stach <l.stach@pengutronix.de> >> > > Cc: Sean Cross <xobs@kosagi.com> >> > > Cc: Jingoo Han <jg1.han@samsung.com> >> > > --- >> > > Tim Harvey (3): >> > > PCI: designware: add legacy PCI interrupt mapping >> > > PCI: designware: add ability for custom swizzle >> > > PCI: imx6: ventana: fixup for IRQ mismapping >> > > >> > > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- >> > > drivers/pci/host/pci-exynos.c | 7 ++++-- >> > > drivers/pci/host/pci-imx6.c | 47 ++++++++++++++++++++++++++++++++++---- >> > > drivers/pci/host/pcie-designware.c | 11 ++++++++- >> > > drivers/pci/host/pcie-designware.h | 3 ++- >> > > include/linux/pci_ids.h | 1 + >> > > 6 files changed, 62 insertions(+), 9 deletions(-) >> > > >> > > -- >> > > 1.8.3.2 >> > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping 2014-02-28 16:52 ` Tim Harvey @ 2014-02-28 16:55 ` Lucas Stach 0 siblings, 0 replies; 29+ messages in thread From: Lucas Stach @ 2014-02-28 16:55 UTC (permalink / raw) To: linux-arm-kernel Am Freitag, den 28.02.2014, 08:52 -0800 schrieb Tim Harvey: > On Thu, Feb 27, 2014 at 10:22 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > On Friday, February 28, 2014 1:16 PM, Tim Harvey wrote: > >> On Thu, Feb 27, 2014 at 5:50 PM, Jingoo Han <jg1.han@samsung.com> wrote: > >> > > >> > On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > >> > > > >> > > An add-in card used on the Ventana IMX6 SoC based family of boards has a > >> > > TI XIO2001 PCIe-to-PCI bridge where the INTA/B/C/D mappings between the > >> > > bridge and the four mini-PCI slots are swapped (INTD/C/B/A). > >> > > >> > (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) > >> > > >> > This problem happens from the 'Board', not a 'SoC'. > >> > 'TI XIO2001 PCIe-to-PCI bridge' is not a 'SoC'. > >> > 'pci-imx6.c' is the driver for 'IMX6 PCI IP', not for 'IMX6 SoC based > >> > board'. Isn't it? > >> > >> Jingoo, > >> > >> Correct, this is an issue in the way the XIO2001 was hooked up to the > >> PCI slots, so it should be viewed as a board issue. > > > > (+CC Arnd Bergmann) > > > > Then, this board fixup code should NOT be placed in './drivers/pci/host/' > > side. > > > >> > >> > > >> > Then, the workaround code for board problem should NOT be > >> > included to './drivers/pci/host/' side. > >> > >> I would agree, but to overcome this sort of interrupt mapping issue > >> one would need to either implement a custom swizzle or perhaps a > >> custom map_irq and both of those are hooked into the pcie driver core. > >> Do you have any suggestions on where/how I would better hook into > >> those? > > > > Anyway, 'TI XIO2001 PCIe-to-PCI bridge' chip on the board is the > > culprit. So, the board specific side is a good place. > > For instance, ./arch/arm/mach-imx/ > > > > I don't know how to handle this problem. > > But, there is no reason that 'pcie-designware.c' should take a care > > of the board specific issue. > > > > Arnd Bergmann, > > Would you give your opinions? > > I agree with what your saying, but I don't see a way to put a custom > map_irq or swizzle in arch/arm/mach-imx when the imx6 pcie driver is > in drivers/pci/host. Many machines sitll have their pcie host > controller drivers in arch/arm/mach-* but the imx6 was placed in > drivers/host/pci because it shares the designware core with other > boards (exynos if not more). > > If you look at the PATCH 3/3, I did take care to work around this in > pci-imx6.c 'not' pcie-designware.c (I had to add support for assigning > a custom swizzle - see PATH 2/3). > > I'm looking for suggestions as to if/how to get it into someplace more > 'board-specific' (arch/arm/mach-imx) or 'device-specific' (ie pci > fixup for xio2001). As far as I can tell this has to be resolved with > either a custom swizzle or map_irq which are function pointers in > struct hw_pci which is passed in to pci_common_init from pci host > controller drivers. > > Perhaps there is a way to re-assign the swizzle function after > pcie-designware.c has called pci_common_init? I see that > pcibios_init_hw assignes the map_irq/swizzle passed in from the struct > hw_pci to a struct pci_sys_data yet I'm not sure if I can get to this > structure within a pci fixup. > I'll send a series to fix the IRQ mapping in the host driver in a few minutes. After this you should be able to trivially represent you swizzled IRQ lines by adding your PCIe-to-PCI bridge in the devicetree of your board and set the correct IRQ mapping table. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2014-03-04 1:01 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-02-28 1:19 [RFC PATCH 0/3] PCI: imx6: fixup for add-in card IRQ mismapping Tim Harvey 2014-02-28 1:19 ` [PATCH 1/3] PCI: designware: add legacy PCI interrupt mapping Tim Harvey 2014-02-28 2:00 ` Jingoo Han 2014-02-28 4:24 ` Tim Harvey 2014-02-28 7:01 ` Jingoo Han 2014-02-28 10:12 ` Marek Vasut 2014-02-28 11:27 ` Lucas Stach 2014-02-28 11:37 ` Arnd Bergmann 2014-02-28 12:09 ` Lucas Stach 2014-02-28 12:27 ` Arnd Bergmann 2014-02-28 13:53 ` Lucas Stach 2014-02-28 2:10 ` Shawn Guo 2014-02-28 1:19 ` [PATCH 2/3] PCI: designware: add ability for custom swizzle Tim Harvey 2014-02-28 1:19 ` [PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping Tim Harvey 2014-02-28 2:10 ` Jingoo Han 2014-02-28 9:27 ` Arnd Bergmann 2014-02-28 17:39 ` Jason Gunthorpe 2014-03-01 0:52 ` Tim Harvey 2014-03-01 1:22 ` Jason Gunthorpe 2014-03-03 19:59 ` Tim Harvey 2014-03-03 23:37 ` Jason Gunthorpe 2014-03-04 0:38 ` Tim Harvey 2014-03-04 1:01 ` Jason Gunthorpe 2014-02-28 1:50 ` [RFC PATCH 0/3] PCI: imx6: fixup for add-in card " Jingoo Han 2014-02-28 4:16 ` Tim Harvey 2014-02-28 6:22 ` Jingoo Han 2014-02-28 10:15 ` Marek Vasut 2014-02-28 16:52 ` Tim Harvey 2014-02-28 16:55 ` Lucas Stach
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.