* [PATCH 0/3] DWC host without MSI controller @ 2017-08-28 14:23 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: Bjorn Helgaas, Tim Harvey, Jingoo Han, Joao Pinto, Shawn Guo Cc: linux-pci, linux-arm-kernel, patchwork-lst, kernel Hi all, this small series tries to fix/workaround a serious design flaw of the DWC PCIe host controller: it is unable to work with both legacy and MSI IRQs enabled at the same time. As soon as the first MSI is enabled in the DWC MSI controller, the host stops forwarding legacy IRQs. If the MSI controller is present, MSIs will be used for the PCIe port services IRQs, leaving endpoint devices which don't support MSIs unable to raise IRQs. It is only safe to enable the MSI controller if it is validated that all PCIe devices and drivers in the system support working MSIs. As most devices support falling back to using legacy PCIe IRQs if MSI support is missing it is much safer to disable the MSI by default and only enable it on validated systems. Feedback welcome. Regards, Lucas Lucas Stach (3): PCI: designware: only register MSI controller when MSI irq line is valid PCI: imx6: allow MSI irq to be absent ARM: dts: imx6qdl: remove MSI irq line .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- arch/arm/boot/dts/imx6qdl.dtsi | 2 -- drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- drivers/pci/dwc/pcie-designware-host.c | 4 ++-- 4 files changed, 17 insertions(+), 20 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] DWC host without MSI controller @ 2017-08-28 14:23 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: linux-arm-kernel Hi all, this small series tries to fix/workaround a serious design flaw of the DWC PCIe host controller: it is unable to work with both legacy and MSI IRQs enabled at the same time. As soon as the first MSI is enabled in the DWC MSI controller, the host stops forwarding legacy IRQs. If the MSI controller is present, MSIs will be used for the PCIe port services IRQs, leaving endpoint devices which don't support MSIs unable to raise IRQs. It is only safe to enable the MSI controller if it is validated that all PCIe devices and drivers in the system support working MSIs. As most devices support falling back to using legacy PCIe IRQs if MSI support is missing it is much safer to disable the MSI by default and only enable it on validated systems. Feedback welcome. Regards, Lucas Lucas Stach (3): PCI: designware: only register MSI controller when MSI irq line is valid PCI: imx6: allow MSI irq to be absent ARM: dts: imx6qdl: remove MSI irq line .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- arch/arm/boot/dts/imx6qdl.dtsi | 2 -- drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- drivers/pci/dwc/pcie-designware-host.c | 4 ++-- 4 files changed, 17 insertions(+), 20 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid 2017-08-28 14:23 ` Lucas Stach @ 2017-08-28 14:23 ` Lucas Stach -1 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: Bjorn Helgaas, Tim Harvey, Jingoo Han, Joao Pinto, Shawn Guo Cc: linux-pci, linux-arm-kernel, patchwork-lst, kernel The MSI part of the controller isn't essential, so the host controller can be registered without the MSI controller being present. This allows the host to work in PCIe legancy interrupt only mode, if the IRQ line for the MSI controller is missing. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/pci/dwc/pcie-designware-host.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c index d29c020da082..8494089f088d 100644 --- a/drivers/pci/dwc/pcie-designware-host.c +++ b/drivers/pci/dwc/pcie-designware-host.c @@ -381,7 +381,7 @@ int dw_pcie_host_init(struct pcie_port *pp) if (ret) pci->num_viewport = 2; - if (IS_ENABLED(CONFIG_PCI_MSI)) { + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { if (!pp->ops->msi_host_init) { pp->irq_domain = irq_domain_add_linear(dev->of_node, MAX_MSI_IRQS, &msi_domain_ops, @@ -412,7 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp) bridge->ops = &dw_pcie_ops; bridge->map_irq = of_irq_parse_and_map_pci; bridge->swizzle_irq = pci_common_swizzle; - if (IS_ENABLED(CONFIG_PCI_MSI)) { + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { bridge->msi = &dw_pcie_msi_chip; dw_pcie_msi_chip.dev = dev; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid @ 2017-08-28 14:23 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: linux-arm-kernel The MSI part of the controller isn't essential, so the host controller can be registered without the MSI controller being present. This allows the host to work in PCIe legancy interrupt only mode, if the IRQ line for the MSI controller is missing. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/pci/dwc/pcie-designware-host.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c index d29c020da082..8494089f088d 100644 --- a/drivers/pci/dwc/pcie-designware-host.c +++ b/drivers/pci/dwc/pcie-designware-host.c @@ -381,7 +381,7 @@ int dw_pcie_host_init(struct pcie_port *pp) if (ret) pci->num_viewport = 2; - if (IS_ENABLED(CONFIG_PCI_MSI)) { + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { if (!pp->ops->msi_host_init) { pp->irq_domain = irq_domain_add_linear(dev->of_node, MAX_MSI_IRQS, &msi_domain_ops, @@ -412,7 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp) bridge->ops = &dw_pcie_ops; bridge->map_irq = of_irq_parse_and_map_pci; bridge->swizzle_irq = pci_common_swizzle; - if (IS_ENABLED(CONFIG_PCI_MSI)) { + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { bridge->msi = &dw_pcie_msi_chip; dw_pcie_msi_chip.dev = dev; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid 2017-08-28 14:23 ` Lucas Stach @ 2017-08-31 16:58 ` Bjorn Helgaas -1 siblings, 0 replies; 20+ messages in thread From: Bjorn Helgaas @ 2017-08-31 16:58 UTC (permalink / raw) To: Lucas Stach Cc: Gabriele Paoloni, linux-pci, Binghui Wang, Roy Zang, Kishon Vijay Abraham I, Jesper Nilsson, Joao Pinto, Pratyush Anand, Krzysztof Kozlowski, patchwork-lst, Kukjin Kim, Niklas Cassel, Richard Zhu, Tim Harvey, Xiaowei Song, Murali Karicheri, Bjorn Helgaas, Mingkai Hu, linux-arm-kernel, Thomas Petazzoni, Jingoo Han, Stanimir Varbanov, Minghuan Lian, Zhou Wang, kernel, Shawn Guo [+cc Kishon, Thomas, Niklas, Jesper, Zhou, Gabriele, Xiaowei, Binghui, Stanimir, Pratyush, Kukjin, Krzysztof, Richard, Murali, Minghuan, Mingkai, Roy] Sorry about the unwieldy cc list. It seems like this affects every DesignWare-based driver. On Mon, Aug 28, 2017 at 04:23:05PM +0200, Lucas Stach wrote: > The MSI part of the controller isn't essential, so the host controller can > be registered without the MSI controller being present. This allows the > host to work in PCIe legancy interrupt only mode, if the IRQ line for the s/legancy/legacy/ > MSI controller is missing. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/pci/dwc/pcie-designware-host.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c > index d29c020da082..8494089f088d 100644 > --- a/drivers/pci/dwc/pcie-designware-host.c > +++ b/drivers/pci/dwc/pcie-designware-host.c > @@ -381,7 +381,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > if (ret) > pci->num_viewport = 2; > > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > if (!pp->ops->msi_host_init) { > pp->irq_domain = irq_domain_add_linear(dev->of_node, > MAX_MSI_IRQS, &msi_domain_ops, > @@ -412,7 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > bridge->ops = &dw_pcie_ops; > bridge->map_irq = of_irq_parse_and_map_pci; > bridge->swizzle_irq = pci_common_swizzle; > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > bridge->msi = &dw_pcie_msi_chip; > dw_pcie_msi_chip.dev = dev; > } Here are the callers of dw_pcie_host_init(): armada8k_add_pcie_port artpec6_add_pcie_port # sets pp->msi_irq dra7xx_add_pcie_port dw_plat_add_pcie_port # sets pp->msi_irq exynos_add_pcie_port # sets pp->msi_irq hisi_add_pcie_port imx6_add_pcie_port # sets pp->msi_irq kirin_add_pcie_port ks_dw_pcie_host_init # sets pp->ops->msi_host_init ls_add_pcie_port # sets pp->ops->msi_host_init qcom_pcie_probe # sets pp->msi_irq spear13xx_add_pcie_port For the drivers that set pp->msi_irq (artpec6, dw_plat, exynos, imx6, qcom), it seems like you'd want a similar follow-up change for all of them to make the MSI IRQ optional, but you only changed imx6. What about the others? For the drivers that don't set pp->msi_irq and don't set pp->ops->msi_host_init (armada8k, dra7xx, hisi, kirin, spear13xx), this patch means they no longer set up pp->irq_domain. Do you intend that? Incidental observations not strictly related to this series: - It looks like exynos_add_pcie_port() incorrectly assumes platform_get_irq() returns zero on failure (twice). - It'd be nice if qcom_pcie_probe() followed the same add_pcie_port() structure as the other drivers. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid @ 2017-08-31 16:58 ` Bjorn Helgaas 0 siblings, 0 replies; 20+ messages in thread From: Bjorn Helgaas @ 2017-08-31 16:58 UTC (permalink / raw) To: linux-arm-kernel [+cc Kishon, Thomas, Niklas, Jesper, Zhou, Gabriele, Xiaowei, Binghui, Stanimir, Pratyush, Kukjin, Krzysztof, Richard, Murali, Minghuan, Mingkai, Roy] Sorry about the unwieldy cc list. It seems like this affects every DesignWare-based driver. On Mon, Aug 28, 2017 at 04:23:05PM +0200, Lucas Stach wrote: > The MSI part of the controller isn't essential, so the host controller can > be registered without the MSI controller being present. This allows the > host to work in PCIe legancy interrupt only mode, if the IRQ line for the s/legancy/legacy/ > MSI controller is missing. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/pci/dwc/pcie-designware-host.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c > index d29c020da082..8494089f088d 100644 > --- a/drivers/pci/dwc/pcie-designware-host.c > +++ b/drivers/pci/dwc/pcie-designware-host.c > @@ -381,7 +381,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > if (ret) > pci->num_viewport = 2; > > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > if (!pp->ops->msi_host_init) { > pp->irq_domain = irq_domain_add_linear(dev->of_node, > MAX_MSI_IRQS, &msi_domain_ops, > @@ -412,7 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > bridge->ops = &dw_pcie_ops; > bridge->map_irq = of_irq_parse_and_map_pci; > bridge->swizzle_irq = pci_common_swizzle; > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > bridge->msi = &dw_pcie_msi_chip; > dw_pcie_msi_chip.dev = dev; > } Here are the callers of dw_pcie_host_init(): armada8k_add_pcie_port artpec6_add_pcie_port # sets pp->msi_irq dra7xx_add_pcie_port dw_plat_add_pcie_port # sets pp->msi_irq exynos_add_pcie_port # sets pp->msi_irq hisi_add_pcie_port imx6_add_pcie_port # sets pp->msi_irq kirin_add_pcie_port ks_dw_pcie_host_init # sets pp->ops->msi_host_init ls_add_pcie_port # sets pp->ops->msi_host_init qcom_pcie_probe # sets pp->msi_irq spear13xx_add_pcie_port For the drivers that set pp->msi_irq (artpec6, dw_plat, exynos, imx6, qcom), it seems like you'd want a similar follow-up change for all of them to make the MSI IRQ optional, but you only changed imx6. What about the others? For the drivers that don't set pp->msi_irq and don't set pp->ops->msi_host_init (armada8k, dra7xx, hisi, kirin, spear13xx), this patch means they no longer set up pp->irq_domain. Do you intend that? Incidental observations not strictly related to this series: - It looks like exynos_add_pcie_port() incorrectly assumes platform_get_irq() returns zero on failure (twice). - It'd be nice if qcom_pcie_probe() followed the same add_pcie_port() structure as the other drivers. Bjorn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid 2017-08-31 16:58 ` Bjorn Helgaas @ 2017-08-31 17:32 ` Fabio Estevam -1 siblings, 0 replies; 20+ messages in thread From: Fabio Estevam @ 2017-08-31 17:32 UTC (permalink / raw) To: Bjorn Helgaas Cc: Lucas Stach, Bjorn Helgaas, Tim Harvey, Jingoo Han, Joao Pinto, Shawn Guo, linux-pci, linux-arm-kernel, patchwork-lst, Sascha Hauer, Kishon Vijay Abraham I, Thomas Petazzoni, Niklas Cassel, Jesper Nilsson, Zhou Wang, Gabriele Paoloni, Xiaowei Song, Binghui Wang, Stanimir Varbanov, Pratyush Anand, Kukjin Kim, Krzysztof Kozlowski, Richard Zhu, Murali Karicheri, Minghuan Lian, Mingkai Hu, Roy Zang Hi Bjorn, On Thu, Aug 31, 2017 at 1:58 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > Incidental observations not strictly related to this series: > > - It looks like exynos_add_pcie_port() incorrectly assumes > platform_get_irq() returns zero on failure (twice). I will send a series cleaning up the error handling in platform_get_irq() for the various PCI drivers. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid @ 2017-08-31 17:32 ` Fabio Estevam 0 siblings, 0 replies; 20+ messages in thread From: Fabio Estevam @ 2017-08-31 17:32 UTC (permalink / raw) To: linux-arm-kernel Hi Bjorn, On Thu, Aug 31, 2017 at 1:58 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > Incidental observations not strictly related to this series: > > - It looks like exynos_add_pcie_port() incorrectly assumes > platform_get_irq() returns zero on failure (twice). I will send a series cleaning up the error handling in platform_get_irq() for the various PCI drivers. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid 2017-08-31 16:58 ` Bjorn Helgaas @ 2017-09-25 23:54 ` Bjorn Helgaas -1 siblings, 0 replies; 20+ messages in thread From: Bjorn Helgaas @ 2017-09-25 23:54 UTC (permalink / raw) To: Lucas Stach Cc: Gabriele Paoloni, linux-pci, Binghui Wang, Shawn Guo, patchwork-lst, Jesper Nilsson, Joao Pinto, Pratyush Anand, Krzysztof Kozlowski, Kishon Vijay Abraham I, Kukjin Kim, Niklas Cassel, Richard Zhu, Tim Harvey, Xiaowei Song, Murali Karicheri, Bjorn Helgaas, Mingkai Hu, linux-arm-kernel, Thomas Petazzoni, Jingoo Han, Stanimir Varbanov, Minghuan Lian, Zhou Wang, kernel, Roy Zang On Thu, Aug 31, 2017 at 11:58:17AM -0500, Bjorn Helgaas wrote: > [+cc Kishon, Thomas, Niklas, Jesper, Zhou, Gabriele, Xiaowei, Binghui, > Stanimir, Pratyush, Kukjin, Krzysztof, Richard, Murali, Minghuan, Mingkai, > Roy] > > Sorry about the unwieldy cc list. It seems like this affects every > DesignWare-based driver. > > On Mon, Aug 28, 2017 at 04:23:05PM +0200, Lucas Stach wrote: > > The MSI part of the controller isn't essential, so the host controller can > > be registered without the MSI controller being present. This allows the > > host to work in PCIe legancy interrupt only mode, if the IRQ line for the > > s/legancy/legacy/ > > > MSI controller is missing. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > drivers/pci/dwc/pcie-designware-host.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c > > index d29c020da082..8494089f088d 100644 > > --- a/drivers/pci/dwc/pcie-designware-host.c > > +++ b/drivers/pci/dwc/pcie-designware-host.c > > @@ -381,7 +381,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > > if (ret) > > pci->num_viewport = 2; > > > > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > > if (!pp->ops->msi_host_init) { > > pp->irq_domain = irq_domain_add_linear(dev->of_node, > > MAX_MSI_IRQS, &msi_domain_ops, > > @@ -412,7 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > > bridge->ops = &dw_pcie_ops; > > bridge->map_irq = of_irq_parse_and_map_pci; > > bridge->swizzle_irq = pci_common_swizzle; > > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > > bridge->msi = &dw_pcie_msi_chip; > > dw_pcie_msi_chip.dev = dev; > > } > > Here are the callers of dw_pcie_host_init(): > > armada8k_add_pcie_port > artpec6_add_pcie_port # sets pp->msi_irq > dra7xx_add_pcie_port > dw_plat_add_pcie_port # sets pp->msi_irq > exynos_add_pcie_port # sets pp->msi_irq > hisi_add_pcie_port > imx6_add_pcie_port # sets pp->msi_irq > kirin_add_pcie_port > ks_dw_pcie_host_init # sets pp->ops->msi_host_init > ls_add_pcie_port # sets pp->ops->msi_host_init > qcom_pcie_probe # sets pp->msi_irq > spear13xx_add_pcie_port > > For the drivers that set pp->msi_irq (artpec6, dw_plat, exynos, imx6, > qcom), it seems like you'd want a similar follow-up change for all of > them to make the MSI IRQ optional, but you only changed imx6. What > about the others? > > For the drivers that don't set pp->msi_irq and don't set > pp->ops->msi_host_init (armada8k, dra7xx, hisi, kirin, spear13xx), > this patch means they no longer set up pp->irq_domain. Do you intend > that? Ping, I'm looking for a v2 that addresses this. No hurry, just making sure you're not waiting on me. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid @ 2017-09-25 23:54 ` Bjorn Helgaas 0 siblings, 0 replies; 20+ messages in thread From: Bjorn Helgaas @ 2017-09-25 23:54 UTC (permalink / raw) To: linux-arm-kernel On Thu, Aug 31, 2017 at 11:58:17AM -0500, Bjorn Helgaas wrote: > [+cc Kishon, Thomas, Niklas, Jesper, Zhou, Gabriele, Xiaowei, Binghui, > Stanimir, Pratyush, Kukjin, Krzysztof, Richard, Murali, Minghuan, Mingkai, > Roy] > > Sorry about the unwieldy cc list. It seems like this affects every > DesignWare-based driver. > > On Mon, Aug 28, 2017 at 04:23:05PM +0200, Lucas Stach wrote: > > The MSI part of the controller isn't essential, so the host controller can > > be registered without the MSI controller being present. This allows the > > host to work in PCIe legancy interrupt only mode, if the IRQ line for the > > s/legancy/legacy/ > > > MSI controller is missing. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > drivers/pci/dwc/pcie-designware-host.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c > > index d29c020da082..8494089f088d 100644 > > --- a/drivers/pci/dwc/pcie-designware-host.c > > +++ b/drivers/pci/dwc/pcie-designware-host.c > > @@ -381,7 +381,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > > if (ret) > > pci->num_viewport = 2; > > > > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > > if (!pp->ops->msi_host_init) { > > pp->irq_domain = irq_domain_add_linear(dev->of_node, > > MAX_MSI_IRQS, &msi_domain_ops, > > @@ -412,7 +412,7 @@ int dw_pcie_host_init(struct pcie_port *pp) > > bridge->ops = &dw_pcie_ops; > > bridge->map_irq = of_irq_parse_and_map_pci; > > bridge->swizzle_irq = pci_common_swizzle; > > - if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + if (IS_ENABLED(CONFIG_PCI_MSI) && pp->msi_irq > 0) { > > bridge->msi = &dw_pcie_msi_chip; > > dw_pcie_msi_chip.dev = dev; > > } > > Here are the callers of dw_pcie_host_init(): > > armada8k_add_pcie_port > artpec6_add_pcie_port # sets pp->msi_irq > dra7xx_add_pcie_port > dw_plat_add_pcie_port # sets pp->msi_irq > exynos_add_pcie_port # sets pp->msi_irq > hisi_add_pcie_port > imx6_add_pcie_port # sets pp->msi_irq > kirin_add_pcie_port > ks_dw_pcie_host_init # sets pp->ops->msi_host_init > ls_add_pcie_port # sets pp->ops->msi_host_init > qcom_pcie_probe # sets pp->msi_irq > spear13xx_add_pcie_port > > For the drivers that set pp->msi_irq (artpec6, dw_plat, exynos, imx6, > qcom), it seems like you'd want a similar follow-up change for all of > them to make the MSI IRQ optional, but you only changed imx6. What > about the others? > > For the drivers that don't set pp->msi_irq and don't set > pp->ops->msi_host_init (armada8k, dra7xx, hisi, kirin, spear13xx), > this patch means they no longer set up pp->irq_domain. Do you intend > that? Ping, I'm looking for a v2 that addresses this. No hurry, just making sure you're not waiting on me. Bjorn ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] PCI: imx6: allow MSI irq to be absent 2017-08-28 14:23 ` Lucas Stach @ 2017-08-28 14:23 ` Lucas Stach -1 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: Bjorn Helgaas, Tim Harvey, Jingoo Han, Joao Pinto, Shawn Guo Cc: linux-pci, linux-arm-kernel, patchwork-lst, kernel The host can fall back to PCIe legacy IRQ only operation if the MSI irq is missing, thus allowing systems to work with peripherals that don't support MSI irqs. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt index cf92d3ba5a26..d85db21503f4 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt @@ -10,14 +10,14 @@ Required properties: - "fsl,imx6qp-pcie" - "fsl,imx7d-pcie" - reg: base address and length of the PCIe controller -- interrupts: A list of interrupt outputs of the controller. Must contain an - entry for each entry in the interrupt-names property. -- interrupt-names: Must include the following entries: - - "msi": The interrupt that is asserted when an MSI is received - clock-names: Must include the following additional entries: - "pcie_phy" Optional properties: +- interrupts: A list of interrupt outputs of the controller. Must contain an + entry for each entry in the interrupt-names property. +- interrupt-names: Must include the following entries: + - "msi": The interrupt that is asserted when an MSI is received - fsl,tx-deemph-gen1: Gen1 De-emphasis value. Default: 0 - fsl,tx-deemph-gen2-3p5db: Gen2 (3.5db) De-emphasis value. Default: 0 - fsl,tx-deemph-gen2-6db: Gen2 (6db) De-emphasis value. Default: 20 diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c index bf5c3616e344..d2507272b3ab 100644 --- a/drivers/pci/dwc/pci-imx6.c +++ b/drivers/pci/dwc/pci-imx6.c @@ -671,18 +671,17 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, if (IS_ENABLED(CONFIG_PCI_MSI)) { pp->msi_irq = platform_get_irq_byname(pdev, "msi"); - if (pp->msi_irq <= 0) { - dev_err(dev, "failed to get MSI irq\n"); - return -ENODEV; - } - - ret = devm_request_irq(dev, pp->msi_irq, - imx6_pcie_msi_handler, - IRQF_SHARED | IRQF_NO_THREAD, - "mx6-pcie-msi", imx6_pcie); - if (ret) { - dev_err(dev, "failed to request MSI irq\n"); - return ret; + if (pp->msi_irq > 0) { + ret = devm_request_irq(dev, pp->msi_irq, + imx6_pcie_msi_handler, + IRQF_SHARED | IRQF_NO_THREAD, + "mx6-pcie-msi", imx6_pcie); + if (ret) { + dev_err(dev, "failed to request MSI irq\n"); + return ret; + } + } else { + dev_info(dev, "missing MSI irq, MSI support unavailable\n"); } } -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] PCI: imx6: allow MSI irq to be absent @ 2017-08-28 14:23 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: linux-arm-kernel The host can fall back to PCIe legacy IRQ only operation if the MSI irq is missing, thus allowing systems to work with peripherals that don't support MSI irqs. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt index cf92d3ba5a26..d85db21503f4 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt @@ -10,14 +10,14 @@ Required properties: - "fsl,imx6qp-pcie" - "fsl,imx7d-pcie" - reg: base address and length of the PCIe controller -- interrupts: A list of interrupt outputs of the controller. Must contain an - entry for each entry in the interrupt-names property. -- interrupt-names: Must include the following entries: - - "msi": The interrupt that is asserted when an MSI is received - clock-names: Must include the following additional entries: - "pcie_phy" Optional properties: +- interrupts: A list of interrupt outputs of the controller. Must contain an + entry for each entry in the interrupt-names property. +- interrupt-names: Must include the following entries: + - "msi": The interrupt that is asserted when an MSI is received - fsl,tx-deemph-gen1: Gen1 De-emphasis value. Default: 0 - fsl,tx-deemph-gen2-3p5db: Gen2 (3.5db) De-emphasis value. Default: 0 - fsl,tx-deemph-gen2-6db: Gen2 (6db) De-emphasis value. Default: 20 diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c index bf5c3616e344..d2507272b3ab 100644 --- a/drivers/pci/dwc/pci-imx6.c +++ b/drivers/pci/dwc/pci-imx6.c @@ -671,18 +671,17 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, if (IS_ENABLED(CONFIG_PCI_MSI)) { pp->msi_irq = platform_get_irq_byname(pdev, "msi"); - if (pp->msi_irq <= 0) { - dev_err(dev, "failed to get MSI irq\n"); - return -ENODEV; - } - - ret = devm_request_irq(dev, pp->msi_irq, - imx6_pcie_msi_handler, - IRQF_SHARED | IRQF_NO_THREAD, - "mx6-pcie-msi", imx6_pcie); - if (ret) { - dev_err(dev, "failed to request MSI irq\n"); - return ret; + if (pp->msi_irq > 0) { + ret = devm_request_irq(dev, pp->msi_irq, + imx6_pcie_msi_handler, + IRQF_SHARED | IRQF_NO_THREAD, + "mx6-pcie-msi", imx6_pcie); + if (ret) { + dev_err(dev, "failed to request MSI irq\n"); + return ret; + } + } else { + dev_info(dev, "missing MSI irq, MSI support unavailable\n"); } } -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] ARM: dts: imx6qdl: remove MSI irq line 2017-08-28 14:23 ` Lucas Stach @ 2017-08-28 14:23 ` Lucas Stach -1 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: Bjorn Helgaas, Tim Harvey, Jingoo Han, Joao Pinto, Shawn Guo Cc: linux-pci, linux-arm-kernel, patchwork-lst, kernel The DWC PCIe host controller doesn't support MSI and legacy IRQs at the same time. If the MSI controller is in use (which is always the case, as PCIe port serives are using MSI IRQs when available), legacy endpoint devices are unable to raise an IRQ. Remove the MSI irq line to inhibit the MSI controller from being used, which is a much better default, as most enpoint devices are able to fall back to legacy PCIe IRQs, if MSI are not available. Systems which are validated to work in MSI only mode can opt-in to use the MSI controller by adding back the MSI irq line in the board DT. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- arch/arm/boot/dts/imx6qdl.dtsi | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index a9723b94bafa..0f47a9d4024e 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -209,8 +209,6 @@ ranges = <0x81000000 0 0 0x01f80000 0 0x00010000 /* downstream I/O */ 0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable memory */ num-lanes = <1>; - interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi"; #interrupt-cells = <1>; interrupt-map-mask = <0 0 0 0x7>; interrupt-map = <0 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] ARM: dts: imx6qdl: remove MSI irq line @ 2017-08-28 14:23 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-08-28 14:23 UTC (permalink / raw) To: linux-arm-kernel The DWC PCIe host controller doesn't support MSI and legacy IRQs at the same time. If the MSI controller is in use (which is always the case, as PCIe port serives are using MSI IRQs when available), legacy endpoint devices are unable to raise an IRQ. Remove the MSI irq line to inhibit the MSI controller from being used, which is a much better default, as most enpoint devices are able to fall back to legacy PCIe IRQs, if MSI are not available. Systems which are validated to work in MSI only mode can opt-in to use the MSI controller by adding back the MSI irq line in the board DT. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- arch/arm/boot/dts/imx6qdl.dtsi | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index a9723b94bafa..0f47a9d4024e 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -209,8 +209,6 @@ ranges = <0x81000000 0 0 0x01f80000 0 0x00010000 /* downstream I/O */ 0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable memory */ num-lanes = <1>; - interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "msi"; #interrupt-cells = <1>; interrupt-map-mask = <0 0 0 0x7>; interrupt-map = <0 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, -- 2.11.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] DWC host without MSI controller 2017-08-28 14:23 ` Lucas Stach @ 2017-08-28 16:59 ` Tim Harvey -1 siblings, 0 replies; 20+ messages in thread From: Tim Harvey @ 2017-08-28 16:59 UTC (permalink / raw) To: Lucas Stach Cc: Joao Pinto, Jingoo Han, patchwork-lst, Sascha Hauer, linux-pci, Bjorn Helgaas, Shawn Guo, linux-arm-kernel On Mon, Aug 28, 2017 at 7:23 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > Hi all, > > this small series tries to fix/workaround a serious design flaw of the DWC PCIe > host controller: it is unable to work with both legacy and MSI IRQs enabled at > the same time. As soon as the first MSI is enabled in the DWC MSI controller, > the host stops forwarding legacy IRQs. > > If the MSI controller is present, MSIs will be used for the PCIe port services > IRQs, leaving endpoint devices which don't support MSIs unable to raise IRQs. > It is only safe to enable the MSI controller if it is validated that all PCIe > devices and drivers in the system support working MSIs. As most devices > support falling back to using legacy PCIe IRQs if MSI support is missing it is > much safer to disable the MSI by default and only enable it on validated > systems. > > Feedback welcome. > > Regards, > Lucas > > Lucas Stach (3): > PCI: designware: only register MSI controller when MSI irq line is > valid > PCI: imx6: allow MSI irq to be absent > ARM: dts: imx6qdl: remove MSI irq line > > .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- > arch/arm/boot/dts/imx6qdl.dtsi | 2 -- > drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- > drivers/pci/dwc/pcie-designware-host.c | 4 ++-- > 4 files changed, 17 insertions(+), 20 deletions(-) > Lucas, Thank you for following up on this! I tested it with and without the third patch that removes msi from the dt thus with both an ath9k 802.11 device which only supports legacy interrupts and a Marvell sky2 device which supports msi as well as legacy interrupts and it works as expected: - without msi enabled in dts (default): both sky2 and ath9k work - with msi enabled in dt: sky2 works ath9k does not Tested-by: Tim Harvey <tharvey@gateworks.com> Regards, Tim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] DWC host without MSI controller @ 2017-08-28 16:59 ` Tim Harvey 0 siblings, 0 replies; 20+ messages in thread From: Tim Harvey @ 2017-08-28 16:59 UTC (permalink / raw) To: linux-arm-kernel On Mon, Aug 28, 2017 at 7:23 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > Hi all, > > this small series tries to fix/workaround a serious design flaw of the DWC PCIe > host controller: it is unable to work with both legacy and MSI IRQs enabled at > the same time. As soon as the first MSI is enabled in the DWC MSI controller, > the host stops forwarding legacy IRQs. > > If the MSI controller is present, MSIs will be used for the PCIe port services > IRQs, leaving endpoint devices which don't support MSIs unable to raise IRQs. > It is only safe to enable the MSI controller if it is validated that all PCIe > devices and drivers in the system support working MSIs. As most devices > support falling back to using legacy PCIe IRQs if MSI support is missing it is > much safer to disable the MSI by default and only enable it on validated > systems. > > Feedback welcome. > > Regards, > Lucas > > Lucas Stach (3): > PCI: designware: only register MSI controller when MSI irq line is > valid > PCI: imx6: allow MSI irq to be absent > ARM: dts: imx6qdl: remove MSI irq line > > .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- > arch/arm/boot/dts/imx6qdl.dtsi | 2 -- > drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- > drivers/pci/dwc/pcie-designware-host.c | 4 ++-- > 4 files changed, 17 insertions(+), 20 deletions(-) > Lucas, Thank you for following up on this! I tested it with and without the third patch that removes msi from the dt thus with both an ath9k 802.11 device which only supports legacy interrupts and a Marvell sky2 device which supports msi as well as legacy interrupts and it works as expected: - without msi enabled in dts (default): both sky2 and ath9k work - with msi enabled in dt: sky2 works ath9k does not Tested-by: Tim Harvey <tharvey@gateworks.com> Regards, Tim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] DWC host without MSI controller 2017-08-28 16:59 ` Tim Harvey @ 2017-10-09 12:14 ` Fabio Estevam -1 siblings, 0 replies; 20+ messages in thread From: Fabio Estevam @ 2017-10-09 12:14 UTC (permalink / raw) To: Tim Harvey, Bjorn Helgaas Cc: Joao Pinto, Jingoo Han, patchwork-lst, Sascha Hauer, linux-pci, Shawn Guo, linux-arm-kernel, Lucas Stach Hi Bjorn, On Mon, Aug 28, 2017 at 1:59 PM, Tim Harvey <tharvey@gateworks.com> wrote: > On Mon, Aug 28, 2017 at 7:23 AM, Lucas Stach <l.stach@pengutronix.de> wrote: >> Hi all, >> >> this small series tries to fix/workaround a serious design flaw of the DWC PCIe >> host controller: it is unable to work with both legacy and MSI IRQs enabled at >> the same time. As soon as the first MSI is enabled in the DWC MSI controller, >> the host stops forwarding legacy IRQs. >> >> If the MSI controller is present, MSIs will be used for the PCIe port services >> IRQs, leaving endpoint devices which don't support MSIs unable to raise IRQs. >> It is only safe to enable the MSI controller if it is validated that all PCIe >> devices and drivers in the system support working MSIs. As most devices >> support falling back to using legacy PCIe IRQs if MSI support is missing it is >> much safer to disable the MSI by default and only enable it on validated >> systems. >> >> Feedback welcome. >> >> Regards, >> Lucas >> >> Lucas Stach (3): >> PCI: designware: only register MSI controller when MSI irq line is >> valid >> PCI: imx6: allow MSI irq to be absent >> ARM: dts: imx6qdl: remove MSI irq line >> >> .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- >> arch/arm/boot/dts/imx6qdl.dtsi | 2 -- >> drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- >> drivers/pci/dwc/pcie-designware-host.c | 4 ++-- >> 4 files changed, 17 insertions(+), 20 deletions(-) >> > > Lucas, > > Thank you for following up on this! > > I tested it with and without the third patch that removes msi from the > dt thus with both an ath9k 802.11 device which only supports legacy > interrupts and a Marvell sky2 device which supports msi as well as > legacy interrupts and it works as expected: > - without msi enabled in dts (default): both sky2 and ath9k work > - with msi enabled in dt: sky2 works ath9k does not > > Tested-by: Tim Harvey <tharvey@gateworks.com> Any comments about this series? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] DWC host without MSI controller @ 2017-10-09 12:14 ` Fabio Estevam 0 siblings, 0 replies; 20+ messages in thread From: Fabio Estevam @ 2017-10-09 12:14 UTC (permalink / raw) To: linux-arm-kernel Hi Bjorn, On Mon, Aug 28, 2017 at 1:59 PM, Tim Harvey <tharvey@gateworks.com> wrote: > On Mon, Aug 28, 2017 at 7:23 AM, Lucas Stach <l.stach@pengutronix.de> wrote: >> Hi all, >> >> this small series tries to fix/workaround a serious design flaw of the DWC PCIe >> host controller: it is unable to work with both legacy and MSI IRQs enabled at >> the same time. As soon as the first MSI is enabled in the DWC MSI controller, >> the host stops forwarding legacy IRQs. >> >> If the MSI controller is present, MSIs will be used for the PCIe port services >> IRQs, leaving endpoint devices which don't support MSIs unable to raise IRQs. >> It is only safe to enable the MSI controller if it is validated that all PCIe >> devices and drivers in the system support working MSIs. As most devices >> support falling back to using legacy PCIe IRQs if MSI support is missing it is >> much safer to disable the MSI by default and only enable it on validated >> systems. >> >> Feedback welcome. >> >> Regards, >> Lucas >> >> Lucas Stach (3): >> PCI: designware: only register MSI controller when MSI irq line is >> valid >> PCI: imx6: allow MSI irq to be absent >> ARM: dts: imx6qdl: remove MSI irq line >> >> .../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 ++++---- >> arch/arm/boot/dts/imx6qdl.dtsi | 2 -- >> drivers/pci/dwc/pci-imx6.c | 23 +++++++++++----------- >> drivers/pci/dwc/pcie-designware-host.c | 4 ++-- >> 4 files changed, 17 insertions(+), 20 deletions(-) >> > > Lucas, > > Thank you for following up on this! > > I tested it with and without the third patch that removes msi from the > dt thus with both an ath9k 802.11 device which only supports legacy > interrupts and a Marvell sky2 device which supports msi as well as > legacy interrupts and it works as expected: > - without msi enabled in dts (default): both sky2 and ath9k work > - with msi enabled in dt: sky2 works ath9k does not > > Tested-by: Tim Harvey <tharvey@gateworks.com> Any comments about this series? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] DWC host without MSI controller 2017-10-09 12:14 ` Fabio Estevam @ 2017-10-09 12:22 ` Lucas Stach -1 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-10-09 12:22 UTC (permalink / raw) To: Fabio Estevam, Tim Harvey, Bjorn Helgaas Cc: Joao Pinto, Jingoo Han, patchwork-lst, Sascha Hauer, linux-pci, Shawn Guo, linux-arm-kernel SGkgRmFiaW8sCgpBbSBNb250YWcsIGRlbiAwOS4xMC4yMDE3LCAwOToxNCAtMDMwMCBzY2hyaWVi IEZhYmlvIEVzdGV2YW06Cj4gSGkgQmpvcm4sCj4gCj4gT24gTW9uLCBBdWcgMjgsIDIwMTcgYXQg MTo1OSBQTSwgVGltIEhhcnZleSA8dGhhcnZleUBnYXRld29ya3MuY29tPgo+IHdyb3RlOgo+ID4g T24gTW9uLCBBdWcgMjgsIDIwMTcgYXQgNzoyMyBBTSwgTHVjYXMgU3RhY2ggPGwuc3RhY2hAcGVu Z3V0cm9uaXguZAo+ID4gZT4gd3JvdGU6Cj4gPiA+IEhpIGFsbCwKPiA+ID4gCj4gPiA+IHRoaXMg c21hbGwgc2VyaWVzIHRyaWVzIHRvIGZpeC93b3JrYXJvdW5kIGEgc2VyaW91cyBkZXNpZ24gZmxh dwo+ID4gPiBvZiB0aGUgRFdDIFBDSWUKPiA+ID4gaG9zdCBjb250cm9sbGVyOiBpdCBpcyB1bmFi bGUgdG8gd29yayB3aXRoIGJvdGggbGVnYWN5IGFuZCBNU0kKPiA+ID4gSVJRcyBlbmFibGVkIGF0 Cj4gPiA+IHRoZSBzYW1lIHRpbWUuIEFzIHNvb24gYXMgdGhlIGZpcnN0IE1TSSBpcyBlbmFibGVk IGluIHRoZSBEV0MgTVNJCj4gPiA+IGNvbnRyb2xsZXIsCj4gPiA+IHRoZSBob3N0IHN0b3BzIGZv cndhcmRpbmcgbGVnYWN5IElSUXMuCj4gPiA+IAo+ID4gPiBJZiB0aGUgTVNJIGNvbnRyb2xsZXIg aXMgcHJlc2VudCwgTVNJcyB3aWxsIGJlIHVzZWQgZm9yIHRoZSBQQ0llCj4gPiA+IHBvcnQgc2Vy dmljZXMKPiA+ID4gSVJRcywgbGVhdmluZyBlbmRwb2ludCBkZXZpY2VzIHdoaWNoIGRvbid0IHN1 cHBvcnQgTVNJcyB1bmFibGUgdG8KPiA+ID4gcmFpc2UgSVJRcy4KPiA+ID4gSXQgaXMgb25seSBz YWZlIHRvIGVuYWJsZSB0aGUgTVNJIGNvbnRyb2xsZXIgaWYgaXQgaXMgdmFsaWRhdGVkCj4gPiA+ IHRoYXQgYWxsIFBDSWUKPiA+ID4gZGV2aWNlcyBhbmQgZHJpdmVycyBpbiB0aGUgc3lzdGVtIHN1 cHBvcnQgd29ya2luZyBNU0lzLiBBcyBtb3N0Cj4gPiA+IGRldmljZXMKPiA+ID4gc3VwcG9ydCBm YWxsaW5nIGJhY2sgdG8gdXNpbmcgbGVnYWN5IFBDSWUgSVJRcyBpZiBNU0kgc3VwcG9ydCBpcwo+ ID4gPiBtaXNzaW5nIGl0IGlzCj4gPiA+IG11Y2ggc2FmZXIgdG8gZGlzYWJsZSB0aGUgTVNJIGJ5 IGRlZmF1bHQgYW5kIG9ubHkgZW5hYmxlIGl0IG9uCj4gPiA+IHZhbGlkYXRlZAo+ID4gPiBzeXN0 ZW1zLgo+ID4gPiAKPiA+ID4gRmVlZGJhY2sgd2VsY29tZS4KPiA+ID4gCj4gPiA+IFJlZ2FyZHMs Cj4gPiA+IEx1Y2FzCj4gPiA+IAo+ID4gPiBMdWNhcyBTdGFjaCAoMyk6Cj4gPiA+IMKgIFBDSTog ZGVzaWdud2FyZTogb25seSByZWdpc3RlciBNU0kgY29udHJvbGxlciB3aGVuIE1TSSBpcnEgbGlu ZQo+ID4gPiBpcwo+ID4gPiDCoMKgwqDCoHZhbGlkCj4gPiA+IMKgIFBDSTogaW14NjogYWxsb3cg TVNJIGlycSB0byBiZSBhYnNlbnQKPiA+ID4gwqAgQVJNOiBkdHM6IGlteDZxZGw6IHJlbW92ZSBN U0kgaXJxIGxpbmUKPiA+ID4gCj4gPiA+IMKgLi4uL2RldmljZXRyZWUvYmluZGluZ3MvcGNpL2Zz bCxpbXg2cS1wY2llLnR4dMKgwqDCoMKgwqB8wqDCoDggKysrKy0tLS0KPiA+ID4gwqBhcmNoL2Fy bS9ib290L2R0cy9pbXg2cWRsLmR0c2nCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqB8wqDCoDIgLS0KPiA+ID4gwqBkcml2ZXJzL3BjaS9kd2MvcGNpLWlteDYuY8KgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgfCAyMwo+ID4gPiAr KysrKysrKysrKy0tLS0tLS0tLS0tCj4gPiA+IMKgZHJpdmVycy9wY2kvZHdjL3BjaWUtZGVzaWdu d2FyZS1ob3N0LmPCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHzCoMKgNCArKy0tCj4gPiA+IMKg NCBmaWxlcyBjaGFuZ2VkLCAxNyBpbnNlcnRpb25zKCspLCAyMCBkZWxldGlvbnMoLSkKPiA+ID4g Cj4gPiAKPiA+IEx1Y2FzLAo+ID4gCj4gPiBUaGFuayB5b3UgZm9yIGZvbGxvd2luZyB1cCBvbiB0 aGlzIQo+ID4gCj4gPiBJIHRlc3RlZCBpdCB3aXRoIGFuZCB3aXRob3V0IHRoZSB0aGlyZCBwYXRj aCB0aGF0IHJlbW92ZXMgbXNpIGZyb20KPiA+IHRoZQo+ID4gZHQgdGh1cyB3aXRoIGJvdGggYW4g YXRoOWsgODAyLjExIGRldmljZSB3aGljaCBvbmx5IHN1cHBvcnRzIGxlZ2FjeQo+ID4gaW50ZXJy dXB0cyBhbmQgYSBNYXJ2ZWxsIHNreTIgZGV2aWNlIHdoaWNoIHN1cHBvcnRzIG1zaSBhcyB3ZWxs IGFzCj4gPiBsZWdhY3kgaW50ZXJydXB0cyBhbmQgaXQgd29ya3MgYXMgZXhwZWN0ZWQ6Cj4gPiDC oC0gd2l0aG91dCBtc2kgZW5hYmxlZCBpbiBkdHMgKGRlZmF1bHQpOiBib3RoIHNreTIgYW5kIGF0 aDlrIHdvcmsKPiA+IMKgLSB3aXRoIG1zaSBlbmFibGVkIGluIGR0OiBza3kyIHdvcmtzIGF0aDlr IGRvZXMgbm90Cj4gPiAKPiA+IFRlc3RlZC1ieTogVGltIEhhcnZleSA8dGhhcnZleUBnYXRld29y a3MuY29tPgo+IAo+IEFueSBjb21tZW50cyBhYm91dCB0aGlzIHNlcmllcz8KCkJqb3JuIGFscmVh ZHkgY29tbWVudGVkIG9uIHRoZSBzZXJpZXMgYW5kIEknbSB0cnlpbmcgZnJlZSB1cCBhIHNsb3Qg dG8KYWRkcmVzcyB0aGUgZmVlZGJhY2suCgpSZWdhcmRzLApMdWNhcwoKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5lbCBtYWlsaW5n IGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5p bmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo= ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] DWC host without MSI controller @ 2017-10-09 12:22 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2017-10-09 12:22 UTC (permalink / raw) To: linux-arm-kernel Hi Fabio, Am Montag, den 09.10.2017, 09:14 -0300 schrieb Fabio Estevam: > Hi Bjorn, > > On Mon, Aug 28, 2017 at 1:59 PM, Tim Harvey <tharvey@gateworks.com> > wrote: > > On Mon, Aug 28, 2017 at 7:23 AM, Lucas Stach <l.stach@pengutronix.d > > e> wrote: > > > Hi all, > > > > > > this small series tries to fix/workaround a serious design flaw > > > of the DWC PCIe > > > host controller: it is unable to work with both legacy and MSI > > > IRQs enabled at > > > the same time. As soon as the first MSI is enabled in the DWC MSI > > > controller, > > > the host stops forwarding legacy IRQs. > > > > > > If the MSI controller is present, MSIs will be used for the PCIe > > > port services > > > IRQs, leaving endpoint devices which don't support MSIs unable to > > > raise IRQs. > > > It is only safe to enable the MSI controller if it is validated > > > that all PCIe > > > devices and drivers in the system support working MSIs. As most > > > devices > > > support falling back to using legacy PCIe IRQs if MSI support is > > > missing it is > > > much safer to disable the MSI by default and only enable it on > > > validated > > > systems. > > > > > > Feedback welcome. > > > > > > Regards, > > > Lucas > > > > > > Lucas Stach (3): > > > ? PCI: designware: only register MSI controller when MSI irq line > > > is > > > ????valid > > > ? PCI: imx6: allow MSI irq to be absent > > > ? ARM: dts: imx6qdl: remove MSI irq line > > > > > > ?.../devicetree/bindings/pci/fsl,imx6q-pcie.txt?????|??8 ++++---- > > > ?arch/arm/boot/dts/imx6qdl.dtsi?????????????????????|??2 -- > > > ?drivers/pci/dwc/pci-imx6.c?????????????????????????| 23 > > > +++++++++++----------- > > > ?drivers/pci/dwc/pcie-designware-host.c?????????????|??4 ++-- > > > ?4 files changed, 17 insertions(+), 20 deletions(-) > > > > > > > Lucas, > > > > Thank you for following up on this! > > > > I tested it with and without the third patch that removes msi from > > the > > dt thus with both an ath9k 802.11 device which only supports legacy > > interrupts and a Marvell sky2 device which supports msi as well as > > legacy interrupts and it works as expected: > > ?- without msi enabled in dts (default): both sky2 and ath9k work > > ?- with msi enabled in dt: sky2 works ath9k does not > > > > Tested-by: Tim Harvey <tharvey@gateworks.com> > > Any comments about this series? Bjorn already commented on the series and I'm trying free up a slot to address the feedback. Regards, Lucas ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-10-09 12:22 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-28 14:23 [PATCH 0/3] DWC host without MSI controller Lucas Stach 2017-08-28 14:23 ` Lucas Stach 2017-08-28 14:23 ` [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid Lucas Stach 2017-08-28 14:23 ` Lucas Stach 2017-08-31 16:58 ` Bjorn Helgaas 2017-08-31 16:58 ` Bjorn Helgaas 2017-08-31 17:32 ` Fabio Estevam 2017-08-31 17:32 ` Fabio Estevam 2017-09-25 23:54 ` Bjorn Helgaas 2017-09-25 23:54 ` Bjorn Helgaas 2017-08-28 14:23 ` [PATCH 2/3] PCI: imx6: allow MSI irq to be absent Lucas Stach 2017-08-28 14:23 ` Lucas Stach 2017-08-28 14:23 ` [PATCH 3/3] ARM: dts: imx6qdl: remove MSI irq line Lucas Stach 2017-08-28 14:23 ` Lucas Stach 2017-08-28 16:59 ` [PATCH 0/3] DWC host without MSI controller Tim Harvey 2017-08-28 16:59 ` Tim Harvey 2017-10-09 12:14 ` Fabio Estevam 2017-10-09 12:14 ` Fabio Estevam 2017-10-09 12:22 ` Lucas Stach 2017-10-09 12:22 ` 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.