[PATCH v4 0/8] PCI: leak fixes, removable generic PCI host, assorted stuff

[PATCH v4 0/8] PCI: leak fixes, removable generic PCI host, assorted stuff

[Jan Kiszka]

Changes in v4:
 - restore pci_free_resource_list() in error path of
   of_pci_get_host_bridge_resources()

Changes in v3:
 - refactor series to be both bisectable and simpler while reworking
   of_pci_get_host_bridge_resources()
 - include of_pci_get_host_bridge_resources() removal
 - include devm_of_pci_get_host_bridge_resources() error path fixes
 - effectively, no functional changes to v2

Changes in v2:
 - patch 1: commit message reworking as suggested by Lorenzo
 - patch 3-6: split-up as suggested by Bjorn
 - patch 8: new
 - patch 10: select PCI_DOMAINS from PCI_HOST_GENERIC, rather than
   allowing manual choice, as suggested by Lorenzo

This primarily enables to unbind the generic PCI host controller without
leaving lots of memory leaks behind. A previous proposal patch 5 was
rejected because of those issues [1].

The fixes have been validated in the Jailhouse setup, where we add and
remove a virtual PCI host controller on hypervisor activation/
deactivation, with the help of kmemleak.

Besides that, there is tiny PCI API cleanup at the beginning and
support for manually enabled PCI domains at the end that enables the
Jailhouse scenario.

Jan

[1] http://lkml.iu.edu/hypermail/linux/kernel/1606.3/00072.html


CC: Jingoo Han <jingoohan1@gmail.com>
CC: Joao Pinto <Joao.Pinto@synopsys.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: Will Deacon <will.deacon@arm.com>

Jan Kiszka (8):
  PCI: Make pci_get_new_domain_nr() static
  PCI: Fix memory leak of devm_pci_alloc_host_bridge()
  PCI: Rename device node parameter of
    of_pci_get_host_bridge_resources()
  PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources
    with device
  PCI: Replace pr_*() with dev_*() in of_pci_get_host_bridge_resources()
  PCI: Rework of_pci_get_host_bridge_resources() to
    devm_of_pci_get_host_bridge_resources()
  PCI: Add support for unbinding the generic PCI host controller
  PCI: Enable PCI_DOMAINS along with generic PCI host controller

 drivers/pci/dwc/pcie-designware-host.c |  2 +-
 drivers/pci/host/Kconfig               |  1 +
 drivers/pci/host/pci-aardvark.c        |  5 ++-
 drivers/pci/host/pci-ftpci100.c        |  4 +--
 drivers/pci/host/pci-host-common.c     | 13 +++++++
 drivers/pci/host/pci-host-generic.c    |  1 +
 drivers/pci/host/pci-v3-semi.c         |  3 +-
 drivers/pci/host/pci-versatile.c       |  3 +-
 drivers/pci/host/pci-xgene.c           |  3 +-
 drivers/pci/host/pcie-altera.c         |  5 ++-
 drivers/pci/host/pcie-iproc-platform.c |  4 +--
 drivers/pci/host/pcie-rcar.c           |  5 ++-
 drivers/pci/host/pcie-rockchip.c       |  4 +--
 drivers/pci/host/pcie-xilinx-nwl.c     |  4 +--
 drivers/pci/host/pcie-xilinx.c         |  4 +--
 drivers/pci/of.c                       | 65 ++++++++++++++++------------------
 drivers/pci/pci.c                      |  6 ++--
 drivers/pci/probe.c                    |  4 ++-
 include/linux/of_pci.h                 |  4 +--
 include/linux/pci-ecam.h               |  1 +
 include/linux/pci.h                    |  3 --
 21 files changed, 76 insertions(+), 68 deletions(-)

-- 
2.13.6

[PATCH v4 1/8] PCI: Make pci_get_new_domain_nr() static

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

The only user of pci_get_new_domain_nr() is of_pci_bus_find_domain_nr().
Since they are defined in the same compilation unit,
pci_get_new_domain_nr() can be made static, which also simplifies
preprocessor conditionals.

No functional change intended.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/pci/pci.c   | 6 ++----
 include/linux/pci.h | 3 ---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index dbfe7c4f3776..fe28cd76eacd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5719,15 +5719,14 @@ static void pci_no_domains(void)
 #endif
 }
 
-#ifdef CONFIG_PCI_DOMAINS
+#ifdef CONFIG_PCI_DOMAINS_GENERIC
 static atomic_t __domain_nr = ATOMIC_INIT(-1);
 
-int pci_get_new_domain_nr(void)
+static int pci_get_new_domain_nr(void)
 {
 	return atomic_inc_return(&__domain_nr);
 }
 
-#ifdef CONFIG_PCI_DOMAINS_GENERIC
 static int of_pci_bus_find_domain_nr(struct device *parent)
 {
 	static int use_dt_domains = -1;
@@ -5782,7 +5781,6 @@ int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
 			       acpi_pci_bus_find_domain_nr(bus);
 }
 #endif
-#endif
 
 /**
  * pci_ext_cfg_avail - can we access extended PCI config space?
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 73178a2fcee0..963232a6cd2e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1510,12 +1510,10 @@ void pci_cfg_access_unlock(struct pci_dev *dev);
  */
 #ifdef CONFIG_PCI_DOMAINS
 extern int pci_domains_supported;
-int pci_get_new_domain_nr(void);
 #else
 enum { pci_domains_supported = 0 };
 static inline int pci_domain_nr(struct pci_bus *bus) { return 0; }
 static inline int pci_proc_domain(struct pci_bus *bus) { return 0; }
-static inline int pci_get_new_domain_nr(void) { return -ENOSYS; }
 #endif /* CONFIG_PCI_DOMAINS */
 
 /*
@@ -1670,7 +1668,6 @@ static inline struct pci_dev *pci_get_domain_bus_and_slot(int domain,
 
 static inline int pci_domain_nr(struct pci_bus *bus) { return 0; }
 static inline struct pci_dev *pci_dev_get(struct pci_dev *dev) { return NULL; }
-static inline int pci_get_new_domain_nr(void) { return -ENOSYS; }
 
 #define dev_is_pci(d) (false)
 #define dev_is_pf(d) (false)
-- 
2.13.6

[PATCH v4 2/8] PCI: Fix memory leak of devm_pci_alloc_host_bridge()

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

devm_pci_release_host_bridge_dev() failed to release the resource list.

Fixes: 5c3f18cce083 ("PCI: Add devm_pci_alloc_host_bridge() interface")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/probe.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac91b6fd0bcd..eccf204c9160 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -526,12 +526,14 @@ static void devm_pci_release_host_bridge_dev(struct device *dev)
 
 	if (bridge->release_fn)
 		bridge->release_fn(bridge);
+
+	pci_free_resource_list(&bridge->windows);
 }
 
 static void pci_release_host_bridge_dev(struct device *dev)
 {
 	devm_pci_release_host_bridge_dev(dev);
-	pci_free_host_bridge(to_pci_host_bridge(dev));
+	kfree(to_pci_host_bridge(dev));
 }
 
 struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
-- 
2.13.6

[PATCH v4 3/8] PCI: Rename device node parameter of of_pci_get_host_bridge_resources()

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

We will add a real device parameter to this function soon.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/of.c       | 18 +++++++++---------
 include/linux/of_pci.h |  4 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index a28355c273ae..8d4778ef5806 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -245,7 +245,7 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 #if defined(CONFIG_OF_ADDRESS)
 /**
  * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
- * @dev: device node of the host bridge having the range property
+ * @dev_node: device node of the host bridge having the range property
  * @busno: bus number associated with the bridge root bus
  * @bus_max: maximum number of buses for this bridge
  * @resources: list where the range of resources will be added after DT parsing
@@ -262,7 +262,7 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
  * It returns zero if the range parsing has been successful or a standard error
  * value if it failed.
  */
-int of_pci_get_host_bridge_resources(struct device_node *dev,
+int of_pci_get_host_bridge_resources(struct device_node *dev_node,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base)
 {
@@ -281,15 +281,15 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
 	if (!bus_range)
 		return -ENOMEM;
 
-	pr_info("host bridge %pOF ranges:\n", dev);
+	pr_info("host bridge %pOF ranges:\n", dev_node);
 
-	err = of_pci_parse_bus_range(dev, bus_range);
+	err = of_pci_parse_bus_range(dev_node, bus_range);
 	if (err) {
 		bus_range->start = busno;
 		bus_range->end = bus_max;
 		bus_range->flags = IORESOURCE_BUS;
 		pr_info("  No bus range found for %pOF, using %pR\n",
-			dev, bus_range);
+			dev_node, bus_range);
 	} else {
 		if (bus_range->end > bus_range->start + bus_max)
 			bus_range->end = bus_range->start + bus_max;
@@ -297,7 +297,7 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
 	pci_add_resource(resources, bus_range);
 
 	/* Check for ranges property */
-	err = of_pci_range_parser_init(&parser, dev);
+	err = of_pci_range_parser_init(&parser, dev_node);
 	if (err)
 		goto parse_failed;
 
@@ -327,7 +327,7 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
 			goto parse_failed;
 		}
 
-		err = of_pci_range_to_resource(&range, dev, res);
+		err = of_pci_range_to_resource(&range, dev_node, res);
 		if (err) {
 			kfree(res);
 			continue;
@@ -336,13 +336,13 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
 		if (resource_type(res) == IORESOURCE_IO) {
 			if (!io_base) {
 				pr_err("I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
-					dev);
+					dev_node);
 				err = -EINVAL;
 				goto conversion_failed;
 			}
 			if (*io_base != (resource_size_t)OF_BAD_ADDR)
 				pr_warn("More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
-					dev);
+					dev_node);
 			*io_base = range.cpu_addr;
 		}
 
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 091033a6b836..74eec1943ad2 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -71,11 +71,11 @@ of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
 #endif
 
 #if defined(CONFIG_OF_ADDRESS)
-int of_pci_get_host_bridge_resources(struct device_node *dev,
+int of_pci_get_host_bridge_resources(struct device_node *dev_node,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base);
 #else
-static inline int of_pci_get_host_bridge_resources(struct device_node *dev,
+static inline int of_pci_get_host_bridge_resources(struct device_node *dev_node,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base)
 {
-- 
2.13.6

Re: [PATCH v4 3/8] PCI: Rename device node parameter of of_pci_get_host_bridge_resources()

[Vladimir Zapolskiy]

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> We will add a real device parameter to this function soon.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

[PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Another step towards a managed version of
of_pci_get_host_bridge_resources(): Feed in the underlying device,
rather than just the OF node. This will allow to use managed resource
allocation internally later on.

CC: Jingoo Han <jingoohan1@gmail.com>
CC: Joao Pinto <Joao.Pinto@synopsys.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/dwc/pcie-designware-host.c | 2 +-
 drivers/pci/host/pci-aardvark.c        | 5 ++---
 drivers/pci/host/pci-ftpci100.c        | 4 ++--
 drivers/pci/host/pci-v3-semi.c         | 3 ++-
 drivers/pci/host/pci-versatile.c       | 3 +--
 drivers/pci/host/pci-xgene.c           | 3 ++-
 drivers/pci/host/pcie-altera.c         | 5 ++---
 drivers/pci/host/pcie-iproc-platform.c | 4 ++--
 drivers/pci/host/pcie-rcar.c           | 5 ++---
 drivers/pci/host/pcie-rockchip.c       | 4 ++--
 drivers/pci/host/pcie-xilinx-nwl.c     | 4 ++--
 drivers/pci/host/pcie-xilinx.c         | 4 ++--
 drivers/pci/of.c                       | 9 +++++----
 include/linux/of_pci.h                 | 4 ++--
 14 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 6c409079d514..5a535690b7b5 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -342,7 +342,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	if (!bridge)
 		return -ENOMEM;
 
-	ret = of_pci_get_host_bridge_resources(np, 0, 0xff,
+	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
 					&bridge->windows, &pp->io_base);
 	if (ret)
 		return ret;
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index 9abf549631b4..39d8fc2a8a76 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -822,14 +822,13 @@ static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie)
 {
 	int err, res_valid = 0;
 	struct device *dev = &pcie->pdev->dev;
-	struct device_node *np = dev->of_node;
 	struct resource_entry *win, *tmp;
 	resource_size_t iobase;
 
 	INIT_LIST_HEAD(&pcie->resources);
 
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
-					       &iobase);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+						    &pcie->resources, &iobase);
 	if (err)
 		return err;
 
diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 5008fd87956a..5c176f806fe5 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -476,8 +476,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
 	if (IS_ERR(p->base))
 		return PTR_ERR(p->base);
 
-	ret = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
-					       &res, &io_base);
+	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+						    &res, &io_base);
 	if (ret)
 		return ret;
 
diff --git a/drivers/pci/host/pci-v3-semi.c b/drivers/pci/host/pci-v3-semi.c
index 0a4dea796663..f3f39935ac2f 100644
--- a/drivers/pci/host/pci-v3-semi.c
+++ b/drivers/pci/host/pci-v3-semi.c
@@ -791,7 +791,8 @@ static int v3_pci_probe(struct platform_device *pdev)
 	if (IS_ERR(v3->config_base))
 		return PTR_ERR(v3->config_base);
 
-	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &io_base);
+	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+						    &io_base);
 	if (ret)
 		return ret;
 
diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
index 5b3876f5312b..ef33ec0a9e1b 100644
--- a/drivers/pci/host/pci-versatile.c
+++ b/drivers/pci/host/pci-versatile.c
@@ -64,11 +64,10 @@ static int versatile_pci_parse_request_of_pci_ranges(struct device *dev,
 						     struct list_head *res)
 {
 	int err, mem = 1, res_valid = 0;
-	struct device_node *np = dev->of_node;
 	resource_size_t iobase;
 	struct resource_entry *win, *tmp;
 
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, res, &iobase);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, res, &iobase);
 	if (err)
 		return err;
 
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 0a0d7ee6d3c9..88e9a6d315b3 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -632,7 +632,8 @@ static int xgene_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = of_pci_get_host_bridge_resources(dn, 0, 0xff, &res, &iobase);
+	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+						    &iobase);
 	if (ret)
 		return ret;
 
diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
index a6af62e0256d..61802e55a00c 100644
--- a/drivers/pci/host/pcie-altera.c
+++ b/drivers/pci/host/pcie-altera.c
@@ -488,11 +488,10 @@ static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
 {
 	int err, res_valid = 0;
 	struct device *dev = &pcie->pdev->dev;
-	struct device_node *np = dev->of_node;
 	struct resource_entry *win;
 
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
-					       NULL);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff
+						    &pcie->resources, NULL);
 	if (err)
 		return err;
 
diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index e764a2a2693c..cec0130326c9 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -99,8 +99,8 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
 		pcie->phy = NULL;
 	}
 
-	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &resources,
-					       &iobase);
+	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff, &resources,
+						    &iobase);
 	if (ret) {
 		dev_err(dev, "unable to get PCI host bridge resources\n");
 		return ret;
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 6ab28f29ac6a..4c1787e021fd 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -1067,12 +1067,11 @@ static int rcar_pcie_parse_request_of_pci_ranges(struct rcar_pcie *pci)
 {
 	int err;
 	struct device *dev = pci->dev;
-	struct device_node *np = dev->of_node;
 	resource_size_t iobase;
 	struct resource_entry *win, *tmp;
 
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
-					       &iobase);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+						    &pci->resources, &iobase);
 	if (err)
 		return err;
 
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index f1e8f97ea1fb..abac972f0dc2 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -1560,8 +1560,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_deinit_port;
 
-	err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
-					       &res, &io_base);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+						    &res, &io_base);
 	if (err)
 		goto err_remove_irq_domain;
 
diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index 4839ae578711..6aea997cd21b 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -825,7 +825,6 @@ static const struct of_device_id nwl_pcie_of_match[] = {
 static int nwl_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *node = dev->of_node;
 	struct nwl_pcie *pcie;
 	struct pci_bus *bus;
 	struct pci_bus *child;
@@ -855,7 +854,8 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = of_pci_get_host_bridge_resources(node, 0, 0xff, &res, &iobase);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+						    &iobase);
 	if (err) {
 		dev_err(dev, "Getting bridge resources failed\n");
 		return err;
diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 0ad188effc09..fa5e44a480a4 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -643,8 +643,8 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff, &res,
-					       &iobase);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+						    &iobase);
 	if (err) {
 		dev_err(dev, "Getting bridge resources failed\n");
 		return err;
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 8d4778ef5806..ac97491ba377 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -245,7 +245,7 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 #if defined(CONFIG_OF_ADDRESS)
 /**
  * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
- * @dev_node: device node of the host bridge having the range property
+ * @dev: host bridge device
  * @busno: bus number associated with the bridge root bus
  * @bus_max: maximum number of buses for this bridge
  * @resources: list where the range of resources will be added after DT parsing
@@ -262,10 +262,11 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
  * It returns zero if the range parsing has been successful or a standard error
  * value if it failed.
  */
-int of_pci_get_host_bridge_resources(struct device_node *dev_node,
+int of_pci_get_host_bridge_resources(struct device *dev,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base)
 {
+	struct device_node *dev_node = dev->of_node;
 	struct resource_entry *window;
 	struct resource *res;
 	struct resource *bus_range;
@@ -599,12 +600,12 @@ int pci_parse_request_of_pci_ranges(struct device *dev,
 				    struct resource **bus_range)
 {
 	int err, res_valid = 0;
-	struct device_node *np = dev->of_node;
 	resource_size_t iobase;
 	struct resource_entry *win, *tmp;
 
 	INIT_LIST_HEAD(resources);
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, resources, &iobase);
+	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, resources,
+						    &iobase);
 	if (err)
 		return err;
 
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 74eec1943ad2..e6684c68cb94 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -71,11 +71,11 @@ of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
 #endif
 
 #if defined(CONFIG_OF_ADDRESS)
-int of_pci_get_host_bridge_resources(struct device_node *dev_node,
+int of_pci_get_host_bridge_resources(struct device *dev,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base);
 #else
-static inline int of_pci_get_host_bridge_resources(struct device_node *dev_node,
+static inline int of_pci_get_host_bridge_resources(struct device *dev,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base)
 {
-- 
2.13.6

Re: [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device

[Vladimir Zapolskiy]

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Another step towards a managed version of
> of_pci_get_host_bridge_resources(): Feed in the underlying device,
> rather than just the OF node. This will allow to use managed resource
> allocation internally later on.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

Re: [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device

[Jingoo Han]

On Tuesday, May 15, 2018 5:07 AM, Jan Kiszka wrote:
> 
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Another step towards a managed version of
> of_pci_get_host_bridge_resources(): Feed in the underlying device,
> rather than just the OF node. This will allow to use managed resource
> allocation internally later on.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/dwc/pcie-designware-host.c | 2 +-
>  drivers/pci/host/pci-aardvark.c        | 5 ++---
>  drivers/pci/host/pci-ftpci100.c        | 4 ++--
>  drivers/pci/host/pci-v3-semi.c         | 3 ++-
>  drivers/pci/host/pci-versatile.c       | 3 +--
>  drivers/pci/host/pci-xgene.c           | 3 ++-
>  drivers/pci/host/pcie-altera.c         | 5 ++---
>  drivers/pci/host/pcie-iproc-platform.c | 4 ++--
>  drivers/pci/host/pcie-rcar.c           | 5 ++---
>  drivers/pci/host/pcie-rockchip.c       | 4 ++--
>  drivers/pci/host/pcie-xilinx-nwl.c     | 4 ++--
>  drivers/pci/host/pcie-xilinx.c         | 4 ++--
>  drivers/pci/of.c                       | 9 +++++----
>  include/linux/of_pci.h                 | 4 ++--
>  14 files changed, 29 insertions(+), 30 deletions(-)
> 

Re: [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device

[Vladimir Zapolskiy]

Hi Jan, Bjorn,

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Another step towards a managed version of
> of_pci_get_host_bridge_resources(): Feed in the underlying device,
> rather than just the OF node. This will allow to use managed resource
> allocation internally later on.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

[snip]

> diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
> index a6af62e0256d..61802e55a00c 100644
> --- a/drivers/pci/host/pcie-altera.c
> +++ b/drivers/pci/host/pcie-altera.c
> @@ -488,11 +488,10 @@ static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
>  {
>  	int err, res_valid = 0;
>  	struct device *dev = &pcie->pdev->dev;
> -	struct device_node *np = dev->of_node;
>  	struct resource_entry *win;
>  
> -	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
> -					       NULL);
> +	err = of_pci_get_host_bridge_resources(dev, 0, 0xff
> +						    &pcie->resources, NULL);
>  	if (err)
>  		return err;
>  

In case if it is an undiscovered issue, a comma was mistakenly removed,
which will result it compilation error.

The problem is also found in pci/next , see commit 88e3909aa125.

--
With best wishes,
Vladimir

Re: [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device

[Jan Kiszka]

On 2018-05-28 12:00, Vladimir Zapolskiy wrote:
> Hi Jan, Bjorn,
> 
> On 05/15/2018 12:07 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Another step towards a managed version of
>> of_pci_get_host_bridge_resources(): Feed in the underlying device,
>> rather than just the OF node. This will allow to use managed resource
>> allocation internally later on.
>>
>> CC: Jingoo Han <jingoohan1@gmail.com>
>> CC: Joao Pinto <Joao.Pinto@synopsys.com>
>> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> [snip]
> 
>> diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
>> index a6af62e0256d..61802e55a00c 100644
>> --- a/drivers/pci/host/pcie-altera.c
>> +++ b/drivers/pci/host/pcie-altera.c
>> @@ -488,11 +488,10 @@ static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
>>  {
>>  	int err, res_valid = 0;
>>  	struct device *dev = &pcie->pdev->dev;
>> -	struct device_node *np = dev->of_node;
>>  	struct resource_entry *win;
>>  
>> -	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
>> -					       NULL);
>> +	err = of_pci_get_host_bridge_resources(dev, 0, 0xff
>> +						    &pcie->resources, NULL);
>>  	if (err)
>>  		return err;
>>  
> 
> In case if it is an undiscovered issue, a comma was mistakenly removed,
> which will result it compilation error.
> 
> The problem is also found in pci/next , see commit 88e3909aa125.

Yes, that's known. We have a bisection breakage: The issue was fixed
again by patch 6 in that series.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device

[Bjorn Helgaas]

On Mon, May 28, 2018 at 12:46:35PM +0200, Jan Kiszka wrote:
> On 2018-05-28 12:00, Vladimir Zapolskiy wrote:
> > Hi Jan, Bjorn,
> > 
> > On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Another step towards a managed version of
> >> of_pci_get_host_bridge_resources(): Feed in the underlying device,
> >> rather than just the OF node. This will allow to use managed resource
> >> allocation internally later on.
> >>
> >> CC: Jingoo Han <jingoohan1@gmail.com>
> >> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> >> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > [snip]
> > 
> >> diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
> >> index a6af62e0256d..61802e55a00c 100644
> >> --- a/drivers/pci/host/pcie-altera.c
> >> +++ b/drivers/pci/host/pcie-altera.c
> >> @@ -488,11 +488,10 @@ static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
> >>  {
> >>  	int err, res_valid = 0;
> >>  	struct device *dev = &pcie->pdev->dev;
> >> -	struct device_node *np = dev->of_node;
> >>  	struct resource_entry *win;
> >>  
> >> -	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
> >> -					       NULL);
> >> +	err = of_pci_get_host_bridge_resources(dev, 0, 0xff
> >> +						    &pcie->resources, NULL);
> >>  	if (err)
> >>  		return err;
> >>  
> > 
> > In case if it is an undiscovered issue, a comma was mistakenly removed,
> > which will result it compilation error.
> > 
> > The problem is also found in pci/next , see commit 88e3909aa125.
> 
> Yes, that's known. We have a bisection breakage: The issue was fixed
> again by patch 6 in that series.

I updated 88e3909aa125 to fix the bisection issue.  I'll rebuild
pci/next later today or tomorrow.

[PATCH v4 5/8] PCI: Replace pr_*() with dev_*() in of_pci_get_host_bridge_resources()

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Now that we have a device reference, make use of it for printing.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/of.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index ac97491ba377..4f21514cb4e4 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -282,15 +282,15 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 	if (!bus_range)
 		return -ENOMEM;
 
-	pr_info("host bridge %pOF ranges:\n", dev_node);
+	dev_info(dev, "host bridge %pOF ranges:\n", dev_node);
 
 	err = of_pci_parse_bus_range(dev_node, bus_range);
 	if (err) {
 		bus_range->start = busno;
 		bus_range->end = bus_max;
 		bus_range->flags = IORESOURCE_BUS;
-		pr_info("  No bus range found for %pOF, using %pR\n",
-			dev_node, bus_range);
+		dev_info(dev, "  No bus range found for %pOF, using %pR\n",
+			 dev_node, bus_range);
 	} else {
 		if (bus_range->end > bus_range->start + bus_max)
 			bus_range->end = bus_range->start + bus_max;
@@ -302,7 +302,7 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 	if (err)
 		goto parse_failed;
 
-	pr_debug("Parsing ranges property...\n");
+	dev_dbg(dev, "Parsing ranges property...\n");
 	for_each_of_pci_range(&parser, &range) {
 		/* Read next ranges element */
 		if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
@@ -311,9 +311,9 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 			snprintf(range_type, 4, "MEM");
 		else
 			snprintf(range_type, 4, "err");
-		pr_info("  %s %#010llx..%#010llx -> %#010llx\n", range_type,
-			range.cpu_addr, range.cpu_addr + range.size - 1,
-			range.pci_addr);
+		dev_info(dev, "  %s %#010llx..%#010llx -> %#010llx\n",
+			 range_type, range.cpu_addr,
+			 range.cpu_addr + range.size - 1, range.pci_addr);
 
 		/*
 		 * If we failed translation or got a zero-sized region
@@ -336,14 +336,16 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 
 		if (resource_type(res) == IORESOURCE_IO) {
 			if (!io_base) {
-				pr_err("I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
+				dev_err(dev,
+					"I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
 					dev_node);
 				err = -EINVAL;
 				goto conversion_failed;
 			}
 			if (*io_base != (resource_size_t)OF_BAD_ADDR)
-				pr_warn("More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
-					dev_node);
+				dev_warn(dev,
+					 "More than one I/O resource converted for %pOF. CPU base address for old range lost!\n",
+					 dev_node);
 			*io_base = range.cpu_addr;
 		}
 
-- 
2.13.6

Re: [PATCH v4 5/8] PCI: Replace pr_*() with dev_*() in of_pci_get_host_bridge_resources()

[Vladimir Zapolskiy]

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Now that we have a device reference, make use of it for printing.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

[PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

of_pci_get_host_bridge_resources() allocates the resource structures it
fills dynamically, but none of its callers care to release them so far.
Rather than requiring everyone to do this explicitly, convert the
existing function to a managed version.

CC: Jingoo Han <jingoohan1@gmail.com>
CC: Joao Pinto <Joao.Pinto@synopsys.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/dwc/pcie-designware-host.c |  2 +-
 drivers/pci/host/pci-aardvark.c        |  2 +-
 drivers/pci/host/pci-ftpci100.c        |  2 +-
 drivers/pci/host/pci-v3-semi.c         |  2 +-
 drivers/pci/host/pci-versatile.c       |  2 +-
 drivers/pci/host/pci-xgene.c           |  2 +-
 drivers/pci/host/pcie-altera.c         |  2 +-
 drivers/pci/host/pcie-iproc-platform.c |  2 +-
 drivers/pci/host/pcie-rcar.c           |  2 +-
 drivers/pci/host/pcie-rockchip.c       |  2 +-
 drivers/pci/host/pcie-xilinx-nwl.c     |  2 +-
 drivers/pci/host/pcie-xilinx.c         |  2 +-
 drivers/pci/of.c                       | 30 ++++++++++++------------------
 include/linux/of_pci.h                 |  4 ++--
 14 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 5a535690b7b5..a8f6ab54b4c0 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -342,7 +342,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	if (!bridge)
 		return -ENOMEM;
 
-	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+	ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
 					&bridge->windows, &pp->io_base);
 	if (ret)
 		return ret;
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index 39d8fc2a8a76..1e048dd806dc 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -827,7 +827,7 @@ static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie)
 
 	INIT_LIST_HEAD(&pcie->resources);
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
 						    &pcie->resources, &iobase);
 	if (err)
 		return err;
diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 5c176f806fe5..87748eaeaaed 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -476,7 +476,7 @@ static int faraday_pci_probe(struct platform_device *pdev)
 	if (IS_ERR(p->base))
 		return PTR_ERR(p->base);
 
-	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+	ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
 						    &res, &io_base);
 	if (ret)
 		return ret;
diff --git a/drivers/pci/host/pci-v3-semi.c b/drivers/pci/host/pci-v3-semi.c
index f3f39935ac2f..167bf6f6b378 100644
--- a/drivers/pci/host/pci-v3-semi.c
+++ b/drivers/pci/host/pci-v3-semi.c
@@ -791,7 +791,7 @@ static int v3_pci_probe(struct platform_device *pdev)
 	if (IS_ERR(v3->config_base))
 		return PTR_ERR(v3->config_base);
 
-	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+	ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
 						    &io_base);
 	if (ret)
 		return ret;
diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
index ef33ec0a9e1b..ff2cd12b3978 100644
--- a/drivers/pci/host/pci-versatile.c
+++ b/drivers/pci/host/pci-versatile.c
@@ -67,7 +67,7 @@ static int versatile_pci_parse_request_of_pci_ranges(struct device *dev,
 	resource_size_t iobase;
 	struct resource_entry *win, *tmp;
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, res, &iobase);
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, res, &iobase);
 	if (err)
 		return err;
 
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 88e9a6d315b3..7b3ed6e34b6c 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -632,7 +632,7 @@ static int xgene_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+	ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
 						    &iobase);
 	if (ret)
 		return ret;
diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
index 61802e55a00c..49410c7ba0cc 100644
--- a/drivers/pci/host/pcie-altera.c
+++ b/drivers/pci/host/pcie-altera.c
@@ -490,7 +490,7 @@ static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
 	struct device *dev = &pcie->pdev->dev;
 	struct resource_entry *win;
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff
 						    &pcie->resources, NULL);
 	if (err)
 		return err;
diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index cec0130326c9..99c2022813e4 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -99,7 +99,7 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
 		pcie->phy = NULL;
 	}
 
-	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff, &resources,
+	ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &resources,
 						    &iobase);
 	if (ret) {
 		dev_err(dev, "unable to get PCI host bridge resources\n");
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 4c1787e021fd..6eb36c924983 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -1070,7 +1070,7 @@ static int rcar_pcie_parse_request_of_pci_ranges(struct rcar_pcie *pci)
 	resource_size_t iobase;
 	struct resource_entry *win, *tmp;
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
 						    &pci->resources, &iobase);
 	if (err)
 		return err;
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index abac972f0dc2..27b97fcddf15 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -1560,7 +1560,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_deinit_port;
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff,
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
 						    &res, &io_base);
 	if (err)
 		goto err_remove_irq_domain;
diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index 6aea997cd21b..64df768c795c 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -854,7 +854,7 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
 						    &iobase);
 	if (err) {
 		dev_err(dev, "Getting bridge resources failed\n");
diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index fa5e44a480a4..88c96e5669e0 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -643,7 +643,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, &res,
 						    &iobase);
 	if (err) {
 		dev_err(dev, "Getting bridge resources failed\n");
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 4f21514cb4e4..b06585a1da75 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -244,7 +244,8 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
 
 #if defined(CONFIG_OF_ADDRESS)
 /**
- * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
+ * devm_of_pci_get_host_bridge_resources() - Resource-managed parsing of PCI
+ *                                           host bridge resources from DT
  * @dev: host bridge device
  * @busno: bus number associated with the bridge root bus
  * @bus_max: maximum number of buses for this bridge
@@ -253,8 +254,6 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
  * address for the start of the I/O range. Can be NULL if the caller doesn't
  * expect I/O ranges to be present in the device tree.
  *
- * It is the caller's job to free the @resources list.
- *
  * This function will parse the "ranges" property of a PCI host bridge device
  * node and setup the resource mapping based on its content. It is expected
  * that the property conforms with the Power ePAPR document.
@@ -262,12 +261,11 @@ EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
  * It returns zero if the range parsing has been successful or a standard error
  * value if it failed.
  */
-int of_pci_get_host_bridge_resources(struct device *dev,
+int devm_of_pci_get_host_bridge_resources(struct device *dev,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base)
 {
 	struct device_node *dev_node = dev->of_node;
-	struct resource_entry *window;
 	struct resource *res;
 	struct resource *bus_range;
 	struct of_pci_range range;
@@ -278,7 +276,7 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 	if (io_base)
 		*io_base = (resource_size_t)OF_BAD_ADDR;
 
-	bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
+	bus_range = devm_kzalloc(dev, sizeof(*bus_range), GFP_KERNEL);
 	if (!bus_range)
 		return -ENOMEM;
 
@@ -300,7 +298,7 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 	/* Check for ranges property */
 	err = of_pci_range_parser_init(&parser, dev_node);
 	if (err)
-		goto parse_failed;
+		goto failed;
 
 	dev_dbg(dev, "Parsing ranges property...\n");
 	for_each_of_pci_range(&parser, &range) {
@@ -322,15 +320,15 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
 			continue;
 
-		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
+		res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
 		if (!res) {
 			err = -ENOMEM;
-			goto parse_failed;
+			goto failed;
 		}
 
 		err = of_pci_range_to_resource(&range, dev_node, res);
 		if (err) {
-			kfree(res);
+			devm_kfree(dev, res);
 			continue;
 		}
 
@@ -340,7 +338,7 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 					"I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n",
 					dev_node);
 				err = -EINVAL;
-				goto conversion_failed;
+				goto failed;
 			}
 			if (*io_base != (resource_size_t)OF_BAD_ADDR)
 				dev_warn(dev,
@@ -354,15 +352,11 @@ int of_pci_get_host_bridge_resources(struct device *dev,
 
 	return 0;
 
-conversion_failed:
-	kfree(res);
-parse_failed:
-	resource_list_for_each_entry(window, resources)
-		kfree(window->res);
+failed:
 	pci_free_resource_list(resources);
 	return err;
 }
-EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
+EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
 #endif /* CONFIG_OF_ADDRESS */
 
 /**
@@ -606,7 +600,7 @@ int pci_parse_request_of_pci_ranges(struct device *dev,
 	struct resource_entry *win, *tmp;
 
 	INIT_LIST_HEAD(resources);
-	err = of_pci_get_host_bridge_resources(dev, 0, 0xff, resources,
+	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, resources,
 						    &iobase);
 	if (err)
 		return err;
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index e6684c68cb94..fa4463a52900 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -71,11 +71,11 @@ of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
 #endif
 
 #if defined(CONFIG_OF_ADDRESS)
-int of_pci_get_host_bridge_resources(struct device *dev,
+int devm_of_pci_get_host_bridge_resources(struct device *dev,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base);
 #else
-static inline int of_pci_get_host_bridge_resources(struct device *dev,
+static inline int devm_of_pci_get_host_bridge_resources(struct device *dev,
 			unsigned char busno, unsigned char bus_max,
 			struct list_head *resources, resource_size_t *io_base)
 {
-- 
2.13.6

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Vladimir Zapolskiy]

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Now it is correct, thanks.

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Joao Pinto]

Hi Jan,

Às 10:07 AM de 5/15/2018, Jan Kiszka escreveu:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/pci-aardvark.c        |  2 +-
>  drivers/pci/host/pci-ftpci100.c        |  2 +-
>  drivers/pci/host/pci-v3-semi.c         |  2 +-
>  drivers/pci/host/pci-versatile.c       |  2 +-
>  drivers/pci/host/pci-xgene.c           |  2 +-
>  drivers/pci/host/pcie-altera.c         |  2 +-
>  drivers/pci/host/pcie-iproc-platform.c |  2 +-
>  drivers/pci/host/pcie-rcar.c           |  2 +-
>  drivers/pci/host/pcie-rockchip.c       |  2 +-
>  drivers/pci/host/pcie-xilinx-nwl.c     |  2 +-
>  drivers/pci/host/pcie-xilinx.c         |  2 +-
>  drivers/pci/of.c                       | 30 ++++++++++++------------------
>  include/linux/of_pci.h                 |  4 ++--
>  14 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 5a535690b7b5..a8f6ab54b4c0 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -342,7 +342,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	if (!bridge)
>  		return -ENOMEM;
>  
> -	ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
> +	ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
>  					&bridge->windows, &pp->io_base);
>  	if (ret)
>  		return ret;

(snip...)

Thanks for this patch!

Acked-by: Joao Pinto <jpinto@synopsys.com>

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Jingoo Han]

On Tuesday, May 15, 2018 5:07 AM, Jan Kiszka wrote:
> 
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/pci-aardvark.c        |  2 +-
>  drivers/pci/host/pci-ftpci100.c        |  2 +-
>  drivers/pci/host/pci-v3-semi.c         |  2 +-
>  drivers/pci/host/pci-versatile.c       |  2 +-
>  drivers/pci/host/pci-xgene.c           |  2 +-
>  drivers/pci/host/pcie-altera.c         |  2 +-
>  drivers/pci/host/pcie-iproc-platform.c |  2 +-
>  drivers/pci/host/pcie-rcar.c           |  2 +-
>  drivers/pci/host/pcie-rockchip.c       |  2 +-
>  drivers/pci/host/pcie-xilinx-nwl.c     |  2 +-
>  drivers/pci/host/pcie-xilinx.c         |  2 +-
>  drivers/pci/of.c                       | 30
++++++++++++------------------
>  include/linux/of_pci.h                 |  4 ++--
>  14 files changed, 26 insertions(+), 32 deletions(-)
> 

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.

> -               res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> +               res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
>                 if (!res) {
>                         err = -ENOMEM;
> -                       goto parse_failed;
> +                       goto failed;
>                 }
>
>                 err = of_pci_range_to_resource(&range, dev_node, res);
>                 if (err) {
> -                       kfree(res);
> +                       devm_kfree(dev, res);
>                         continue;
>                 }

Can't you rather make it better, i.e.

struct resource tmp;
...

err = of_pci_range_to_resource(&range, dev_node, &tmp);
if (err)
   continue;

res = devm_kmemdump();
if (!res) {
 ret = -ENOMEM;
 goto failed;
}

?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Tue, May 15, 2018 at 7:48 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

>> +                       devm_kfree(dev, res);

devm_kfree() makes my eyes hurt.

> res = devm_kmemdump();

devm_kmemdup(&tmp...);

-- 
With Best Regards,
Andy Shevchenko

[PATCH] PCI: Clean up resource allocation in devm_of_pci_get_host_bridge_resources()

[Jan Kiszka]

Instead of first allocating and then freeing memory for struct resource
in case we cannot parse a PCI resource from the device tree, work
against a local struct and kmemdup it when we decide to go with it.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/of.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index b06585a1da75..fc0f906c5c25 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -266,7 +266,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
 			struct list_head *resources, resource_size_t *io_base)
 {
 	struct device_node *dev_node = dev->of_node;
-	struct resource *res;
+	struct resource *res, tmp_res;
 	struct resource *bus_range;
 	struct of_pci_range range;
 	struct of_pci_range_parser parser;
@@ -320,18 +320,16 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
 		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
 			continue;
 
-		res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
+		err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
+		if (err)
+			continue;
+
+		res = devm_kmemdup(dev, &tmp_res, sizeof(tmp_res), GFP_KERNEL);
 		if (!res) {
 			err = -ENOMEM;
 			goto failed;
 		}
 
-		err = of_pci_range_to_resource(&range, dev_node, res);
-		if (err) {
-			devm_kfree(dev, res);
-			continue;
-		}
-
 		if (resource_type(res) == IORESOURCE_IO) {
 			if (!io_base) {
 				dev_err(dev,
-- 
2.13.6

Re: [PATCH] PCI: Clean up resource allocation in devm_of_pci_get_host_bridge_resources()

[Vladimir Zapolskiy]

On 05/16/2018 03:31 PM, Jan Kiszka wrote:
> Instead of first allocating and then freeing memory for struct resource
> in case we cannot parse a PCI resource from the device tree, work
> against a local struct and kmemdup it when we decide to go with it.
> 
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/pci/of.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index b06585a1da75..fc0f906c5c25 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -266,7 +266,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
>  			struct list_head *resources, resource_size_t *io_base)
>  {
>  	struct device_node *dev_node = dev->of_node;
> -	struct resource *res;
> +	struct resource *res, tmp_res;
>  	struct resource *bus_range;
>  	struct of_pci_range range;
>  	struct of_pci_range_parser parser;
> @@ -320,18 +320,16 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
>  		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>  			continue;
>  
> -		res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> +		err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
> +		if (err)
> +			continue;
> +
> +		res = devm_kmemdup(dev, &tmp_res, sizeof(tmp_res), GFP_KERNEL);

The change looks okay, apparently probable garbage in tmp_res.desc has no impact.

Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

Re: [PATCH] PCI: Clean up resource allocation in devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Wed, May 16, 2018 at 3:31 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Instead of first allocating and then freeing memory for struct resource
> in case we cannot parse a PCI resource from the device tree, work
> against a local struct and kmemdup it when we decide to go with it.
>

Bjorn, any comments on this?

> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/pci/of.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index b06585a1da75..fc0f906c5c25 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -266,7 +266,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
>                         struct list_head *resources, resource_size_t *io_base)
>  {
>         struct device_node *dev_node = dev->of_node;
> -       struct resource *res;
> +       struct resource *res, tmp_res;
>         struct resource *bus_range;
>         struct of_pci_range range;
>         struct of_pci_range_parser parser;
> @@ -320,18 +320,16 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
>                 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>                         continue;
>
> -               res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> +               err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
> +               if (err)
> +                       continue;
> +
> +               res = devm_kmemdup(dev, &tmp_res, sizeof(tmp_res), GFP_KERNEL);
>                 if (!res) {
>                         err = -ENOMEM;
>                         goto failed;
>                 }
>
> -               err = of_pci_range_to_resource(&range, dev_node, res);
> -               if (err) {
> -                       devm_kfree(dev, res);
> -                       continue;
> -               }
> -
>                 if (resource_type(res) == IORESOURCE_IO) {
>                         if (!io_base) {
>                                 dev_err(dev,
> --
> 2.13.6



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] PCI: Clean up resource allocation in devm_of_pci_get_host_bridge_resources()

[Bjorn Helgaas]

On Tue, Jun 05, 2018 at 07:26:57PM +0300, Andy Shevchenko wrote:
> On Wed, May 16, 2018 at 3:31 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> > Instead of first allocating and then freeing memory for struct resource
> > in case we cannot parse a PCI resource from the device tree, work
> > against a local struct and kmemdup it when we decide to go with it.
> >
> 
> Bjorn, any comments on this?

I'm trying to finish up the v4.18 changes now, but I'll look at this later.
If it's urgent, let me know and I can look sooner.

> > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> >  drivers/pci/of.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index b06585a1da75..fc0f906c5c25 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -266,7 +266,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
> >                         struct list_head *resources, resource_size_t *io_base)
> >  {
> >         struct device_node *dev_node = dev->of_node;
> > -       struct resource *res;
> > +       struct resource *res, tmp_res;
> >         struct resource *bus_range;
> >         struct of_pci_range range;
> >         struct of_pci_range_parser parser;
> > @@ -320,18 +320,16 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
> >                 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> >                         continue;
> >
> > -               res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> > +               err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
> > +               if (err)
> > +                       continue;
> > +
> > +               res = devm_kmemdup(dev, &tmp_res, sizeof(tmp_res), GFP_KERNEL);
> >                 if (!res) {
> >                         err = -ENOMEM;
> >                         goto failed;
> >                 }
> >
> > -               err = of_pci_range_to_resource(&range, dev_node, res);
> > -               if (err) {
> > -                       devm_kfree(dev, res);
> > -                       continue;
> > -               }
> > -
> >                 if (resource_type(res) == IORESOURCE_IO) {
> >                         if (!io_base) {
> >                                 dev_err(dev,
> > --
> > 2.13.6
> 
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] PCI: Clean up resource allocation in devm_of_pci_get_host_bridge_resources()

[Bjorn Helgaas]

On Wed, May 16, 2018 at 02:31:44PM +0200, Jan Kiszka wrote:
> Instead of first allocating and then freeing memory for struct resource
> in case we cannot parse a PCI resource from the device tree, work
> against a local struct and kmemdup it when we decide to go with it.
> 
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Applied with Vladimir's reviewed-by to pci/resource for v4.19, thanks!

> ---
>  drivers/pci/of.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index b06585a1da75..fc0f906c5c25 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -266,7 +266,7 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
>  			struct list_head *resources, resource_size_t *io_base)
>  {
>  	struct device_node *dev_node = dev->of_node;
> -	struct resource *res;
> +	struct resource *res, tmp_res;
>  	struct resource *bus_range;
>  	struct of_pci_range range;
>  	struct of_pci_range_parser parser;
> @@ -320,18 +320,16 @@ int devm_of_pci_get_host_bridge_resources(struct device *dev,
>  		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>  			continue;
>  
> -		res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> +		err = of_pci_range_to_resource(&range, dev_node, &tmp_res);
> +		if (err)
> +			continue;
> +
> +		res = devm_kmemdup(dev, &tmp_res, sizeof(tmp_res), GFP_KERNEL);
>  		if (!res) {
>  			err = -ENOMEM;
>  			goto failed;
>  		}
>  
> -		err = of_pci_range_to_resource(&range, dev_node, res);
> -		if (err) {
> -			devm_kfree(dev, res);
> -			continue;
> -		}
> -
>  		if (resource_type(res) == IORESOURCE_IO) {
>  			if (!io_base) {
>  				dev_err(dev,
> -- 
> 2.13.6

[PATCH v4 7/8] PCI: Add support for unbinding the generic PCI host controller

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Particularly useful when working in virtual environments where the
controller may come and go, but possibly not only there.

CC: Will Deacon <will.deacon@arm.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/host/pci-host-common.c  | 13 +++++++++++++
 drivers/pci/host/pci-host-generic.c |  1 +
 include/linux/pci-ecam.h            |  1 +
 3 files changed, 15 insertions(+)

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index 5d028f53fdcd..d8f10451f273 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -101,5 +101,18 @@ int pci_host_common_probe(struct platform_device *pdev,
 		return ret;
 	}
 
+	platform_set_drvdata(pdev, bridge->bus);
+	return 0;
+}
+
+int pci_host_common_remove(struct platform_device *pdev)
+{
+	struct pci_bus *bus = platform_get_drvdata(pdev);
+
+	pci_lock_rescan_remove();
+	pci_stop_root_bus(bus);
+	pci_remove_root_bus(bus);
+	pci_unlock_rescan_remove();
+
 	return 0;
 }
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 45319ee3b484..dea3ec7592a2 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -95,5 +95,6 @@ static struct platform_driver gen_pci_driver = {
 		.suppress_bind_attrs = true,
 	},
 	.probe = gen_pci_probe,
+	.remove = pci_host_common_remove,
 };
 builtin_platform_driver(gen_pci_driver);
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index baadad1aabbc..29efa09d686b 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -62,5 +62,6 @@ extern struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
 /* for DT-based PCI controllers that support ECAM */
 int pci_host_common_probe(struct platform_device *pdev,
 			  struct pci_ecam_ops *ops);
+int pci_host_common_remove(struct platform_device *pdev);
 #endif
 #endif
-- 
2.13.6

[PATCH v4 8/8] PCI: Enable PCI_DOMAINS along with generic PCI host controller

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

This controller is often instantiated by hypervisors, and they may add
multiple of them or add them in addition to a physical host controller
like the Jailhouse hypervisor is doing. Therefore allow for multiple
domains so that we can handle them all.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/pci/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 0d0177ce436c..3d25b35bb5ab 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -68,6 +68,7 @@ config PCI_HOST_GENERIC
 	depends on (ARM || ARM64) && OF
 	select PCI_HOST_COMMON
 	select IRQ_DOMAIN
+	select PCI_DOMAINS
 	help
 	  Say Y here if you want to support a simple generic PCI host
 	  controller, such as the one emulated by kvmtool.
-- 
2.13.6

Re: [PATCH v4 0/8] PCI: leak fixes, removable generic PCI host, assorted stuff

[Bjorn Helgaas]

On Tue, May 15, 2018 at 11:06:59AM +0200, Jan Kiszka wrote:
> Changes in v4:
>  - restore pci_free_resource_list() in error path of
>    of_pci_get_host_bridge_resources()
> 
> Changes in v3:
>  - refactor series to be both bisectable and simpler while reworking
>    of_pci_get_host_bridge_resources()
>  - include of_pci_get_host_bridge_resources() removal
>  - include devm_of_pci_get_host_bridge_resources() error path fixes
>  - effectively, no functional changes to v2
> 
> Changes in v2:
>  - patch 1: commit message reworking as suggested by Lorenzo
>  - patch 3-6: split-up as suggested by Bjorn
>  - patch 8: new
>  - patch 10: select PCI_DOMAINS from PCI_HOST_GENERIC, rather than
>    allowing manual choice, as suggested by Lorenzo
> 
> This primarily enables to unbind the generic PCI host controller without
> leaving lots of memory leaks behind. A previous proposal patch 5 was
> rejected because of those issues [1].
> 
> The fixes have been validated in the Jailhouse setup, where we add and
> remove a virtual PCI host controller on hypervisor activation/
> deactivation, with the help of kmemleak.
> 
> Besides that, there is tiny PCI API cleanup at the beginning and
> support for manually enabled PCI domains at the end that enables the
> Jailhouse scenario.

This is beautiful!  Thanks for tweaking this one more time; I think it
really makes the patches much more readable.

I applied these with the tested-by/reviewed-by/acks to pci/resource for
v4.18.

Andy's comments certainly make sense, but doing it in the "Rework
of_pci_get_host_bridge_resources()" patch would make that patch less
obvious, so I'd rather do it as a cleanup on top.

> [1] http://lkml.iu.edu/hypermail/linux/kernel/1606.3/00072.html
> 
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> 
> Jan Kiszka (8):
>   PCI: Make pci_get_new_domain_nr() static
>   PCI: Fix memory leak of devm_pci_alloc_host_bridge()
>   PCI: Rename device node parameter of
>     of_pci_get_host_bridge_resources()
>   PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources
>     with device
>   PCI: Replace pr_*() with dev_*() in of_pci_get_host_bridge_resources()
>   PCI: Rework of_pci_get_host_bridge_resources() to
>     devm_of_pci_get_host_bridge_resources()
>   PCI: Add support for unbinding the generic PCI host controller
>   PCI: Enable PCI_DOMAINS along with generic PCI host controller
> 
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/Kconfig               |  1 +
>  drivers/pci/host/pci-aardvark.c        |  5 ++-
>  drivers/pci/host/pci-ftpci100.c        |  4 +--
>  drivers/pci/host/pci-host-common.c     | 13 +++++++
>  drivers/pci/host/pci-host-generic.c    |  1 +
>  drivers/pci/host/pci-v3-semi.c         |  3 +-
>  drivers/pci/host/pci-versatile.c       |  3 +-
>  drivers/pci/host/pci-xgene.c           |  3 +-
>  drivers/pci/host/pcie-altera.c         |  5 ++-
>  drivers/pci/host/pcie-iproc-platform.c |  4 +--
>  drivers/pci/host/pcie-rcar.c           |  5 ++-
>  drivers/pci/host/pcie-rockchip.c       |  4 +--
>  drivers/pci/host/pcie-xilinx-nwl.c     |  4 +--
>  drivers/pci/host/pcie-xilinx.c         |  4 +--
>  drivers/pci/of.c                       | 65 ++++++++++++++++------------------
>  drivers/pci/pci.c                      |  6 ++--
>  drivers/pci/probe.c                    |  4 ++-
>  include/linux/of_pci.h                 |  4 +--
>  include/linux/pci-ecam.h               |  1 +
>  include/linux/pci.h                    |  3 --
>  21 files changed, 76 insertions(+), 68 deletions(-)
> 
> -- 
> 2.13.6
>