[PATCH 0/3] DWC host without MSI controller

[PATCH 0/3] DWC host without MSI controller

[Lucas Stach]

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

[PATCH 0/3] DWC host without MSI controller

[Lucas Stach]

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

[PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Lucas Stach]

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

[PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Lucas Stach]

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

[PATCH 2/3] PCI: imx6: allow MSI irq to be absent

[Lucas Stach]

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

[PATCH 2/3] PCI: imx6: allow MSI irq to be absent

[Lucas Stach]

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

[PATCH 3/3] ARM: dts: imx6qdl: remove MSI irq line

[Lucas Stach]

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

[PATCH 3/3] ARM: dts: imx6qdl: remove MSI irq line

[Lucas Stach]

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

Re: [PATCH 0/3] DWC host without MSI controller

[Tim Harvey]

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

[PATCH 0/3] DWC host without MSI controller

[Tim Harvey]

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

Re: [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Bjorn Helgaas]

[+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

[PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Bjorn Helgaas]

[+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

Re: [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Fabio Estevam]

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.

[PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Fabio Estevam]

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.

Re: [PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Bjorn Helgaas]

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

[PATCH 1/3] PCI: designware: only register MSI controller when MSI irq line is valid

[Bjorn Helgaas]

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

Re: [PATCH 0/3] DWC host without MSI controller

[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.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

[PATCH 0/3] DWC host without MSI controller

[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.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?

Re: [PATCH 0/3] DWC host without MSI controller

[Lucas Stach]

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=

[PATCH 0/3] DWC host without MSI controller

[Lucas Stach]

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