* [PATCH 1/6] PCI: Make pci_get_new_domain_nr static
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
@ 2018-04-24 15:13 ` Jan Kiszka
2018-04-25 16:27 ` Lorenzo Pieralisi
2018-04-24 15:13 ` [PATCH 2/6] PCI: Fix memory leak of devm_pci_alloc_host_bridge Jan Kiszka
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Jan Kiszka @ 2018-04-24 15:13 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
From: Jan Kiszka <jan.kiszka@siemens.com>
The only user of that function is of_pci_bus_find_domain_nr. Pure
cleanup.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.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 e597655a5643..695c2bb4e853 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5702,15 +5702,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;
@@ -5765,7 +5764,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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/6] PCI: Make pci_get_new_domain_nr static
2018-04-24 15:13 ` [PATCH 1/6] PCI: Make pci_get_new_domain_nr static Jan Kiszka
@ 2018-04-25 16:27 ` Lorenzo Pieralisi
2018-04-25 17:21 ` Jan Kiszka
0 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2018-04-25 16:27 UTC (permalink / raw)
To: Jan Kiszka
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, linux-arm-kernel
On Tue, Apr 24, 2018 at 05:13:37PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> The only user of that function is of_pci_bus_find_domain_nr. Pure
> cleanup.
"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>
> ---
> drivers/pci/pci.c | 6 ++----
> include/linux/pci.h | 3 ---
> 2 files changed, 2 insertions(+), 7 deletions(-)
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e597655a5643..695c2bb4e853 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5702,15 +5702,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;
> @@ -5765,7 +5764,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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/6] PCI: Make pci_get_new_domain_nr static
2018-04-25 16:27 ` Lorenzo Pieralisi
@ 2018-04-25 17:21 ` Jan Kiszka
0 siblings, 0 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-04-25 17:21 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
On 2018-04-25 18:27, Lorenzo Pieralisi wrote:
> On Tue, Apr 24, 2018 at 05:13:37PM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> The only user of that function is of_pci_bus_find_domain_nr. Pure
>> cleanup.
>
> "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."
>
Thanks, wording adopted for v2.
Jan
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> drivers/pci/pci.c | 6 ++----
>> include/linux/pci.h | 3 ---
>> 2 files changed, 2 insertions(+), 7 deletions(-)
>
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index e597655a5643..695c2bb4e853 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -5702,15 +5702,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;
>> @@ -5765,7 +5764,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
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/6] PCI: Fix memory leak of devm_pci_alloc_host_bridge
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
2018-04-24 15:13 ` [PATCH 1/6] PCI: Make pci_get_new_domain_nr static Jan Kiszka
@ 2018-04-24 15:13 ` Jan Kiszka
2018-04-24 15:13 ` [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources Jan Kiszka
` (4 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-04-24 15:13 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
2018-04-24 15:13 ` [PATCH 1/6] PCI: Make pci_get_new_domain_nr static Jan Kiszka
2018-04-24 15:13 ` [PATCH 2/6] PCI: Fix memory leak of devm_pci_alloc_host_bridge Jan Kiszka
@ 2018-04-24 15:13 ` Jan Kiszka
2018-04-25 10:40 ` Jan Kiszka
2018-04-27 22:24 ` Bjorn Helgaas
2018-04-24 15:13 ` [PATCH 4/6] PCI: Convert of_pci_get_host_bridge_resources users to devm variant Jan Kiszka
` (3 subsequent siblings)
6 siblings, 2 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-04-24 15:13 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
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, introduce a
managed version of that service. This differs API-wise only in taking a
reference to the associated device, rather than to the device tree node.
As of_pci_get_host_bridge_resources is an exported interface, we cannot
simply drop it at this point. After converting all in-tree users to the
new API, we could phase out the unmanaged one over some grace period.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/pci/of.c | 89 ++++++++++++++++++++++++++++++++------------------
include/linux/of_pci.h | 14 ++++++--
2 files changed, 70 insertions(+), 33 deletions(-)
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index a28355c273ae..6eab0bde2ab3 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -243,26 +243,8 @@ void of_pci_check_probe_only(void)
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
- * @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
- * @io_base: pointer to a variable that will contain on return the physical
- * 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.
- *
- * 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,
+static int __of_pci_get_host_bridge_resources(struct device *dev,
+ struct device_node *dev_node,
unsigned char busno, unsigned char bus_max,
struct list_head *resources, resource_size_t *io_base)
{
@@ -277,19 +259,22 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
if (io_base)
*io_base = (resource_size_t)OF_BAD_ADDR;
- bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
+ if (dev)
+ bus_range = devm_kzalloc(dev,sizeof(*bus_range), GFP_KERNEL);
+ else
+ bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
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 +282,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;
@@ -321,13 +306,17 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
continue;
- res = kzalloc(sizeof(struct resource), GFP_KERNEL);
+ if (dev)
+ res = devm_kzalloc(dev, sizeof(struct resource),
+ GFP_KERNEL);
+ else
+ res = kzalloc(sizeof(struct resource), GFP_KERNEL);
if (!res) {
err = -ENOMEM;
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 +325,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;
}
@@ -354,12 +343,50 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
conversion_failed:
kfree(res);
parse_failed:
- resource_list_for_each_entry(window, resources)
- kfree(window->res);
+ if (!dev);
+ resource_list_for_each_entry(window, resources)
+ kfree(window->res);
pci_free_resource_list(resources);
return err;
}
+
+/**
+ * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
+ * @dev: 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
+ * @io_base: pointer to a variable that will contain on return the physical
+ * 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.
+ *
+ * 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,
+ unsigned char busno, unsigned char bus_max,
+ struct list_head *resources, resource_size_t *io_base)
+{
+ return __of_pci_get_host_bridge_resources(NULL, dev_node, busno,
+ bus_max, resources, io_base);
+}
EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
+
+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)
+{
+ return __of_pci_get_host_bridge_resources(dev, dev->of_node, busno,
+ bus_max, resources, io_base);
+}
+EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
+
#endif /* CONFIG_OF_ADDRESS */
/**
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 091033a6b836..08b8f02426a5 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -71,11 +71,21 @@ 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);
+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_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)
+{
+ return -EINVAL;
+}
+
+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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-24 15:13 ` [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources Jan Kiszka
@ 2018-04-25 10:40 ` Jan Kiszka
2018-04-27 22:24 ` Bjorn Helgaas
1 sibling, 0 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-04-25 10:40 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
On 2018-04-24 17:13, 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, introduce a
> managed version of that service. This differs API-wise only in taking a
> reference to the associated device, rather than to the device tree node.
>
> As of_pci_get_host_bridge_resources is an exported interface, we cannot
> simply drop it at this point. After converting all in-tree users to the
> new API, we could phase out the unmanaged one over some grace period.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> drivers/pci/of.c | 89 ++++++++++++++++++++++++++++++++------------------
> include/linux/of_pci.h | 14 ++++++--
> 2 files changed, 70 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a28355c273ae..6eab0bde2ab3 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -243,26 +243,8 @@ void of_pci_check_probe_only(void)
> 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
> - * @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
> - * @io_base: pointer to a variable that will contain on return the physical
> - * 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.
> - *
> - * 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,
> +static int __of_pci_get_host_bridge_resources(struct device *dev,
> + struct device_node *dev_node,
> unsigned char busno, unsigned char bus_max,
> struct list_head *resources, resource_size_t *io_base)
> {
> @@ -277,19 +259,22 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> if (io_base)
> *io_base = (resource_size_t)OF_BAD_ADDR;
>
> - bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
> + if (dev)
> + bus_range = devm_kzalloc(dev,sizeof(*bus_range), GFP_KERNEL);
> + else
> + bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
> 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 +282,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;
>
> @@ -321,13 +306,17 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> continue;
>
> - res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> + if (dev)
> + res = devm_kzalloc(dev, sizeof(struct resource),
> + GFP_KERNEL);
> + else
> + res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> if (!res) {
> err = -ENOMEM;
> 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 +325,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;
> }
>
> @@ -354,12 +343,50 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> conversion_failed:
> kfree(res);
> parse_failed:
> - resource_list_for_each_entry(window, resources)
> - kfree(window->res);
> + if (!dev);
> + resource_list_for_each_entry(window, resources)
> + kfree(window->res);
> pci_free_resource_list(resources);
> return err;
> }
> +
> +/**
> + * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
> + * @dev: 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
> + * @io_base: pointer to a variable that will contain on return the physical
> + * 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.
> + *
> + * 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,
> + unsigned char busno, unsigned char bus_max,
> + struct list_head *resources, resource_size_t *io_base)
> +{
> + return __of_pci_get_host_bridge_resources(NULL, dev_node, busno,
> + bus_max, resources, io_base);
> +}
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> +
> +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)
> +{
> + return __of_pci_get_host_bridge_resources(dev, dev->of_node, busno,
> + bus_max, resources, io_base);
> +}
> +EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
> +
> #endif /* CONFIG_OF_ADDRESS */
>
> /**
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 091033a6b836..08b8f02426a5 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -71,11 +71,21 @@ 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);
> +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_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)
> +{
> + return -EINVAL;
> +}
> +
> +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)
> {
>
I left a small reviewer exercise in this patch behind, just noticed
during a backport. Will resolved it via some v2, just waiting in case
there will be further comments.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-24 15:13 ` [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources Jan Kiszka
2018-04-25 10:40 ` Jan Kiszka
@ 2018-04-27 22:24 ` Bjorn Helgaas
2018-04-28 7:28 ` Jan Kiszka
1 sibling, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2018-04-27 22:24 UTC (permalink / raw)
To: Jan Kiszka
Cc: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
On Tue, Apr 24, 2018 at 05:13:39PM +0200, 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, introduce a
> managed version of that service. This differs API-wise only in taking a
> reference to the associated device, rather than to the device tree node.
>
> As of_pci_get_host_bridge_resources is an exported interface, we cannot
> simply drop it at this point. After converting all in-tree users to the
> new API, we could phase out the unmanaged one over some grace period.
It looks like it might be possible to split this into three or four
patches:
1) Factor __of_pci_get_host_bridge_resources() out of
of_pci_get_host_bridge_resources()
2) Add struct device * argument
3) Convert pr_info() to dev_info()
4) Add devm_of_pci_get_host_bridge_resources()
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> drivers/pci/of.c | 89 ++++++++++++++++++++++++++++++++------------------
> include/linux/of_pci.h | 14 ++++++--
> 2 files changed, 70 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a28355c273ae..6eab0bde2ab3 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -243,26 +243,8 @@ void of_pci_check_probe_only(void)
> 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
> - * @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
> - * @io_base: pointer to a variable that will contain on return the physical
> - * 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.
> - *
> - * 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,
> +static int __of_pci_get_host_bridge_resources(struct device *dev,
> + struct device_node *dev_node,
> unsigned char busno, unsigned char bus_max,
> struct list_head *resources, resource_size_t *io_base)
> {
> @@ -277,19 +259,22 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> if (io_base)
> *io_base = (resource_size_t)OF_BAD_ADDR;
>
> - bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
> + if (dev)
> + bus_range = devm_kzalloc(dev,sizeof(*bus_range), GFP_KERNEL);
> + else
> + bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
> if (!bus_range)
> return -ENOMEM;
>
> - pr_info("host bridge %pOF ranges:\n", dev);
> + pr_info("host bridge %pOF ranges:\n", dev_node);
Since you now have a struct device *, maybe convert these to dev_info()?
It looks like __dev_printk() does something sensible if "dev" is NULL.
>
> - 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 +282,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;
>
> @@ -321,13 +306,17 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> continue;
>
> - res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> + if (dev)
> + res = devm_kzalloc(dev, sizeof(struct resource),
> + GFP_KERNEL);
> + else
> + res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> if (!res) {
> err = -ENOMEM;
> 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 +325,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;
> }
>
> @@ -354,12 +343,50 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> conversion_failed:
> kfree(res);
> parse_failed:
> - resource_list_for_each_entry(window, resources)
> - kfree(window->res);
> + if (!dev);
> + resource_list_for_each_entry(window, resources)
> + kfree(window->res);
> pci_free_resource_list(resources);
> return err;
> }
> +
> +/**
> + * of_pci_get_host_bridge_resources - Parse PCI host bridge resources from DT
> + * @dev: 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
> + * @io_base: pointer to a variable that will contain on return the physical
> + * 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.
> + *
> + * 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,
> + unsigned char busno, unsigned char bus_max,
> + struct list_head *resources, resource_size_t *io_base)
> +{
> + return __of_pci_get_host_bridge_resources(NULL, dev_node, busno,
> + bus_max, resources, io_base);
> +}
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> +
> +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)
> +{
> + return __of_pci_get_host_bridge_resources(dev, dev->of_node, busno,
> + bus_max, resources, io_base);
> +}
> +EXPORT_SYMBOL_GPL(devm_of_pci_get_host_bridge_resources);
> +
> #endif /* CONFIG_OF_ADDRESS */
>
> /**
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 091033a6b836..08b8f02426a5 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -71,11 +71,21 @@ 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);
> +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_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)
> +{
> + return -EINVAL;
> +}
> +
> +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
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-27 22:24 ` Bjorn Helgaas
@ 2018-04-28 7:28 ` Jan Kiszka
2018-04-30 18:40 ` Bjorn Helgaas
0 siblings, 1 reply; 19+ messages in thread
From: Jan Kiszka @ 2018-04-28 7:28 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, linux-arm-kernel
On 2018-04-28 00:24, Bjorn Helgaas wrote:
> On Tue, Apr 24, 2018 at 05:13:39PM +0200, 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, introduce a
>> managed version of that service. This differs API-wise only in taking a
>> reference to the associated device, rather than to the device tree node.
>>
>> As of_pci_get_host_bridge_resources is an exported interface, we cannot
>> simply drop it at this point. After converting all in-tree users to the
>> new API, we could phase out the unmanaged one over some grace period.
>
> It looks like it might be possible to split this into three or four
> patches:
>
> 1) Factor __of_pci_get_host_bridge_resources() out of
> of_pci_get_host_bridge_resources()
>
> 2) Add struct device * argument
>
> 3) Convert pr_info() to dev_info()
>
> 4) Add devm_of_pci_get_host_bridge_resources()
Will do. I'm even considering
5) mark of_pci_get_host_bridge_resources() __deprecated, due to the leak
and no remaining in-tree user - what do you think?
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-28 7:28 ` Jan Kiszka
@ 2018-04-30 18:40 ` Bjorn Helgaas
2018-04-30 18:43 ` Sinan Kaya
0 siblings, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2018-04-30 18:40 UTC (permalink / raw)
To: Jan Kiszka
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, linux-arm-kernel
On Sat, Apr 28, 2018 at 09:28:47AM +0200, Jan Kiszka wrote:
> On 2018-04-28 00:24, Bjorn Helgaas wrote:
> > On Tue, Apr 24, 2018 at 05:13:39PM +0200, 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, introduce a
> >> managed version of that service. This differs API-wise only in taking a
> >> reference to the associated device, rather than to the device tree node.
> >>
> >> As of_pci_get_host_bridge_resources is an exported interface, we cannot
> >> simply drop it at this point. After converting all in-tree users to the
> >> new API, we could phase out the unmanaged one over some grace period.
> >
> > It looks like it might be possible to split this into three or four
> > patches:
> >
> > 1) Factor __of_pci_get_host_bridge_resources() out of
> > of_pci_get_host_bridge_resources()
> >
> > 2) Add struct device * argument
> >
> > 3) Convert pr_info() to dev_info()
> >
> > 4) Add devm_of_pci_get_host_bridge_resources()
>
> Will do. I'm even considering
>
> 5) mark of_pci_get_host_bridge_resources() __deprecated, due to the leak
> and no remaining in-tree user - what do you think?
Sounds good.
It'd be nice if we had some guideline about deprecation -- whether we
actually need to mark things __deprecated, and then how long to wait
before actually removing them, but I don't see anything in
Documentation/.
Looks like it was added by cbe4097f8ae6 ("of/pci: Add support for
parsing PCI host bridge resources from DT") in v3.18, so it's been
around for a while and I guess it would be nice to have a grace period
before removing it.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-30 18:40 ` Bjorn Helgaas
@ 2018-04-30 18:43 ` Sinan Kaya
2018-05-02 5:39 ` Jan Kiszka
0 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2018-04-30 18:43 UTC (permalink / raw)
To: Bjorn Helgaas, Jan Kiszka
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, linux-arm-kernel
On 4/30/2018 2:40 PM, Bjorn Helgaas wrote:
> On Sat, Apr 28, 2018 at 09:28:47AM +0200, Jan Kiszka wrote:
>> On 2018-04-28 00:24, Bjorn Helgaas wrote:
>>> On Tue, Apr 24, 2018 at 05:13:39PM +0200, 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, introduce a
>>>> managed version of that service. This differs API-wise only in taking a
>>>> reference to the associated device, rather than to the device tree node.
>>>>
>>>> As of_pci_get_host_bridge_resources is an exported interface, we cannot
>>>> simply drop it at this point. After converting all in-tree users to the
>>>> new API, we could phase out the unmanaged one over some grace period.
>>>
>>> It looks like it might be possible to split this into three or four
>>> patches:
>>>
>>> 1) Factor __of_pci_get_host_bridge_resources() out of
>>> of_pci_get_host_bridge_resources()
>>>
>>> 2) Add struct device * argument
>>>
>>> 3) Convert pr_info() to dev_info()
>>>
>>> 4) Add devm_of_pci_get_host_bridge_resources()
>>
>> Will do. I'm even considering
>>
>> 5) mark of_pci_get_host_bridge_resources() __deprecated, due to the leak
>> and no remaining in-tree user - what do you think?
>
> Sounds good.
>
> It'd be nice if we had some guideline about deprecation -- whether we
> actually need to mark things __deprecated, and then how long to wait
> before actually removing them, but I don't see anything in
> Documentation/.
I'm under the impression that we don't quite care about out-of-tree drivers.
I have seen many times out-of-tree drivers to be broken due to API changes,
renames or even parameter meaning change.
If the plan is to remove the API, just remove the API today.
>
> Looks like it was added by cbe4097f8ae6 ("of/pci: Add support for
> parsing PCI host bridge resources from DT") in v3.18, so it's been
> around for a while and I guess it would be nice to have a grace period
> before removing it.
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources
2018-04-30 18:43 ` Sinan Kaya
@ 2018-05-02 5:39 ` Jan Kiszka
0 siblings, 0 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-05-02 5:39 UTC (permalink / raw)
To: Sinan Kaya, Bjorn Helgaas
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, linux-arm-kernel
On 2018-04-30 20:43, Sinan Kaya wrote:
> On 4/30/2018 2:40 PM, Bjorn Helgaas wrote:
>> On Sat, Apr 28, 2018 at 09:28:47AM +0200, Jan Kiszka wrote:
>>> On 2018-04-28 00:24, Bjorn Helgaas wrote:
>>>> On Tue, Apr 24, 2018 at 05:13:39PM +0200, 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, introduce a
>>>>> managed version of that service. This differs API-wise only in taking a
>>>>> reference to the associated device, rather than to the device tree node.
>>>>>
>>>>> As of_pci_get_host_bridge_resources is an exported interface, we cannot
>>>>> simply drop it at this point. After converting all in-tree users to the
>>>>> new API, we could phase out the unmanaged one over some grace period.
>>>>
>>>> It looks like it might be possible to split this into three or four
>>>> patches:
>>>>
>>>> 1) Factor __of_pci_get_host_bridge_resources() out of
>>>> of_pci_get_host_bridge_resources()
>>>>
>>>> 2) Add struct device * argument
>>>>
>>>> 3) Convert pr_info() to dev_info()
>>>>
>>>> 4) Add devm_of_pci_get_host_bridge_resources()
>>>
>>> Will do. I'm even considering
>>>
>>> 5) mark of_pci_get_host_bridge_resources() __deprecated, due to the leak
>>> and no remaining in-tree user - what do you think?
>>
>> Sounds good.
>>
>> It'd be nice if we had some guideline about deprecation -- whether we
>> actually need to mark things __deprecated, and then how long to wait
>> before actually removing them, but I don't see anything in
>> Documentation/.
>
> I'm under the impression that we don't quite care about out-of-tree drivers.
> I have seen many times out-of-tree drivers to be broken due to API changes,
> renames or even parameter meaning change.
>
> If the plan is to remove the API, just remove the API today.
I've already sent a __deprecated patch on Monday [1], in v2 of this
series [2]. Please vote against that there, and I will replace it with a
complete removal of that API.
Thanks,
Jan
[1] https://lkml.org/lkml/2018/4/30/22
[2] https://lkml.org/lkml/2018/4/30/24
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/6] PCI: Convert of_pci_get_host_bridge_resources users to devm variant
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
` (2 preceding siblings ...)
2018-04-24 15:13 ` [PATCH 3/6] PCI: Introduce devm_of_pci_get_host_bridge_resources Jan Kiszka
@ 2018-04-24 15:13 ` Jan Kiszka
2018-04-25 19:47 ` Jingoo Han
2018-04-24 15:13 ` [PATCH 5/6] PCI: Add support for unbinding the generic PCI host controller Jan Kiszka
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Jan Kiszka @ 2018-04-24 15:13 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
Cc: Jingoo Han, Joao Pinto, Lorenzo Pieralisi
From: Jan Kiszka <jan.kiszka@siemens.com>
Straightforward for all of them, no more leaks afterwards.
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 | 4 ++--
13 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 6c409079d514..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(np, 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 b04d37b3c5de..709f0d69e35b 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -815,14 +815,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 = 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 5008fd87956a..87748eaeaaed 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 = 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 0a4dea796663..167bf6f6b378 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 = 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 5b3876f5312b..ff2cd12b3978 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 = 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 0a0d7ee6d3c9..7b3ed6e34b6c 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 = 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 a6af62e0256d..49410c7ba0cc 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 = 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 e764a2a2693c..99c2022813e4 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 = 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");
return ret;
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 6ab28f29ac6a..6eb36c924983 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 = 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 f1e8f97ea1fb..27b97fcddf15 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 = 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 4839ae578711..64df768c795c 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 = devm_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..88c96e5669e0 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 = devm_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 6eab0bde2ab3..4f816c93e562 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -626,12 +626,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 = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, resources,
+ &iobase);
if (err)
return err;
--
2.13.6
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] PCI: Convert of_pci_get_host_bridge_resources users to devm variant
2018-04-24 15:13 ` [PATCH 4/6] PCI: Convert of_pci_get_host_bridge_resources users to devm variant Jan Kiszka
@ 2018-04-25 19:47 ` Jingoo Han
0 siblings, 0 replies; 19+ messages in thread
From: Jingoo Han @ 2018-04-25 19:47 UTC (permalink / raw)
To: 'Jan Kiszka', 'Bjorn Helgaas',
'Linux Kernel Mailing List',
linux-pci, linux-arm-kernel
Cc: 'Lorenzo Pieralisi', 'Joao Pinto'
> -----Original Message-----
> From: Jan Kiszka <jan.kiszka@siemens.com>
> Sent: Tuesday, April 24, 2018 11:14 AM
> To: Bjorn Helgaas <bhelgaas@google.com>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Cc: Jingoo Han <jingoohan1@gmail.com>; Joao Pinto
> <Joao.Pinto@synopsys.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Subject: [PATCH 4/6] PCI: Convert of_pci_get_host_bridge_resources users
> to devm variant
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Straightforward for all of them, no more leaks afterwards.
>
> CC: Jingoo Han <jingoohan1@gmail.com>
For drivers/pci/dwc/pcie-designware-host.c,
Acked-by: Jingoo Han <jingoo1han@gmail.com>
Best regards,
Jingoo Han
> 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 | 4 ++--
> 13 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-designware-host.c
> b/drivers/pci/dwc/pcie-designware-host.c
> index 6c409079d514..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(np, 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 b04d37b3c5de..709f0d69e35b 100644
> --- a/drivers/pci/host/pci-aardvark.c
> +++ b/drivers/pci/host/pci-aardvark.c
> @@ -815,14 +815,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 = 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 5008fd87956a..87748eaeaaed 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 = 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 0a4dea796663..167bf6f6b378 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 = 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 5b3876f5312b..ff2cd12b3978 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 = 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 0a0d7ee6d3c9..7b3ed6e34b6c 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 = 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 a6af62e0256d..49410c7ba0cc 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 = 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 e764a2a2693c..99c2022813e4 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 = 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");
> return ret;
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 6ab28f29ac6a..6eb36c924983 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 = 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 f1e8f97ea1fb..27b97fcddf15 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 = 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 4839ae578711..64df768c795c 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 = devm_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..88c96e5669e0 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 = devm_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 6eab0bde2ab3..4f816c93e562 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -626,12 +626,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 = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff, resources,
> + &iobase);
> if (err)
> return err;
>
> --
> 2.13.6
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/6] PCI: Add support for unbinding the generic PCI host controller
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
` (3 preceding siblings ...)
2018-04-24 15:13 ` [PATCH 4/6] PCI: Convert of_pci_get_host_bridge_resources users to devm variant Jan Kiszka
@ 2018-04-24 15:13 ` Jan Kiszka
2018-04-24 15:13 ` [PATCH 6/6] arm: Allow to enable PCI_DOMAINS manually Jan Kiszka
2018-04-27 22:16 ` [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Bjorn Helgaas
6 siblings, 0 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-04-24 15:13 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
Cc: Will Deacon, Lorenzo Pieralisi
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/6] arm: Allow to enable PCI_DOMAINS manually
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
` (4 preceding siblings ...)
2018-04-24 15:13 ` [PATCH 5/6] PCI: Add support for unbinding the generic PCI host controller Jan Kiszka
@ 2018-04-24 15:13 ` Jan Kiszka
2018-04-25 17:54 ` Lorenzo Pieralisi
2018-04-27 22:16 ` [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Bjorn Helgaas
6 siblings, 1 reply; 19+ messages in thread
From: Jan Kiszka @ 2018-04-24 15:13 UTC (permalink / raw)
To: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
From: Jan Kiszka <jan.kiszka@siemens.com>
Required when running over Jailhouse, and there is already a physical
host controller that Jailhouse does not intercept and rather adds a
virtual one. That is the case for the Tegra TK1, e.g.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
arch/arm/Kconfig | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a7f8e7f4b88f..5f8190cb057d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1248,8 +1248,13 @@ config PCI
VESA. If you have PCI, say Y, otherwise N.
config PCI_DOMAINS
- bool
+ bool "Support for multiple PCI domains"
depends on PCI
+ help
+ Automatically enabled if the platform supports multiple PCI host
+ controllers. Say Y if running over a hypervisor like Jailhouse that
+ dynamically adds further host controllers while the system is
+ running. Say N otherwise.
config PCI_DOMAINS_GENERIC
def_bool PCI_DOMAINS
--
2.13.6
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] arm: Allow to enable PCI_DOMAINS manually
2018-04-24 15:13 ` [PATCH 6/6] arm: Allow to enable PCI_DOMAINS manually Jan Kiszka
@ 2018-04-25 17:54 ` Lorenzo Pieralisi
2018-04-26 7:19 ` Jan Kiszka
0 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2018-04-25 17:54 UTC (permalink / raw)
To: Jan Kiszka
Cc: Bjorn Helgaas, linux-pci, Linux Kernel Mailing List, linux-arm-kernel
On Tue, Apr 24, 2018 at 05:13:42PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Required when running over Jailhouse, and there is already a physical
> host controller that Jailhouse does not intercept and rather adds a
> virtual one. That is the case for the Tegra TK1, e.g.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> arch/arm/Kconfig | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a7f8e7f4b88f..5f8190cb057d 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1248,8 +1248,13 @@ config PCI
> VESA. If you have PCI, say Y, otherwise N.
>
> config PCI_DOMAINS
> - bool
> + bool "Support for multiple PCI domains"
> depends on PCI
> + help
> + Automatically enabled if the platform supports multiple PCI host
> + controllers. Say Y if running over a hypervisor like Jailhouse that
> + dynamically adds further host controllers while the system is
> + running. Say N otherwise.
Alternatively, you could select it under PCI_HOST_GENERIC if that's all
you need in Jailhouse. Actually that's a change that makes sense anyway
I think.
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] arm: Allow to enable PCI_DOMAINS manually
2018-04-25 17:54 ` Lorenzo Pieralisi
@ 2018-04-26 7:19 ` Jan Kiszka
0 siblings, 0 replies; 19+ messages in thread
From: Jan Kiszka @ 2018-04-26 7:19 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci, linux-arm-kernel
On 2018-04-25 19:54, Lorenzo Pieralisi wrote:
> On Tue, Apr 24, 2018 at 05:13:42PM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Required when running over Jailhouse, and there is already a physical
>> host controller that Jailhouse does not intercept and rather adds a
>> virtual one. That is the case for the Tegra TK1, e.g.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> arch/arm/Kconfig | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index a7f8e7f4b88f..5f8190cb057d 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1248,8 +1248,13 @@ config PCI
>> VESA. If you have PCI, say Y, otherwise N.
>>
>> config PCI_DOMAINS
>> - bool
>> + bool "Support for multiple PCI domains"
>> depends on PCI
>> + help
>> + Automatically enabled if the platform supports multiple PCI host
>> + controllers. Say Y if running over a hypervisor like Jailhouse that
>> + dynamically adds further host controllers while the system is
>> + running. Say N otherwise.
>
> Alternatively, you could select it under PCI_HOST_GENERIC if that's all
> you need in Jailhouse. Actually that's a change that makes sense anyway
> I think.
If that's preferred - will switch!
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff
2018-04-24 15:13 [PATCH 0/6] PCI: leak fixes, removable generic PCI host, assorted stuff Jan Kiszka
` (5 preceding siblings ...)
2018-04-24 15:13 ` [PATCH 6/6] arm: Allow to enable PCI_DOMAINS manually Jan Kiszka
@ 2018-04-27 22:16 ` Bjorn Helgaas
6 siblings, 0 replies; 19+ messages in thread
From: Bjorn Helgaas @ 2018-04-27 22:16 UTC (permalink / raw)
To: Jan Kiszka
Cc: Bjorn Helgaas, Linux Kernel Mailing List, linux-pci,
linux-arm-kernel, Jingoo Han, Joao Pinto, Lorenzo Pieralisi,
Will Deacon
On Tue, Apr 24, 2018 at 05:13:36PM +0200, Jan Kiszka wrote:
> 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.
Sounds like you have a couple things lined up for a v2 posting. When you
do that, would you mind updating the function references in subjects,
changelogs, and comments to include "()" after the function name, e.g.,
s/pci_get_new_domain_nr/pci_get_new_domain_nr()/?
> [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 (6):
> PCI: Make pci_get_new_domain_nr static
> PCI: Fix memory leak of devm_pci_alloc_host_bridge
> PCI: Introduce devm_of_pci_get_host_bridge_resources
> PCI: Convert of_pci_get_host_bridge_resources users to devm variant
> PCI: Add support for unbinding the generic PCI host controller
> arm: Allow to enable PCI_DOMAINS manually
>
> arch/arm/Kconfig | 7 ++-
> 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-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 | 93 ++++++++++++++++++++++------------
> drivers/pci/pci.c | 6 +--
> drivers/pci/probe.c | 4 +-
> include/linux/of_pci.h | 14 ++++-
> include/linux/pci-ecam.h | 1 +
> include/linux/pci.h | 3 --
> 21 files changed, 120 insertions(+), 68 deletions(-)
>
> --
> 2.13.6
>
^ permalink raw reply [flat|nested] 19+ messages in thread