[PATCH 0/3] pcie-designware: support for external MSI controller

Hello,

This short patch series extends the pcie-designware code to support
using an external MSI controller, rather than the built-in MSI
controller. It does so by making the pcie-designware driver handle the
"msi-parent" DT property.

The first patch updates the Device Tree binding documentation, the
second patch makes the change to the Designware PCIe controller driver
itself, and finally the third patch updates the Marvell Armada 7K/8K
Device Tree to use the msi-parent property.

Thanks,

Thomas

Thomas Petazzoni (3):
  dt-bindings: designware-pcie: document optional msi-parent property
  pci: pcie-designware: add support for external MSI controller
  arm64: dts: marvell: enable MSI for PCIe on Armada 7K/8K

 .../devicetree/bindings/pci/designware-pcie.txt    |  2 ++
 .../boot/dts/marvell/armada-cp110-master.dtsi      |  3 +++
 .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi |  3 +++
 drivers/pci/host/pcie-designware.c                 | 24 ++++++++++++++++------
 4 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.7.4

The Designware PCIe controllers have a built-in MSI controller, which is
already supported by the existing. However, in some situations, it might
be a better choice to use an external MSI controller, especially when it
provides a higher number of MSI interrupts than the built-in one.

Therefore, this commit extends the pcie-designware driver to support the
"msi-parent" DT property, already used by other drivers. It contains a
phandle pointing to the external MSI controller to be used.

Following this commit, the pcie-designware code supports three
possibilities, in this order:

 1. If msi-parent is provided, then the MSI controller pointed by this
    property is used.

 2. Otherwise, and if no ->msi_host_init() function is provided by the
    platform-specific "glue", then the built-in MSI controller of the
    Designware controller is used.

 3. Otherwise, the ->msi_host_init() function of the platform-specific
    "glue" is used to do some additional initialization, but it's still
    the built-in MSI controller that is used.

Signed-off-by: Thomas Petazzoni <redacted>
---
 drivers/pci/host/pcie-designware.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 12afce1..1e18a85 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c

@@ -437,6 +437,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	int i, ret;
 	LIST_HEAD(res);
 	struct resource_entry *win;
+	struct msi_controller *msi = NULL;
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
 	if (cfg_res) {

@@ -525,10 +526,21 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		pp->lanes = 0;
 
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		if (!pp->ops->msi_host_init) {
+		if (of_find_property(pp->dev->of_node, "msi-parent", NULL)) {
+			struct device_node *msi_node;
+
+			msi_node = of_parse_phandle(pp->dev->of_node,
+						    "msi-parent", 0);
+			if (!msi_node)
+				return -ENODEV;
+
+			msi = of_pci_find_msi_chip_by_node(msi_node);
+		} else if (!pp->ops->msi_host_init) {
+			msi = &dw_pcie_msi_chip;
+			msi->dev = pp->dev;
 			pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
 						MAX_MSI_IRQS, &msi_domain_ops,
-						&dw_pcie_msi_chip);
+						msi);
 			if (!pp->irq_domain) {
 				dev_err(pp->dev, "irq domain init failed\n");
 				ret = -ENXIO;

@@ -538,7 +550,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			for (i = 0; i < MAX_MSI_IRQS; i++)
 				irq_create_mapping(pp->irq_domain, i);
 		} else {
-			ret = pp->ops->msi_host_init(pp, &dw_pcie_msi_chip);
+			msi = &dw_pcie_msi_chip;
+			msi->dev = pp->dev;
+			ret = pp->ops->msi_host_init(pp, msi);
 			if (ret < 0)
 				goto error;
 		}

@@ -550,9 +564,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	pp->root_bus_nr = pp->busn->start;
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		bus = pci_scan_root_bus_msi(pp->dev, pp->root_bus_nr,
-					    &dw_pcie_ops, pp, &res,
-					    &dw_pcie_msi_chip);
-		dw_pcie_msi_chip.dev = pp->dev;
+					    &dw_pcie_ops, pp, &res, msi);
 	} else
 		bus = pci_scan_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
 					pp, &res);

-- 
2.7.4

On Tue, Aug 30, 2016 at 02:57:09PM +0200, Thomas Petazzoni wrote:

The Designware PCIe controllers have a built-in MSI controller, which is
already supported by the existing. However, in some situations, it might

                                   ^ driver.

be a better choice to use an external MSI controller, especially when it
provides a higher number of MSI interrupts than the built-in one.

Therefore, this commit extends the pcie-designware driver to support the
"msi-parent" DT property, already used by other drivers. It contains a
phandle pointing to the external MSI controller to be used.

Following this commit, the pcie-designware code supports three
possibilities, in this order:

 1. If msi-parent is provided, then the MSI controller pointed by this

                                                                 ^ to  

    property is used.

 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		if (!pp->ops->msi_host_init) {
+		if (of_find_property(pp->dev->of_node, "msi-parent", NULL)) {
+			struct device_node *msi_node;
+
+			msi_node = of_parse_phandle(pp->dev->of_node,
+						    "msi-parent", 0);
+			if (!msi_node)
+				return -ENODEV;
+
+			msi = of_pci_find_msi_chip_by_node(msi_node);

By this point, device tree tells us the external MSI controller
should exist. So if we get a NULL here, should we not return
-EPROBE_DIFFERED?

	Andrew

Hello,

On Tue, 30 Aug 2016 14:57:07 +0200, Thomas Petazzoni wrote:

Thomas Petazzoni (3):
  dt-bindings: designware-pcie: document optional msi-parent property
  pci: pcie-designware: add support for external MSI controller
  arm64: dts: marvell: enable MSI for PCIe on Armada 7K/8K

Please discard this patch series, it's just pure crap. No changes at
all are needed on the pcie-designware driver, using the msi-parent
property already works, and my new code does nothing good.

The of_pci_find_msi_chip_by_node() API is on its way to being removed,
and it doesn't return anything useful.

Only PATCH 3/3 is needed, which I'll resubmit separately.

Sorry for the noise.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

On Tuesday, August 30, 2016 11:25 AM, Thomas Petazzoni wrote:

Hello,

On Tue, 30 Aug 2016 14:57:07 +0200, Thomas Petazzoni wrote:

quoted

Thomas Petazzoni (3):
  dt-bindings: designware-pcie: document optional msi-parent property
  pci: pcie-designware: add support for external MSI controller
  arm64: dts: marvell: enable MSI for PCIe on Armada 7K/8K

Please discard this patch series, it's just pure crap. No changes at all
are needed on the pcie-designware driver, using the msi-parent property
already works, and my new code does nothing good.

The of_pci_find_msi_chip_by_node() API is on its way to being removed, and
it doesn't return anything useful.

Only PATCH 3/3 is needed, which I'll resubmit separately.

Sorry for the noise.

Oh, good.
The support for External MSI controller is really necessary.
Thank you for your patch!

Best regards,
Jingoo Han

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Hello,

On Tue, 30 Aug 2016 11:46:13 -0400, Jingoo Han wrote:

quoted

Please discard this patch series, it's just pure crap. No changes at all
are needed on the pcie-designware driver, using the msi-parent property
already works, and my new code does nothing good.

The of_pci_find_msi_chip_by_node() API is on its way to being removed, and
it doesn't return anything useful.

Only PATCH 3/3 is needed, which I'll resubmit separately.

Sorry for the noise.  

Oh, good.
The support for External MSI controller is really necessary.
Thank you for your patch!

Well, no change in the driver is needed to use an external MSI
controller. Just specify:

	msi-parent = <&....>

in the DT description of your Designware PCIe controller. And that's
it. I've tested it with the GIC v2m MSI controller on an ARM64
platform, and it works as expected.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com