[PATCH v2 0/6] PCI: mvebu: cleanup and improvements

[PATCH v2 0/6] PCI: mvebu: cleanup and improvements

[Thomas Petazzoni]

Hello,

This is the v2 of the pci-mvebu cleanup and improvements, mainly
aiming at using pci_host_bridge.

Since v1, the changes are:

 - Rebase on v4.18-rc1 to make sure the patches apply

 - Add a patch fixing the I/O space end calculation (using
   resource_size() as the .end address of a resource is not correct)

 - Only remap the I/O space if there is a PCI I/O aperture described
   in the DT.

 - Use resource_size() in the loop mapping the I/O space, as suggested
   by Lorenzo.

 - Use pci_add_resource() instead of pci_add_resource_offset(), as
   suggested by Lorenzo.

 - Revert to using pci_ioremap_io(), since we need to map with a
   special memory type on Armada platforms, and pci_remap_iospace()
   doesn't allow to do that. Noticed by Lorenzo.

Thanks!

Thomas

Thomas Petazzoni (6):
  PCI: mvebu: Remove redundant platform_set_drvdata() call
  PCI: mvebu: fix I/O space end address calculation
  PCI: mvebu: only remap I/O space if configured
  PCI: mvebu: use resource_size() to remap I/O space
  PCI: mvebu: Convert to use pci_host_bridge directly
  PCI: mvebu: Drop bogus comment above mvebu_pcie_map_registers()

 drivers/pci/controller/pci-mvebu.c | 153 +++++++++++++++++--------------------
 1 file changed, 68 insertions(+), 85 deletions(-)

-- 
2.14.4

[PATCH v2 1/6] PCI: mvebu: Remove redundant platform_set_drvdata() call

[Thomas Petazzoni]

This is already done earlier in mvebu_pcie_probe().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-mvebu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 23e270839e6a..38fa2d08527d 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -1283,8 +1283,6 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 
 	mvebu_pcie_enable(pcie);
 
-	platform_set_drvdata(pdev, pcie);
-
 	return 0;
 }
 
-- 
2.14.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/6] PCI: mvebu: fix I/O space end address calculation

[Thomas Petazzoni]

pcie->realio.end should be the address of last byte of the area,
therefore using resource_size() of another resource is not correct, we
must substract 1 to get the address of the last byte.

Fixes: 11be65472a427 ("PCI: mvebu: Adapt to the new device tree layout")
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
This bug never had any visible impact, so there is no need to push to
stable.
---
 drivers/pci/controller/pci-mvebu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 38fa2d08527d..a195592723c2 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -1219,7 +1219,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 		pcie->realio.start = PCIBIOS_MIN_IO;
 		pcie->realio.end = min_t(resource_size_t,
 					 IO_SPACE_LIMIT,
-					 resource_size(&pcie->io));
+					 resource_size(&pcie->io) - 1);
 	} else
 		pcie->realio = pcie->io;
 
-- 
2.14.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/6] PCI: mvebu: only remap I/O space if configured

[Thomas Petazzoni]

If there is no PCI I/O aperture configured in the Device Tree, it does
not make sense to create the virtual mapping for the PCI I/O space,
since we will anyway not create the MBus window that will allow to
access it. Therefore, do the pci_ioremap_io() only if necessary.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-mvebu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index a195592723c2..9aa224f2f009 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -1220,6 +1220,9 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 		pcie->realio.end = min_t(resource_size_t,
 					 IO_SPACE_LIMIT,
 					 resource_size(&pcie->io) - 1);
+
+		for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
+			pci_ioremap_io(i, pcie->io.start + i);
 	} else
 		pcie->realio = pcie->io;
 
@@ -1278,9 +1281,6 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 
 	pcie->nports = i;
 
-	for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
-		pci_ioremap_io(i, pcie->io.start + i);
-
 	mvebu_pcie_enable(pcie);
 
 	return 0;
-- 
2.14.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/6] PCI: mvebu: use resource_size() to remap I/O space

[Thomas Petazzoni]

Instead of hardcoding the remapping of IO_SPACE_LIMIT - SZ_64K, use
resource_size(), as suggested by Lorenzo Pieralisi.

However, we cannot use just IO_SPACE_LIMIT, because pci_ioremap_io()
has a bug (which will be fixed separately) and doesn't allow remapping
the last 64 KB before IO_SPACE_LIMIT, so we ensure that we do not
exceed this limit. A separate patch will be sent to fix the
pci_ioremap_io() issue, and once it is merged, we will be able to drop
this work-around. Note that this workaround already existed, since we
were mapping only up to IO_SPACE_LIMIT - SZ_64K.

Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 9aa224f2f009..05f863435e5e 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -1218,10 +1218,10 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 		pcie->realio.flags = pcie->io.flags;
 		pcie->realio.start = PCIBIOS_MIN_IO;
 		pcie->realio.end = min_t(resource_size_t,
-					 IO_SPACE_LIMIT,
+					 IO_SPACE_LIMIT - SZ_64K,
 					 resource_size(&pcie->io) - 1);
 
-		for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
+		for (i = 0; i < resource_size(&pcie->realio); i += SZ_64K)
 			pci_ioremap_io(i, pcie->io.start + i);
 	} else
 		pcie->realio = pcie->io;
-- 
2.14.4

[PATCH v2 5/6] PCI: mvebu: Convert to use pci_host_bridge directly

[Thomas Petazzoni]

Rather than using the ARM-specific pci_common_init_dev() API, use the
pci_host_bridge logic directly.

Unfortunately, we can't use devm_of_pci_get_host_bridge_resources(),
because the DT binding for describing PCIe apertures for this PCI
controller is a bit special, and we cannot retrieve them from the
'ranges' property. Therefore, we still have some special code to
handle this.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-mvebu.c | 136 +++++++++++++++++--------------------
 1 file changed, 63 insertions(+), 73 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 05f863435e5e..9055f03596ef 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -125,6 +125,7 @@ struct mvebu_pcie {
 	struct platform_device *pdev;
 	struct mvebu_pcie_port *ports;
 	struct msi_controller *msi;
+	struct list_head resources;
 	struct resource io;
 	struct resource realio;
 	struct resource mem;
@@ -800,7 +801,7 @@ static struct mvebu_pcie_port *mvebu_pcie_find_port(struct mvebu_pcie *pcie,
 static int mvebu_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			      int where, int size, u32 val)
 {
-	struct mvebu_pcie *pcie = sys_to_pcie(bus->sysdata);
+	struct mvebu_pcie *pcie = bus->sysdata;
 	struct mvebu_pcie_port *port;
 	int ret;
 
@@ -826,7 +827,7 @@ static int mvebu_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			      int size, u32 *val)
 {
-	struct mvebu_pcie *pcie = sys_to_pcie(bus->sysdata);
+	struct mvebu_pcie *pcie = bus->sysdata;
 	struct mvebu_pcie_port *port;
 	int ret;
 
@@ -857,36 +858,6 @@ static struct pci_ops mvebu_pcie_ops = {
 	.write = mvebu_pcie_wr_conf,
 };
 
-static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys)
-{
-	struct mvebu_pcie *pcie = sys_to_pcie(sys);
-	int err, i;
-
-	pcie->mem.name = "PCI MEM";
-	pcie->realio.name = "PCI I/O";
-
-	if (resource_size(&pcie->realio) != 0)
-		pci_add_resource_offset(&sys->resources, &pcie->realio,
-					sys->io_offset);
-
-	pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset);
-	pci_add_resource(&sys->resources, &pcie->busn);
-
-	err = devm_request_pci_bus_resources(&pcie->pdev->dev, &sys->resources);
-	if (err)
-		return 0;
-
-	for (i = 0; i < pcie->nports; i++) {
-		struct mvebu_pcie_port *port = &pcie->ports[i];
-
-		if (!port->base)
-			continue;
-		mvebu_pcie_setup_hw(port);
-	}
-
-	return 1;
-}
-
 static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
 						 const struct resource *res,
 						 resource_size_t start,
@@ -917,26 +888,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
 		return start;
 }
 
-static void mvebu_pcie_enable(struct mvebu_pcie *pcie)
-{
-	struct hw_pci hw;
-
-	memset(&hw, 0, sizeof(hw));
-
-#ifdef CONFIG_PCI_MSI
-	hw.msi_ctrl = pcie->msi;
-#endif
-
-	hw.nr_controllers = 1;
-	hw.private_data   = (void **)&pcie;
-	hw.setup          = mvebu_pcie_setup;
-	hw.map_irq        = of_irq_parse_and_map_pci;
-	hw.ops            = &mvebu_pcie_ops;
-	hw.align_resource = mvebu_pcie_align_resource;
-
-	pci_common_init_dev(&pcie->pdev->dev, &hw);
-}
-
 /*
  * Looks up the list of register addresses encoded into the reg =
  * <...> property for one that matches the given port/lane. Once
@@ -1190,28 +1141,39 @@ static void mvebu_pcie_powerdown(struct mvebu_pcie_port *port)
 	clk_disable_unprepare(port->clk);
 }
 
-static int mvebu_pcie_probe(struct platform_device *pdev)
+/*
+ * We can't use devm_of_pci_get_host_bridge_resources() because we
+ * need to parse our special DT properties encoding the MEM and IO
+ * apertures.
+ */
+static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
 {
-	struct device *dev = &pdev->dev;
-	struct mvebu_pcie *pcie;
+	struct device *dev = &pcie->pdev->dev;
 	struct device_node *np = dev->of_node;
-	struct device_node *child;
-	int num, i, ret;
+	unsigned int i;
+	int ret;
 
-	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
-	if (!pcie)
-		return -ENOMEM;
+	INIT_LIST_HEAD(&pcie->resources);
 
-	pcie->pdev = pdev;
-	platform_set_drvdata(pdev, pcie);
+	/* Get the bus range */
+	ret = of_pci_parse_bus_range(np, &pcie->busn);
+	if (ret) {
+		dev_err(dev, "failed to parse bus-range property: %d\n", ret);
+		return ret;
+	}
+	pci_add_resource(&pcie->resources, &pcie->busn);
 
-	/* Get the PCIe memory and I/O aperture */
+	/* Get the PCIe memory aperture */
 	mvebu_mbus_get_pcie_mem_aperture(&pcie->mem);
 	if (resource_size(&pcie->mem) == 0) {
 		dev_err(dev, "invalid memory aperture size\n");
 		return -EINVAL;
 	}
 
+	pcie->mem.name = "PCI MEM";
+	pci_add_resource(&pcie->resources, &pcie->mem);
+
+	/* Get the PCIe IO aperture */
 	mvebu_mbus_get_pcie_io_aperture(&pcie->io);
 
 	if (resource_size(&pcie->io) != 0) {
@@ -1220,19 +1182,38 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 		pcie->realio.end = min_t(resource_size_t,
 					 IO_SPACE_LIMIT - SZ_64K,
 					 resource_size(&pcie->io) - 1);
+		pcie->realio.name = "PCI I/O";
 
 		for (i = 0; i < resource_size(&pcie->realio); i += SZ_64K)
 			pci_ioremap_io(i, pcie->io.start + i);
-	} else
-		pcie->realio = pcie->io;
 
-	/* Get the bus range */
-	ret = of_pci_parse_bus_range(np, &pcie->busn);
-	if (ret) {
-		dev_err(dev, "failed to parse bus-range property: %d\n", ret);
-		return ret;
+		pci_add_resource(&pcie->resources, &pcie->realio);
 	}
 
+	return devm_request_pci_bus_resources(dev, &pcie->resources);
+}
+
+static int mvebu_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mvebu_pcie *pcie;
+	struct pci_host_bridge *bridge;
+	struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int num, i, ret;
+
+	bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct mvebu_pcie));
+	if (!bridge)
+		return -ENOMEM;
+
+	pcie = pci_host_bridge_priv(bridge);
+	pcie->pdev = pdev;
+	platform_set_drvdata(pdev, pcie);
+
+	ret = mvebu_pcie_parse_request_resources(pcie);
+	if (ret)
+		return ret;
+
 	num = of_get_available_child_count(np);
 
 	pcie->ports = devm_kcalloc(dev, num, sizeof(*pcie->ports), GFP_KERNEL);
@@ -1275,15 +1256,24 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 			continue;
 		}
 
+		mvebu_pcie_setup_hw(port);
 		mvebu_pcie_set_local_dev_nr(port, 1);
 		mvebu_sw_pci_bridge_init(port);
 	}
 
 	pcie->nports = i;
 
-	mvebu_pcie_enable(pcie);
-
-	return 0;
+	list_splice_init(&pcie->resources, &bridge->windows);
+	bridge->dev.parent = dev;
+	bridge->sysdata = pcie;
+	bridge->busnr = 0;
+	bridge->ops = &mvebu_pcie_ops;
+	bridge->map_irq = of_irq_parse_and_map_pci;
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->align_resource = mvebu_pcie_align_resource;
+	bridge->msi = pcie->msi;
+
+	return pci_host_probe(bridge);
 }
 
 static const struct of_device_id mvebu_pcie_of_match_table[] = {
-- 
2.14.4

[PATCH v2 6/6] PCI: mvebu: Drop bogus comment above mvebu_pcie_map_registers()

[Thomas Petazzoni]

This comment has been there since the driver was introduced, but seems
to be a leftover from previous iterations of the driver. Indeed, we do
not lookup in a list to find the register ranges that matches the
given port/lane, as the "reg" property is in each sub-node
representing a PCI port. There is no lookup involved at all.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/pci/controller/pci-mvebu.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 9055f03596ef..50eb0729385b 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -888,11 +888,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
 		return start;
 }
 
-/*
- * Looks up the list of register addresses encoded into the reg =
- * <...> property for one that matches the given port/lane. Once
- * found, maps it.
- */
 static void __iomem *mvebu_pcie_map_registers(struct platform_device *pdev,
 					      struct device_node *np,
 					      struct mvebu_pcie_port *port)
-- 
2.14.4

Re: [PATCH v2 0/6] PCI: mvebu: cleanup and improvements

[Lorenzo Pieralisi]

On Fri, Aug 03, 2018 at 04:38:42PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> This is the v2 of the pci-mvebu cleanup and improvements, mainly
> aiming at using pci_host_bridge.
> 
> Since v1, the changes are:
> 
>  - Rebase on v4.18-rc1 to make sure the patches apply
> 
>  - Add a patch fixing the I/O space end calculation (using
>    resource_size() as the .end address of a resource is not correct)
> 
>  - Only remap the I/O space if there is a PCI I/O aperture described
>    in the DT.
> 
>  - Use resource_size() in the loop mapping the I/O space, as suggested
>    by Lorenzo.
> 
>  - Use pci_add_resource() instead of pci_add_resource_offset(), as
>    suggested by Lorenzo.
> 
>  - Revert to using pci_ioremap_io(), since we need to map with a
>    special memory type on Armada platforms, and pci_remap_iospace()
>    doesn't allow to do that. Noticed by Lorenzo.

Pulled into pci/mvebu (I reworded some commit logs in the process
please have a look) for (tentatively) v4.19 but it is a bit late
in the cycle so let's see how things go.

Thanks,
Lorenzo

> 
> Thanks!
> 
> Thomas
> 
> Thomas Petazzoni (6):
>   PCI: mvebu: Remove redundant platform_set_drvdata() call
>   PCI: mvebu: fix I/O space end address calculation
>   PCI: mvebu: only remap I/O space if configured
>   PCI: mvebu: use resource_size() to remap I/O space
>   PCI: mvebu: Convert to use pci_host_bridge directly
>   PCI: mvebu: Drop bogus comment above mvebu_pcie_map_registers()
> 
>  drivers/pci/controller/pci-mvebu.c | 153 +++++++++++++++++--------------------
>  1 file changed, 68 insertions(+), 85 deletions(-)
> 
> -- 
> 2.14.4
>