[PATCH 0/4] Various PCI-related device tree helpers

[PATCH 0/4] Various PCI-related device tree helpers

[Thierry Reding]

Hi,

These patches add a few device tree helpers that are used are partially
shared by Thomas' Marvell PCIe controller and my Tegra PCIe controller
series. In an attempt to decrease the number of dependencies between
trees, it would be nice to get these in for 3.9. There are a few ARM
specific patches that the series depend on which have also been
submitted and will hopefully make it into 3.9 so we can use the 3.9
cycle to focus on getting the driver patches merged.

Thierry

Andrew Murray (1):
  of/pci: Provide support for parsing PCI DT ranges property

Thierry Reding (3):
  of/pci: Add of_pci_get_devfn() function
  of/pci: Add of_pci_get_bus() function
  of/pci: Add of_pci_parse_bus_range() function

 drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++
 drivers/of/of_pci.c        | 80 +++++++++++++++++++++++++++++++++++++++++++---
 include/linux/of_address.h |  9 ++++++
 include/linux/of_pci.h     |  3 ++
 4 files changed, 150 insertions(+), 5 deletions(-)

-- 
1.8.1.2

[PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thierry Reding]

From: Andrew Murray <andrew.murray@arm.com>

DT bindings for PCI host bridges often use the ranges property to describe
memory and IO ranges - this binding tends to be the same across architectures
yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
functionality provided by drivers/of/address.c.

This patch provides a common iterator-based parser for the ranges property, it
is hoped this will reduce DT representation differences between architectures
and that architectures will migrate in part to this new parser.

It is also hoped (and the motativation for the patch) that this patch will
reduce duplication of code when writing host bridge drivers that are supported
by multiple architectures.

This patch provides struct resources from a device tree node, e.g.:

	u32 *last = NULL;
	struct resource res;
	while ((last = of_pci_process_ranges(np, res, last))) {
		//do something with res
	}

Platforms with quirks can then do what they like with the resource or migrate
common quirk handling to the parser. In an ideal world drivers can just request
the obtained resources and pass them on (e.g. pci_add_resource_offset).

Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_address.h |  9 +++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 04da786..f607008 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -13,6 +13,7 @@
 #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
 
 static struct of_bus *of_match_bus(struct device_node *np);
+static struct of_bus *of_find_bus(const char *name);
 static int __of_address_to_resource(struct device_node *dev,
 		const __be32 *addrp, u64 size, unsigned int flags,
 		const char *name, struct resource *r);
@@ -227,6 +228,57 @@ int of_pci_address_to_resource(struct device_node *dev, int bar,
 	return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
 }
 EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
+
+const __be32 *of_pci_process_ranges(struct device_node *node,
+				    struct resource *res, const __be32 *from)
+{
+	const __be32 *start, *end;
+	int na, ns, np, pna;
+	int rlen;
+	struct of_bus *bus;
+
+	WARN_ON(!res);
+
+	bus = of_find_bus("pci");
+	bus->count_cells(node, &na, &ns);
+	if (!OF_CHECK_COUNTS(na, ns)) {
+		pr_err("Bad cell count for %s\n", node->full_name);
+		return NULL;
+	}
+
+	pna = of_n_addr_cells(node);
+	np = pna + na + ns;
+
+	start = of_get_property(node, "ranges", &rlen);
+	if (start == NULL)
+		return NULL;
+
+	end = start + rlen / sizeof(__be32);
+
+	if (!from)
+		from = start;
+
+	while (from + np <= end) {
+		u64 cpu_addr, size;
+
+		cpu_addr = of_translate_address(node, from + na);
+		size = of_read_number(from + na + pna, ns);
+		res->flags = bus->get_flags(from);
+		from += np;
+
+		if (cpu_addr == OF_BAD_ADDR || size == 0)
+			continue;
+
+		res->name = node->full_name;
+		res->start = cpu_addr;
+		res->end = res->start + size - 1;
+		res->parent = res->child = res->sibling = NULL;
+		return from;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_pci_process_ranges);
 #endif /* CONFIG_PCI */
 
 /*
@@ -337,6 +389,17 @@ static struct of_bus *of_match_bus(struct device_node *np)
 	return NULL;
 }
 
+static struct of_bus *of_find_bus(const char *name)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
+		if (strcmp(name, of_busses[i].name) == 0)
+			return &of_busses[i];
+
+	return NULL;
+}
+
 static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 			    struct of_bus *pbus, __be32 *addr,
 			    int na, int ns, int pna, const char *rprop)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 0506eb5..751e889 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -27,6 +27,8 @@ static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; }
 #define pci_address_to_pio pci_address_to_pio
 #endif
 
+const __be32 *of_pci_process_ranges(struct device_node *node,
+				    struct resource *res, const __be32 *from);
 #else /* CONFIG_OF_ADDRESS */
 #ifndef of_address_to_resource
 static inline int of_address_to_resource(struct device_node *dev, int index,
@@ -53,6 +55,13 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
 {
 	return NULL;
 }
+
+static inline const __be32 *of_pci_process_ranges(struct device_node *node,
+						  struct resource *res,
+						  const __be32 *from)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF_ADDRESS */
 
 
-- 
1.8.1.2

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Rob Herring]

On 02/11/2013 02:22 AM, Thierry Reding wrote:
> From: Andrew Murray <andrew.murray@arm.com>
> 
> DT bindings for PCI host bridges often use the ranges property to describe
> memory and IO ranges - this binding tends to be the same across architectures
> yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
> functionality provided by drivers/of/address.c.
> 
> This patch provides a common iterator-based parser for the ranges property, it
> is hoped this will reduce DT representation differences between architectures
> and that architectures will migrate in part to this new parser.
> 
> It is also hoped (and the motativation for the patch) that this patch will
> reduce duplication of code when writing host bridge drivers that are supported
> by multiple architectures.
> 
> This patch provides struct resources from a device tree node, e.g.:
> 
> 	u32 *last = NULL;
> 	struct resource res;
> 	while ((last = of_pci_process_ranges(np, res, last))) {
> 		//do something with res
> 	}
> 
> Platforms with quirks can then do what they like with the resource or migrate
> common quirk handling to the parser. In an ideal world drivers can just request
> the obtained resources and pass them on (e.g. pci_add_resource_offset).
> 
> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_address.h |  9 +++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 04da786..f607008 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -13,6 +13,7 @@
>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
>  
>  static struct of_bus *of_match_bus(struct device_node *np);
> +static struct of_bus *of_find_bus(const char *name);

Can you move this function up to avoid the forward declaration.

>  static int __of_address_to_resource(struct device_node *dev,
>  		const __be32 *addrp, u64 size, unsigned int flags,
>  		const char *name, struct resource *r);
> @@ -227,6 +228,57 @@ int of_pci_address_to_resource(struct device_node *dev, int bar,
>  	return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
>  }
>  EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
> +
> +const __be32 *of_pci_process_ranges(struct device_node *node,
> +				    struct resource *res, const __be32 *from)
> +{
> +	const __be32 *start, *end;
> +	int na, ns, np, pna;
> +	int rlen;
> +	struct of_bus *bus;
> +
> +	WARN_ON(!res);
> +
> +	bus = of_find_bus("pci");
> +	bus->count_cells(node, &na, &ns);
> +	if (!OF_CHECK_COUNTS(na, ns)) {
> +		pr_err("Bad cell count for %s\n", node->full_name);
> +		return NULL;
> +	}
> +
> +	pna = of_n_addr_cells(node);
> +	np = pna + na + ns;
> +
> +	start = of_get_property(node, "ranges", &rlen);
> +	if (start == NULL)
> +		return NULL;
> +
> +	end = start + rlen / sizeof(__be32);
> +
> +	if (!from)
> +		from = start;
> +
> +	while (from + np <= end) {
> +		u64 cpu_addr, size;
> +
> +		cpu_addr = of_translate_address(node, from + na);
> +		size = of_read_number(from + na + pna, ns);
> +		res->flags = bus->get_flags(from);
> +		from += np;
> +
> +		if (cpu_addr == OF_BAD_ADDR || size == 0)
> +			continue;
> +
> +		res->name = node->full_name;
> +		res->start = cpu_addr;
> +		res->end = res->start + size - 1;
> +		res->parent = res->child = res->sibling = NULL;
> +		return from;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_process_ranges);
>  #endif /* CONFIG_PCI */
>  
>  /*
> @@ -337,6 +389,17 @@ static struct of_bus *of_match_bus(struct device_node *np)
>  	return NULL;
>  }
>  
> +static struct of_bus *of_find_bus(const char *name)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
> +		if (strcmp(name, of_busses[i].name) == 0)
                                              ^
space needed.

> +			return &of_busses[i];
> +
> +	return NULL;
> +}
> +
>  static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>  			    struct of_bus *pbus, __be32 *addr,
>  			    int na, int ns, int pna, const char *rprop)
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 0506eb5..751e889 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -27,6 +27,8 @@ static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; }
>  #define pci_address_to_pio pci_address_to_pio
>  #endif
>  
> +const __be32 *of_pci_process_ranges(struct device_node *node,
> +				    struct resource *res, const __be32 *from);
>  #else /* CONFIG_OF_ADDRESS */
>  #ifndef of_address_to_resource
>  static inline int of_address_to_resource(struct device_node *dev, int index,
> @@ -53,6 +55,13 @@ static inline const __be32 *of_get_address(struct device_node *dev, int index,
>  {
>  	return NULL;
>  }
> +
> +static inline const __be32 *of_pci_process_ranges(struct device_node *node,
> +						  struct resource *res,
> +						  const __be32 *from)
> +{
> +	return NULL;
> +}
>  #endif /* CONFIG_OF_ADDRESS */
>  
>  
> 

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2980 bytes --]

On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
> On 02/11/2013 02:22 AM, Thierry Reding wrote:
> > From: Andrew Murray <andrew.murray@arm.com>
> > 
> > DT bindings for PCI host bridges often use the ranges property to describe
> > memory and IO ranges - this binding tends to be the same across architectures
> > yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> > arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> > arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
> > functionality provided by drivers/of/address.c.
> > 
> > This patch provides a common iterator-based parser for the ranges property, it
> > is hoped this will reduce DT representation differences between architectures
> > and that architectures will migrate in part to this new parser.
> > 
> > It is also hoped (and the motativation for the patch) that this patch will
> > reduce duplication of code when writing host bridge drivers that are supported
> > by multiple architectures.
> > 
> > This patch provides struct resources from a device tree node, e.g.:
> > 
> > 	u32 *last = NULL;
> > 	struct resource res;
> > 	while ((last = of_pci_process_ranges(np, res, last))) {
> > 		//do something with res
> > 	}
> > 
> > Platforms with quirks can then do what they like with the resource or migrate
> > common quirk handling to the parser. In an ideal world drivers can just request
> > the obtained resources and pass them on (e.g. pci_add_resource_offset).
> > 
> > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> >  drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/of_address.h |  9 +++++++
> >  2 files changed, 72 insertions(+)
> > 
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 04da786..f607008 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -13,6 +13,7 @@
> >  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
> >  
> >  static struct of_bus *of_match_bus(struct device_node *np);
> > +static struct of_bus *of_find_bus(const char *name);
> 
> Can you move this function up to avoid the forward declaration.

It needs to be defined after the of_busses structure, which is defined
below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
have to move that one as well and add another #ifdef CONFIG_PCI section.
If you prefer that I can do that.

> > +static struct of_bus *of_find_bus(const char *name)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
> > +		if (strcmp(name, of_busses[i].name) == 0)
>                                               ^
> space needed.

I don't understand. Do you want the space to go between '.' and "name"?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Rob Herring]

On 02/12/2013 12:45 AM, Thierry Reding wrote:
> On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
>> On 02/11/2013 02:22 AM, Thierry Reding wrote:
>>> From: Andrew Murray <andrew.murray@arm.com>

>>> @@ -13,6 +13,7 @@
>>>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
>>>  
>>>  static struct of_bus *of_match_bus(struct device_node *np);
>>> +static struct of_bus *of_find_bus(const char *name);
>>
>> Can you move this function up to avoid the forward declaration.
> 
> It needs to be defined after the of_busses structure, which is defined
> below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
> have to move that one as well and add another #ifdef CONFIG_PCI section.
> If you prefer that I can do that.

Okay, it's fine as is.

>>> +static struct of_bus *of_find_bus(const char *name)
>>> +{
>>> +	unsigned int i;
>>> +
>>> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
>>> +		if (strcmp(name, of_busses[i].name) == 0)
>>                                               ^
>> space needed.
> 
> I don't understand. Do you want the space to go between '.' and "name"?

Must have been some dirt on my screen... Never mind.

I'll apply these for 3.9.

Rob

> 
> Thierry
> 

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On Wed, Feb 13, 2013 at 08:23:28AM -0600, Rob Herring wrote:
> On 02/12/2013 12:45 AM, Thierry Reding wrote:
> > On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
> >> On 02/11/2013 02:22 AM, Thierry Reding wrote:
> >>> From: Andrew Murray <andrew.murray@arm.com>
> 
> >>> @@ -13,6 +13,7 @@
> >>>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
> >>>  
> >>>  static struct of_bus *of_match_bus(struct device_node *np);
> >>> +static struct of_bus *of_find_bus(const char *name);
> >>
> >> Can you move this function up to avoid the forward declaration.
> > 
> > It needs to be defined after the of_busses structure, which is defined
> > below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
> > have to move that one as well and add another #ifdef CONFIG_PCI section.
> > If you prefer that I can do that.
> 
> Okay, it's fine as is.
> 
> >>> +static struct of_bus *of_find_bus(const char *name)
> >>> +{
> >>> +	unsigned int i;
> >>> +
> >>> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
> >>> +		if (strcmp(name, of_busses[i].name) == 0)
> >>                                               ^
> >> space needed.
> > 
> > I don't understand. Do you want the space to go between '.' and "name"?
> 
> Must have been some dirt on my screen... Never mind.
> 
> I'll apply these for 3.9.

Great, thanks!

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Rob Herring]

On 02/13/2013 08:25 AM, Thierry Reding wrote:
> On Wed, Feb 13, 2013 at 08:23:28AM -0600, Rob Herring wrote:
>> On 02/12/2013 12:45 AM, Thierry Reding wrote:
>>> On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
>>>> On 02/11/2013 02:22 AM, Thierry Reding wrote:
>>>>> From: Andrew Murray <andrew.murray@arm.com>
>>
>>>>> @@ -13,6 +13,7 @@
>>>>>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
>>>>>  
>>>>>  static struct of_bus *of_match_bus(struct device_node *np);
>>>>> +static struct of_bus *of_find_bus(const char *name);
>>>>
>>>> Can you move this function up to avoid the forward declaration.
>>>
>>> It needs to be defined after the of_busses structure, which is defined
>>> below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
>>> have to move that one as well and add another #ifdef CONFIG_PCI section.
>>> If you prefer that I can do that.
>>
>> Okay, it's fine as is.
>>
>>>>> +static struct of_bus *of_find_bus(const char *name)
>>>>> +{
>>>>> +	unsigned int i;
>>>>> +
>>>>> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
>>>>> +		if (strcmp(name, of_busses[i].name) == 0)
>>>>                                               ^
>>>> space needed.
>>>
>>> I don't understand. Do you want the space to go between '.' and "name"?
>>
>> Must have been some dirt on my screen... Never mind.
>>
>> I'll apply these for 3.9.
> 
> Great, thanks!

Grant vetoed merging. We need to see the other architectures using these
functions rather than add yet another copy.

Rob

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

On Wed, Feb 13, 2013 at 01:54:53PM -0600, Rob Herring wrote:
> On 02/13/2013 08:25 AM, Thierry Reding wrote:
> > On Wed, Feb 13, 2013 at 08:23:28AM -0600, Rob Herring wrote:
> >> On 02/12/2013 12:45 AM, Thierry Reding wrote:
> >>> On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
> >>>> On 02/11/2013 02:22 AM, Thierry Reding wrote:
> >>>>> From: Andrew Murray <andrew.murray@arm.com>
> >>
> >>>>> @@ -13,6 +13,7 @@
> >>>>>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
> >>>>>  
> >>>>>  static struct of_bus *of_match_bus(struct device_node *np);
> >>>>> +static struct of_bus *of_find_bus(const char *name);
> >>>>
> >>>> Can you move this function up to avoid the forward declaration.
> >>>
> >>> It needs to be defined after the of_busses structure, which is defined
> >>> below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
> >>> have to move that one as well and add another #ifdef CONFIG_PCI section.
> >>> If you prefer that I can do that.
> >>
> >> Okay, it's fine as is.
> >>
> >>>>> +static struct of_bus *of_find_bus(const char *name)
> >>>>> +{
> >>>>> +	unsigned int i;
> >>>>> +
> >>>>> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
> >>>>> +		if (strcmp(name, of_busses[i].name) == 0)
> >>>>                                               ^
> >>>> space needed.
> >>>
> >>> I don't understand. Do you want the space to go between '.' and "name"?
> >>
> >> Must have been some dirt on my screen... Never mind.
> >>
> >> I'll apply these for 3.9.
> > 
> > Great, thanks!
> 
> Grant vetoed merging. We need to see the other architectures using these
> functions rather than add yet another copy.

I think I've said this before, but converting the other architectures
isn't very trivial, mostly because each has a specific way of storing
the values read from these properties.

So unless we want to block any new drivers from being merged, the only
alternatives are to either merge these patches or include a copy of the
same code in each new driver that needs the functionality (or open-code
the functions in each driver).

There is quite a bit of rework required to unify the architecture
specific PCI frameworks and I think Bjorn and others have been making
quite a bit of progress in that direction. I think that these patches
can help refactor some of those parts.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Grant Likely]

On Wed, 13 Feb 2013 22:29:51 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> On Wed, Feb 13, 2013 at 01:54:53PM -0600, Rob Herring wrote:
> > On 02/13/2013 08:25 AM, Thierry Reding wrote:
> > > On Wed, Feb 13, 2013 at 08:23:28AM -0600, Rob Herring wrote:
> > >> On 02/12/2013 12:45 AM, Thierry Reding wrote:
> > >>> On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
> > >>>> On 02/11/2013 02:22 AM, Thierry Reding wrote:
> > >>>>> From: Andrew Murray <andrew.murray@arm.com>
> > >>
> > >>>>> @@ -13,6 +13,7 @@
> > >>>>>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
> > >>>>>  
> > >>>>>  static struct of_bus *of_match_bus(struct device_node *np);
> > >>>>> +static struct of_bus *of_find_bus(const char *name);
> > >>>>
> > >>>> Can you move this function up to avoid the forward declaration.
> > >>>
> > >>> It needs to be defined after the of_busses structure, which is defined
> > >>> below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
> > >>> have to move that one as well and add another #ifdef CONFIG_PCI section.
> > >>> If you prefer that I can do that.
> > >>
> > >> Okay, it's fine as is.
> > >>
> > >>>>> +static struct of_bus *of_find_bus(const char *name)
> > >>>>> +{
> > >>>>> +	unsigned int i;
> > >>>>> +
> > >>>>> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
> > >>>>> +		if (strcmp(name, of_busses[i].name) == 0)
> > >>>>                                               ^
> > >>>> space needed.
> > >>>
> > >>> I don't understand. Do you want the space to go between '.' and "name"?
> > >>
> > >> Must have been some dirt on my screen... Never mind.
> > >>
> > >> I'll apply these for 3.9.
> > > 
> > > Great, thanks!
> > 
> > Grant vetoed merging. We need to see the other architectures using these
> > functions rather than add yet another copy.
> 
> I think I've said this before, but converting the other architectures
> isn't very trivial, mostly because each has a specific way of storing
> the values read from these properties.

Sorry to be harsh, but this isn't new information. I've had to deal with
the pain more than once before of copied infrastructure that at some
time in the future needs to be merged again. Just looking at your patch
I can tell that it is directly derived from the powerpc
pci_process_bridge_OF_ranges() and which microblaze has already has a
verbatum copy of.

So, no, I'm not okay with it for v3.9. I don't want more copies of the
same code. This doesn't block your v3.10 drivers. When a better patch is
ready we can set up a separate branch with just the new functions in it
and the various subsystems can merge that in if needed to resolve
dependencies.

Instead, here is what you do; you've got the bones of a good approach,
but you need to show how it is derived from the powerpc approach. I'll
reply in specifics to the patches themselves, but I can definitely see
large blocks of code that can be moved out of powerpc & microblaze and
into drivers/of/address.c without getting into the platform-specific
PCI representations that you're concerned about.

Now, to be clear here, I'm asking you to change powerpc/microblaze code,
but I am *not asking you to test it*. This is a code move exercises, and
I will help you with it if you need.

> 
> So unless we want to block any new drivers from being merged, the only
> alternatives are to either merge these patches or include a copy of the
> same code in each new driver that needs the functionality (or open-code
> the functions in each driver).
> 
> There is quite a bit of rework required to unify the architecture
> specific PCI frameworks and I think Bjorn and others have been making
> quite a bit of progress in that direction. I think that these patches
> can help refactor some of those parts.
> 
> Thierry

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3800 bytes --]

On Wed, Feb 13, 2013 at 10:09:56PM +0000, Grant Likely wrote:
> On Wed, 13 Feb 2013 22:29:51 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> > On Wed, Feb 13, 2013 at 01:54:53PM -0600, Rob Herring wrote:
> > > On 02/13/2013 08:25 AM, Thierry Reding wrote:
> > > > On Wed, Feb 13, 2013 at 08:23:28AM -0600, Rob Herring wrote:
> > > >> On 02/12/2013 12:45 AM, Thierry Reding wrote:
> > > >>> On Mon, Feb 11, 2013 at 01:43:03PM -0600, Rob Herring wrote:
> > > >>>> On 02/11/2013 02:22 AM, Thierry Reding wrote:
> > > >>>>> From: Andrew Murray <andrew.murray@arm.com>
> > > >>
> > > >>>>> @@ -13,6 +13,7 @@
> > > >>>>>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
> > > >>>>>  
> > > >>>>>  static struct of_bus *of_match_bus(struct device_node *np);
> > > >>>>> +static struct of_bus *of_find_bus(const char *name);
> > > >>>>
> > > >>>> Can you move this function up to avoid the forward declaration.
> > > >>>
> > > >>> It needs to be defined after the of_busses structure, which is defined
> > > >>> below the CONFIG_PCI block where of_pci_process_ranges() is defined. I'd
> > > >>> have to move that one as well and add another #ifdef CONFIG_PCI section.
> > > >>> If you prefer that I can do that.
> > > >>
> > > >> Okay, it's fine as is.
> > > >>
> > > >>>>> +static struct of_bus *of_find_bus(const char *name)
> > > >>>>> +{
> > > >>>>> +	unsigned int i;
> > > >>>>> +
> > > >>>>> +	for (i = 0; i < ARRAY_SIZE(of_busses); i++)
> > > >>>>> +		if (strcmp(name, of_busses[i].name) == 0)
> > > >>>>                                               ^
> > > >>>> space needed.
> > > >>>
> > > >>> I don't understand. Do you want the space to go between '.' and "name"?
> > > >>
> > > >> Must have been some dirt on my screen... Never mind.
> > > >>
> > > >> I'll apply these for 3.9.
> > > > 
> > > > Great, thanks!
> > > 
> > > Grant vetoed merging. We need to see the other architectures using these
> > > functions rather than add yet another copy.
> > 
> > I think I've said this before, but converting the other architectures
> > isn't very trivial, mostly because each has a specific way of storing
> > the values read from these properties.
> 
> Sorry to be harsh, but this isn't new information. I've had to deal with
> the pain more than once before of copied infrastructure that at some
> time in the future needs to be merged again. Just looking at your patch
> I can tell that it is directly derived from the powerpc
> pci_process_bridge_OF_ranges() and which microblaze has already has a
> verbatum copy of.
> 
> So, no, I'm not okay with it for v3.9. I don't want more copies of the
> same code. This doesn't block your v3.10 drivers. When a better patch is
> ready we can set up a separate branch with just the new functions in it
> and the various subsystems can merge that in if needed to resolve
> dependencies.
> 
> Instead, here is what you do; you've got the bones of a good approach,
> but you need to show how it is derived from the powerpc approach. I'll
> reply in specifics to the patches themselves, but I can definitely see
> large blocks of code that can be moved out of powerpc & microblaze and
> into drivers/of/address.c without getting into the platform-specific
> PCI representations that you're concerned about.
> 
> Now, to be clear here, I'm asking you to change powerpc/microblaze code,
> but I am *not asking you to test it*. This is a code move exercises, and
> I will help you with it if you need.

Alright. I have no idea about how this is going to affect the timeframe,
though. Granted, this doesn't sound as painful as I had assumed, but it
is quite a bit of work and I have to see how I can squeeze it in with
everything else.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Grant Likely]

On Mon, 11 Feb 2013 09:22:17 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> From: Andrew Murray <andrew.murray@arm.com>
> 
> DT bindings for PCI host bridges often use the ranges property to describe
> memory and IO ranges - this binding tends to be the same across architectures
> yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
> functionality provided by drivers/of/address.c.

Hi Thierry,

Following from my comments on not merging these patches, here are my
comments on this patch...

> This patch provides a common iterator-based parser for the ranges property, it
> is hoped this will reduce DT representation differences between architectures
> and that architectures will migrate in part to this new parser.
> 
> It is also hoped (and the motativation for the patch) that this patch will
> reduce duplication of code when writing host bridge drivers that are supported
> by multiple architectures.
> 
> This patch provides struct resources from a device tree node, e.g.:
> 
> 	u32 *last = NULL;
> 	struct resource res;
> 	while ((last = of_pci_process_ranges(np, res, last))) {
> 		//do something with res
> 	}

The approach seems reasonable, but it isn't optimal. For one, the setup
code ends up getting run every time of_pci_process_ranges() gets called.
It would also be more user-friendly to wrap it up in a
"for_each_of_pci_range()" macro.

> Platforms with quirks can then do what they like with the resource or migrate
> common quirk handling to the parser. In an ideal world drivers can just request
> the obtained resources and pass them on (e.g. pci_add_resource_offset).
> 
> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_address.h |  9 +++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 04da786..f607008 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -13,6 +13,7 @@
>  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
>  
>  static struct of_bus *of_match_bus(struct device_node *np);
> +static struct of_bus *of_find_bus(const char *name);
>  static int __of_address_to_resource(struct device_node *dev,
>  		const __be32 *addrp, u64 size, unsigned int flags,
>  		const char *name, struct resource *r);
> @@ -227,6 +228,57 @@ int of_pci_address_to_resource(struct device_node *dev, int bar,
>  	return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
>  }
>  EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
> +
> +const __be32 *of_pci_process_ranges(struct device_node *node,
> +				    struct resource *res, const __be32 *from)
> +{
> +	const __be32 *start, *end;
> +	int na, ns, np, pna;
> +	int rlen;
> +	struct of_bus *bus;
> +
> +	WARN_ON(!res);
> +
> +	bus = of_find_bus("pci");
> +	bus->count_cells(node, &na, &ns);
> +	if (!OF_CHECK_COUNTS(na, ns)) {
> +		pr_err("Bad cell count for %s\n", node->full_name);
> +		return NULL;
> +	}

Looking up the pci of_bus structure isn't really warrented here. This
function will only ever be used on PCI busses, and na/ns for PCI is
always 3/2. Just use those numbers here. You could however validate that
the node has the correct values in #address-cells and #size-cells

> +
> +	pna = of_n_addr_cells(node);
> +	np = pna + na + ns;
> +
> +	start = of_get_property(node, "ranges", &rlen);
> +	if (start == NULL)
> +		return NULL;
> +
> +	end = start + rlen / sizeof(__be32);

The above structure means that the ranges property has to be looked up
each and every time this function is called. It would be better to have
a state structure that can look it up once and then keep track of the
iteration.

> +
> +	if (!from)
> +		from = start;
> +
> +	while (from + np <= end) {
> +		u64 cpu_addr, size;
> +
> +		cpu_addr = of_translate_address(node, from + na);
> +		size = of_read_number(from + na + pna, ns);
> +		res->flags = bus->get_flags(from);
> +		from += np;
> +
> +		if (cpu_addr == OF_BAD_ADDR || size == 0)
> +			continue;
> +
> +		res->name = node->full_name;
> +		res->start = cpu_addr;
> +		res->end = res->start + size - 1;
> +		res->parent = res->child = res->sibling = NULL;
> +		return from;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_process_ranges);

All of the above can be directly factored out of the PCI implementation.
You don't even need to touch most of the powerpc code. Create your
iterator helper functions in the same patch that makes PowerPC and
Microblaze use them, and then improve/modify the behaviour in seperate
patches. The delta to ppc/microblaze really will be very small with this
approach.  Here are the relevant PPC lines:

void pci_process_bridge_OF_ranges(struct pci_controller *hose,
				  struct device_node *dev, int primary)
{
	const u32 *ranges;
	int rlen;
	int pna = of_n_addr_cells(dev);
	int np = pna + 5;
	int memno = 0, isa_hole = -1;
	u32 pci_space;
	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
	unsigned long long isa_mb = 0;
	struct resource *res;

	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
	       dev->full_name, primary ? "(primary)" : "");

	/* Get ranges property */
	ranges = of_get_property(dev, "ranges", &rlen);
	if (ranges == NULL)
		return;

	/* Parse it */
	while ((rlen -= np * 4) >= 0) {
		/* Read next ranges element */
		pci_space = ranges[0];
		pci_addr = of_read_number(ranges + 1, 2);
		cpu_addr = of_translate_address(dev, ranges + 3);
		size = of_read_number(ranges + pna + 3, 2);
		ranges += np;

		/* If we failed translation or got a zero-sized region
		 * (some FW try to feed us with non sensical zero sized regions
		 * such as power3 which look like some kind of attempt at exposing
		 * the VGA memory hole)
		 */
		if (cpu_addr == OF_BAD_ADDR || size == 0)
			continue;

		/* Now consume following elements while they are contiguous */
		for (; rlen >= np * sizeof(u32);
		     ranges += np, rlen -= np * 4) {
			if (ranges[0] != pci_space)
				break;
			pci_next = of_read_number(ranges + 1, 2);
			cpu_next = of_translate_address(dev, ranges + 3);
			if (pci_next != pci_addr + size ||
			    cpu_next != cpu_addr + size)
				break;
			size += of_read_number(ranges + pna + 3, 2);
		}

After factoring out the bits you want to use it will probably look like
this:

void pci_process_bridge_OF_ranges(struct pci_controller *hose,
				  struct device_node *dev, int primary)
{
	const u32 *ranges;
	int memno = 0, isa_hole = -1;
	unsigned long long isa_mb = 0;
	struct resource *res;
	struct of_pci_range_iter iter;

	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
	       dev->full_name, primary ? "(primary)" : "");

	for_each_of_pci_range(iter, np) {
		/* Read next ranges element */
		u32 pci_space = iter->pci_space;
		unsigned long long pci_addr = iter->pci_addr;
		unsigned long long cpu_addr = iter->cpu_addr;
		unsigned long long size = iter->size;
		int pna = iter->pna;
		/* or the remainder of the body of this function could
		 * have 'iter->' prefixed to each reference, which is
		 * better, but also a bit more invasive */

here really shouldn't be any further changes the the rest of the
function. I don't think that is unreasonable to ask, and I can help with
putting this together if you need it. Plus it will /reduce/ the number
of lines in the kernel instead of adding to them. That is something I
always want more of. :-)

Actually, a lot of the powerpc behaviour should still be
relevant to all architectures, but I haven't dug that deep yet.

g.

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Andrew Murray]

On Wed, Feb 13, 2013 at 10:53:11PM +0000, Grant Likely wrote:
> On Mon, 11 Feb 2013 09:22:17 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> > From: Andrew Murray <andrew.murray@arm.com>
> > 
> > DT bindings for PCI host bridges often use the ranges property to describe
> > memory and IO ranges - this binding tends to be the same across architectures
> > yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> > arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> > arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
> > functionality provided by drivers/of/address.c.
> 
> Hi Thierry,
> 
> Following from my comments on not merging these patches, here are my
> comments on this patch...
> 
> > This patch provides a common iterator-based parser for the ranges property, it
> > is hoped this will reduce DT representation differences between architectures
> > and that architectures will migrate in part to this new parser.
> > 
> > It is also hoped (and the motativation for the patch) that this patch will
> > reduce duplication of code when writing host bridge drivers that are supported
> > by multiple architectures.
> > 
> > This patch provides struct resources from a device tree node, e.g.:
> > 
> > 	u32 *last = NULL;
> > 	struct resource res;
> > 	while ((last = of_pci_process_ranges(np, res, last))) {
> > 		//do something with res
> > 	}
> 
> The approach seems reasonable, but it isn't optimal. For one, the setup
> code ends up getting run every time of_pci_process_ranges() gets called.
> It would also be more user-friendly to wrap it up in a
> "for_each_of_pci_range()" macro.
> 
> > Platforms with quirks can then do what they like with the resource or migrate
> > common quirk handling to the parser. In an ideal world drivers can just request
> > the obtained resources and pass them on (e.g. pci_add_resource_offset).
> > 
> > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> >  drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/of_address.h |  9 +++++++
> >  2 files changed, 72 insertions(+)
> > 
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 04da786..f607008 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -13,6 +13,7 @@
> >  #define OF_CHECK_COUNTS(na, ns)	(OF_CHECK_ADDR_COUNT(na) && (ns) > 0)
> >  
> >  static struct of_bus *of_match_bus(struct device_node *np);
> > +static struct of_bus *of_find_bus(const char *name);
> >  static int __of_address_to_resource(struct device_node *dev,
> >  		const __be32 *addrp, u64 size, unsigned int flags,
> >  		const char *name, struct resource *r);
> > @@ -227,6 +228,57 @@ int of_pci_address_to_resource(struct device_node *dev, int bar,
> >  	return __of_address_to_resource(dev, addrp, size, flags, NULL, r);
> >  }
> >  EXPORT_SYMBOL_GPL(of_pci_address_to_resource);
> > +
> > +const __be32 *of_pci_process_ranges(struct device_node *node,
> > +				    struct resource *res, const __be32 *from)
> > +{
> > +	const __be32 *start, *end;
> > +	int na, ns, np, pna;
> > +	int rlen;
> > +	struct of_bus *bus;
> > +
> > +	WARN_ON(!res);
> > +
> > +	bus = of_find_bus("pci");
> > +	bus->count_cells(node, &na, &ns);
> > +	if (!OF_CHECK_COUNTS(na, ns)) {
> > +		pr_err("Bad cell count for %s\n", node->full_name);
> > +		return NULL;
> > +	}
> 
> Looking up the pci of_bus structure isn't really warrented here. This
> function will only ever be used on PCI busses, and na/ns for PCI is
> always 3/2. Just use those numbers here. You could however validate that
> the node has the correct values in #address-cells and #size-cells
> 
> > +
> > +	pna = of_n_addr_cells(node);
> > +	np = pna + na + ns;
> > +
> > +	start = of_get_property(node, "ranges", &rlen);
> > +	if (start == NULL)
> > +		return NULL;
> > +
> > +	end = start + rlen / sizeof(__be32);
> 
> The above structure means that the ranges property has to be looked up
> each and every time this function is called. It would be better to have
> a state structure that can look it up once and then keep track of the
> iteration.
> 
> > +
> > +	if (!from)
> > +		from = start;
> > +
> > +	while (from + np <= end) {
> > +		u64 cpu_addr, size;
> > +
> > +		cpu_addr = of_translate_address(node, from + na);
> > +		size = of_read_number(from + na + pna, ns);
> > +		res->flags = bus->get_flags(from);
> > +		from += np;
> > +
> > +		if (cpu_addr == OF_BAD_ADDR || size == 0)
> > +			continue;
> > +
> > +		res->name = node->full_name;
> > +		res->start = cpu_addr;
> > +		res->end = res->start + size - 1;
> > +		res->parent = res->child = res->sibling = NULL;
> > +		return from;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(of_pci_process_ranges);
> 
> All of the above can be directly factored out of the PCI implementation.
> You don't even need to touch most of the powerpc code. Create your
> iterator helper functions in the same patch that makes PowerPC and
> Microblaze use them, and then improve/modify the behaviour in seperate
> patches. The delta to ppc/microblaze really will be very small with this
> approach.  Here are the relevant PPC lines:
> 
> void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> 				  struct device_node *dev, int primary)
> {
> 	const u32 *ranges;
> 	int rlen;
> 	int pna = of_n_addr_cells(dev);
> 	int np = pna + 5;
> 	int memno = 0, isa_hole = -1;
> 	u32 pci_space;
> 	unsigned long long pci_addr, cpu_addr, pci_next, cpu_next, size;
> 	unsigned long long isa_mb = 0;
> 	struct resource *res;
> 
> 	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
> 	       dev->full_name, primary ? "(primary)" : "");
> 
> 	/* Get ranges property */
> 	ranges = of_get_property(dev, "ranges", &rlen);
> 	if (ranges == NULL)
> 		return;
> 
> 	/* Parse it */
> 	while ((rlen -= np * 4) >= 0) {
> 		/* Read next ranges element */
> 		pci_space = ranges[0];
> 		pci_addr = of_read_number(ranges + 1, 2);
> 		cpu_addr = of_translate_address(dev, ranges + 3);
> 		size = of_read_number(ranges + pna + 3, 2);
> 		ranges += np;
> 
> 		/* If we failed translation or got a zero-sized region
> 		 * (some FW try to feed us with non sensical zero sized regions
> 		 * such as power3 which look like some kind of attempt at exposing
> 		 * the VGA memory hole)
> 		 */
> 		if (cpu_addr == OF_BAD_ADDR || size == 0)
> 			continue;
> 
> 		/* Now consume following elements while they are contiguous */
> 		for (; rlen >= np * sizeof(u32);
> 		     ranges += np, rlen -= np * 4) {
> 			if (ranges[0] != pci_space)
> 				break;
> 			pci_next = of_read_number(ranges + 1, 2);
> 			cpu_next = of_translate_address(dev, ranges + 3);
> 			if (pci_next != pci_addr + size ||
> 			    cpu_next != cpu_addr + size)
> 				break;
> 			size += of_read_number(ranges + pna + 3, 2);
> 		}
> 
> After factoring out the bits you want to use it will probably look like
> this:
> 
> void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> 				  struct device_node *dev, int primary)
> {
> 	const u32 *ranges;
> 	int memno = 0, isa_hole = -1;
> 	unsigned long long isa_mb = 0;
> 	struct resource *res;
> 	struct of_pci_range_iter iter;
> 
> 	printk(KERN_INFO "PCI host bridge %s %s ranges:\n",
> 	       dev->full_name, primary ? "(primary)" : "");
> 
> 	for_each_of_pci_range(iter, np) {
> 		/* Read next ranges element */
> 		u32 pci_space = iter->pci_space;
> 		unsigned long long pci_addr = iter->pci_addr;
> 		unsigned long long cpu_addr = iter->cpu_addr;
> 		unsigned long long size = iter->size;
> 		int pna = iter->pna;
> 		/* or the remainder of the body of this function could
> 		 * have 'iter->' prefixed to each reference, which is
> 		 * better, but also a bit more invasive */
> 
> here really shouldn't be any further changes the the rest of the
> function. I don't think that is unreasonable to ask, and I can help with
> putting this together if you need it. Plus it will /reduce/ the number
> of lines in the kernel instead of adding to them. That is something I
> always want more of. :-)
> 
> Actually, a lot of the powerpc behaviour should still be
> relevant to all architectures, but I haven't dug that deep yet.

Thierry,

If you don't have much bandwidth I'd be quite happy to take this on - this
would be beneficial for my eventual patchset. I can start by refactoring common
implementations of pci_process_bridge_OF_ranges or similar across the
architectures as per Grant's suggestion? I didn't do this when I first posted
the patch as I was concerned about the testing effort.

Andrew Murray

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Thu, Feb 14, 2013 at 04:53:41PM +0000, Andrew Murray wrote:
> Thierry,
> 
> If you don't have much bandwidth I'd be quite happy to take this on - this
> would be beneficial for my eventual patchset. I can start by refactoring common
> implementations of pci_process_bridge_OF_ranges or similar across the
> architectures as per Grant's suggestion? I didn't do this when I first posted
> the patch as I was concerned about the testing effort.

Absolutely! Since it was your patch in the first place you're just as
well suited to do this if you want to and have the time.

Thanks,
Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Thomas Petazzoni]

Hello,

On Thu, 14 Feb 2013 20:17:45 +0100, Thierry Reding wrote:
> On Thu, Feb 14, 2013 at 04:53:41PM +0000, Andrew Murray wrote:
> > Thierry,
> > 
> > If you don't have much bandwidth I'd be quite happy to take this on - this
> > would be beneficial for my eventual patchset. I can start by refactoring common
> > implementations of pci_process_bridge_OF_ranges or similar across the
> > architectures as per Grant's suggestion? I didn't do this when I first posted
> > the patch as I was concerned about the testing effort.
> 
> Absolutely! Since it was your patch in the first place you're just as
> well suited to do this if you want to and have the time.

And I'll be more than happy to test your patches in the context of the
Marvell PCIe driver.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Linus Walleij]

n Thu, Feb 14, 2013 at 8:17 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Thu, Feb 14, 2013 at 04:53:41PM +0000, Andrew Murray wrote:
>> Thierry,
>>
>> If you don't have much bandwidth I'd be quite happy to take this on - this
>> would be beneficial for my eventual patchset. I can start by refactoring common
>> implementations of pci_process_bridge_OF_ranges or similar across the
>> architectures as per Grant's suggestion? I didn't do this when I first posted
>> the patch as I was concerned about the testing effort.
>
> Absolutely! Since it was your patch in the first place you're just as
> well suited to do this if you want to and have the time.

I am working on device tree patches for the Integrator/AP with it's
PCIv3 bridge, and I also follow this with great interest. I was almost
going to start the copy/paste cycle but now I think it's better if
I wait for this to happen.

Yours,
Linus Walleij

Re: [PATCH 1/4] of/pci: Provide support for parsing PCI DT ranges property

[Andrew Murray]

On Fri, Feb 15, 2013 at 01:16:17PM +0000, Linus Walleij wrote:
> n Thu, Feb 14, 2013 at 8:17 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > On Thu, Feb 14, 2013 at 04:53:41PM +0000, Andrew Murray wrote:
> >> Thierry,
> >>
> >> If you don't have much bandwidth I'd be quite happy to take this on - this
> >> would be beneficial for my eventual patchset. I can start by refactoring common
> >> implementations of pci_process_bridge_OF_ranges or similar across the
> >> architectures as per Grant's suggestion? I didn't do this when I first posted
> >> the patch as I was concerned about the testing effort.
> >
> > Absolutely! Since it was your patch in the first place you're just as
> > well suited to do this if you want to and have the time.
> 
> I am working on device tree patches for the Integrator/AP with it's
> PCIv3 bridge, and I also follow this with great interest. I was almost
> going to start the copy/paste cycle but now I think it's better if
> I wait for this to happen.

No problem - I am currently looking at this.

Andrew Murray

[PATCH v2 2/4] of/pci: Add of_pci_get_devfn() function

[Thierry Reding]

This function can be used to parse the device and function number from a
standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
the returned value obtain the device and function numbers respectively.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
Changes in v2:
- rename devfn and err variables for clarity

 drivers/of/of_pci.c    | 34 +++++++++++++++++++++++++++++-----
 include/linux/of_pci.h |  1 +
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 13e37e2..4dd7b9b 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -5,14 +5,15 @@
 #include <asm/prom.h>
 
 static inline int __of_pci_pci_compare(struct device_node *node,
-				       unsigned int devfn)
+				       unsigned int data)
 {
-	unsigned int size;
-	const __be32 *reg = of_get_property(node, "reg", &size);
+	int devfn;
 
-	if (!reg || size < 5 * sizeof(__be32))
+	devfn = of_pci_get_devfn(node);
+	if (devfn < 0)
 		return 0;
-	return ((be32_to_cpup(&reg[0]) >> 8) & 0xff) == devfn;
+
+	return devfn == data;
 }
 
 struct device_node *of_pci_find_child_device(struct device_node *parent,
@@ -40,3 +41,26 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(of_pci_find_child_device);
+
+/**
+ * of_pci_get_devfn() - Get device and function numbers for a device node
+ * @np: device node
+ *
+ * Parses a standard 5-cell PCI resource and returns an 8-bit value that can
+ * be passed to the PCI_SLOT() and PCI_FUNC() macros to extract the device
+ * and function numbers respectively. On error a negative error code is
+ * returned.
+ */
+int of_pci_get_devfn(struct device_node *np)
+{
+	unsigned int size;
+	const __be32 *reg;
+
+	reg = of_get_property(np, "reg", &size);
+
+	if (!reg || size < 5 * sizeof(__be32))
+		return -EINVAL;
+
+	return (be32_to_cpup(reg) >> 8) & 0xff;
+}
+EXPORT_SYMBOL_GPL(of_pci_get_devfn);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bb115de..91ec484 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -10,5 +10,6 @@ int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq *out_irq);
 struct device_node;
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
+int of_pci_get_devfn(struct device_node *np);
 
 #endif
-- 
1.8.1.2

Re: [PATCH v2 2/4] of/pci: Add of_pci_get_devfn() function

[Grant Likely]

On Mon, 11 Feb 2013 09:22:18 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> This function can be used to parse the device and function number from a
> standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
> the returned value obtain the device and function numbers respectively.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
> Changes in v2:
> - rename devfn and err variables for clarity
> 
>  drivers/of/of_pci.c    | 34 +++++++++++++++++++++++++++++-----
>  include/linux/of_pci.h |  1 +

There isn't a whole lot of value in this patch without another user.
I'll need to see the other patches that make use of this.

g.

>  2 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 13e37e2..4dd7b9b 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -5,14 +5,15 @@
>  #include <asm/prom.h>
>  
>  static inline int __of_pci_pci_compare(struct device_node *node,
> -				       unsigned int devfn)
> +				       unsigned int data)
>  {
> -	unsigned int size;
> -	const __be32 *reg = of_get_property(node, "reg", &size);
> +	int devfn;
>  
> -	if (!reg || size < 5 * sizeof(__be32))
> +	devfn = of_pci_get_devfn(node);
> +	if (devfn < 0)
>  		return 0;
> -	return ((be32_to_cpup(&reg[0]) >> 8) & 0xff) == devfn;
> +
> +	return devfn == data;
>  }
>  
>  struct device_node *of_pci_find_child_device(struct device_node *parent,
> @@ -40,3 +41,26 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
> +
> +/**
> + * of_pci_get_devfn() - Get device and function numbers for a device node
> + * @np: device node
> + *
> + * Parses a standard 5-cell PCI resource and returns an 8-bit value that can
> + * be passed to the PCI_SLOT() and PCI_FUNC() macros to extract the device
> + * and function numbers respectively. On error a negative error code is
> + * returned.
> + */
> +int of_pci_get_devfn(struct device_node *np)
> +{
> +	unsigned int size;
> +	const __be32 *reg;
> +
> +	reg = of_get_property(np, "reg", &size);
> +
> +	if (!reg || size < 5 * sizeof(__be32))
> +		return -EINVAL;
> +
> +	return (be32_to_cpup(reg) >> 8) & 0xff;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_get_devfn);
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index bb115de..91ec484 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -10,5 +10,6 @@ int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq *out_irq);
>  struct device_node;
>  struct device_node *of_pci_find_child_device(struct device_node *parent,
>  					     unsigned int devfn);
> +int of_pci_get_devfn(struct device_node *np);
>  
>  #endif
> -- 
> 1.8.1.2
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH v2 2/4] of/pci: Add of_pci_get_devfn() function

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On Wed, Feb 13, 2013 at 10:59:50PM +0000, Grant Likely wrote:
> On Mon, 11 Feb 2013 09:22:18 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> > This function can be used to parse the device and function number from a
> > standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
> > the returned value obtain the device and function numbers respectively.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> > Changes in v2:
> > - rename devfn and err variables for clarity
> > 
> >  drivers/of/of_pci.c    | 34 +++++++++++++++++++++++++++++-----
> >  include/linux/of_pci.h |  1 +
> 
> There isn't a whole lot of value in this patch without another user.
> I'll need to see the other patches that make use of this.

Alright, I'll take that back into the Tegra series as well.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/4] of/pci: Add of_pci_get_bus() function

[Thierry Reding]

This function can be used to parse the number of a device's parent PCI
bus from a standard 5-cell PCI resource.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 drivers/of/of_pci.c    | 21 +++++++++++++++++++++
 include/linux/of_pci.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 4dd7b9b..d6e6de5 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -43,6 +43,27 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 EXPORT_SYMBOL_GPL(of_pci_find_child_device);
 
 /**
+ * of_pci_get_bus() - Get bus number for a device node
+ * @np: device node
+ *
+ * Parses a standard 5-cell PCI resource and returns the 8-bit bus number of
+ * the device's parent PCI bus. On error a negative error code is returned.
+ */
+int of_pci_get_bus(struct device_node *np)
+{
+	unsigned int size;
+	const __be32 *reg;
+
+	reg = of_get_property(np, "reg", &size);
+
+	if (!reg || size < 5 * sizeof(__be32))
+		return -EINVAL;
+
+	return (be32_to_cpup(reg) >> 16) & 0xff;
+}
+EXPORT_SYMBOL_GPL(of_pci_get_bus);
+
+/**
  * of_pci_get_devfn() - Get device and function numbers for a device node
  * @np: device node
  *
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 91ec484..9118321 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -10,6 +10,7 @@ int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq *out_irq);
 struct device_node;
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
+int of_pci_get_bus(struct device_node *np);
 int of_pci_get_devfn(struct device_node *np);
 
 #endif
-- 
1.8.1.2

Re: [PATCH 3/4] of/pci: Add of_pci_get_bus() function

[Grant Likely]

On Mon, 11 Feb 2013 09:22:19 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> This function can be used to parse the number of a device's parent PCI
> bus from a standard 5-cell PCI resource.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

This patch should be deferred until there is another patch in the same
series that actually uses it.

g.

> ---
>  drivers/of/of_pci.c    | 21 +++++++++++++++++++++
>  include/linux/of_pci.h |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 4dd7b9b..d6e6de5 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -43,6 +43,27 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
>  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
>  
>  /**
> + * of_pci_get_bus() - Get bus number for a device node
> + * @np: device node
> + *
> + * Parses a standard 5-cell PCI resource and returns the 8-bit bus number of
> + * the device's parent PCI bus. On error a negative error code is returned.
> + */
> +int of_pci_get_bus(struct device_node *np)
> +{
> +	unsigned int size;
> +	const __be32 *reg;
> +
> +	reg = of_get_property(np, "reg", &size);
> +
> +	if (!reg || size < 5 * sizeof(__be32))
> +		return -EINVAL;
> +
> +	return (be32_to_cpup(reg) >> 16) & 0xff;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_get_bus);
> +
> +/**
>   * of_pci_get_devfn() - Get device and function numbers for a device node
>   * @np: device node
>   *
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 91ec484..9118321 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -10,6 +10,7 @@ int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq *out_irq);
>  struct device_node;
>  struct device_node *of_pci_find_child_device(struct device_node *parent,
>  					     unsigned int devfn);
> +int of_pci_get_bus(struct device_node *np);
>  int of_pci_get_devfn(struct device_node *np);
>  
>  #endif
> -- 
> 1.8.1.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 3/4] of/pci: Add of_pci_get_bus() function

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 500 bytes --]

On Wed, Feb 13, 2013 at 10:56:07PM +0000, Grant Likely wrote:
> On Mon, 11 Feb 2013 09:22:19 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> > This function can be used to parse the number of a device's parent PCI
> > bus from a standard 5-cell PCI resource.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> 
> This patch should be deferred until there is another patch in the same
> series that actually uses it.

And this one too.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 4/4] of/pci: Add of_pci_parse_bus_range() function

[Thierry Reding]

This function can be used to parse a bus-range property as specified by
device nodes representing PCI bridges.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 drivers/of/of_pci.c    | 25 +++++++++++++++++++++++++
 include/linux/of_pci.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index d6e6de5..fb92ded 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -85,3 +85,28 @@ int of_pci_get_devfn(struct device_node *np)
 	return (be32_to_cpup(reg) >> 8) & 0xff;
 }
 EXPORT_SYMBOL_GPL(of_pci_get_devfn);
+
+/**
+ * of_pci_parse_bus_range() - parse the bus-range property of a PCI device
+ * @node: device node
+ * @res: address to a struct resource to return the bus-range
+ *
+ * Returns 0 on success or a negative error-code on failure.
+ */
+int of_pci_parse_bus_range(struct device_node *node, struct resource *res)
+{
+	const __be32 *values;
+	int len;
+
+	values = of_get_property(node, "bus-range", &len);
+	if (!values || len < sizeof(*values) * 2)
+		return -EINVAL;
+
+	res->name = node->name;
+	res->start = be32_to_cpup(values++);
+	res->end = be32_to_cpup(values);
+	res->flags = IORESOURCE_BUS;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_pci_parse_bus_range);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 9118321..fb6e95e 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -12,5 +12,6 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_bus(struct device_node *np);
 int of_pci_get_devfn(struct device_node *np);
+int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
 
 #endif
-- 
1.8.1.2

Re: [PATCH 4/4] of/pci: Add of_pci_parse_bus_range() function

[Grant Likely]

On Mon, 11 Feb 2013 09:22:20 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> This function can be used to parse a bus-range property as specified by
> device nodes representing PCI bridges.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

Ditto for this one. We can wait on it until there is a user.

> ---
>  drivers/of/of_pci.c    | 25 +++++++++++++++++++++++++
>  include/linux/of_pci.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index d6e6de5..fb92ded 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -85,3 +85,28 @@ int of_pci_get_devfn(struct device_node *np)
>  	return (be32_to_cpup(reg) >> 8) & 0xff;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_get_devfn);
> +
> +/**
> + * of_pci_parse_bus_range() - parse the bus-range property of a PCI device
> + * @node: device node
> + * @res: address to a struct resource to return the bus-range
> + *
> + * Returns 0 on success or a negative error-code on failure.
> + */
> +int of_pci_parse_bus_range(struct device_node *node, struct resource *res)
> +{
> +	const __be32 *values;
> +	int len;
> +
> +	values = of_get_property(node, "bus-range", &len);
> +	if (!values || len < sizeof(*values) * 2)
> +		return -EINVAL;
> +
> +	res->name = node->name;
> +	res->start = be32_to_cpup(values++);
> +	res->end = be32_to_cpup(values);
> +	res->flags = IORESOURCE_BUS;

Is there precedence for using struct resource for passing around the PCI
bus range values? Who will be the user of this function?

g.

Re: [PATCH 4/4] of/pci: Add of_pci_parse_bus_range() function

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

On Wed, Feb 13, 2013 at 10:58:44PM +0000, Grant Likely wrote:
> On Mon, 11 Feb 2013 09:22:20 +0100, Thierry Reding <thierry.reding@avionic-design.de> wrote:
> > This function can be used to parse a bus-range property as specified by
> > device nodes representing PCI bridges.
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> 
> Ditto for this one. We can wait on it until there is a user.

This is used by the Tegra driver and as I've explained the reason for
sending these patches separately was to get them merged beforehand to
reduce the number of dependencies that need to be tracked once the
driver is merged (possibly in 3.10, maybe later given the amount of
extra work you want done).

The patch used to be part of the Tegra series, but since Thomas started
using them for his Marvell work I thought it might be a good idea to
make them available separately.

But I can take it back into the Tegra series since that's where we seem
to be headed.

> > +/**
> > + * of_pci_parse_bus_range() - parse the bus-range property of a PCI device
> > + * @node: device node
> > + * @res: address to a struct resource to return the bus-range
> > + *
> > + * Returns 0 on success or a negative error-code on failure.
> > + */
> > +int of_pci_parse_bus_range(struct device_node *node, struct resource *res)
> > +{
> > +	const __be32 *values;
> > +	int len;
> > +
> > +	values = of_get_property(node, "bus-range", &len);
> > +	if (!values || len < sizeof(*values) * 2)
> > +		return -EINVAL;
> > +
> > +	res->name = node->name;
> > +	res->start = be32_to_cpup(values++);
> > +	res->end = be32_to_cpup(values);
> > +	res->flags = IORESOURCE_BUS;
> 
> Is there precedence for using struct resource for passing around the PCI
> bus range values? Who will be the user of this function?

The PCI core code actually keeps track of bus-ranges this way. See
drivers/pci/probe.c for instance which uses the global the busn_resource
variable and keeps a list of struct resource:s for each domain as well.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/4] Various PCI-related device tree helpers

[Thomas Petazzoni]

Hello,

On Mon, 11 Feb 2013 09:22:16 +0100, Thierry Reding wrote:
> Hi,
> 
> These patches add a few device tree helpers that are used are partially
> shared by Thomas' Marvell PCIe controller and my Tegra PCIe controller
> series. In an attempt to decrease the number of dependencies between
> trees, it would be nice to get these in for 3.9. There are a few ARM
> specific patches that the series depend on which have also been
> submitted and will hopefully make it into 3.9 so we can use the 3.9
> cycle to focus on getting the driver patches merged.
> 
> Thierry
> 
> Andrew Murray (1):
>   of/pci: Provide support for parsing PCI DT ranges property
> 
> Thierry Reding (3):
>   of/pci: Add of_pci_get_devfn() function
>   of/pci: Add of_pci_get_bus() function
>   of/pci: Add of_pci_parse_bus_range() function
> 
>  drivers/of/address.c       | 63 ++++++++++++++++++++++++++++++++++++
>  drivers/of/of_pci.c        | 80 +++++++++++++++++++++++++++++++++++++++++++---
>  include/linux/of_address.h |  9 ++++++
>  include/linux/of_pci.h     |  3 ++
>  4 files changed, 150 insertions(+), 5 deletions(-)

For the patches:

Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

As Thierry said, we are also using those patches for the Marvell PCIe
driver, so it would be really nice to have them in 3.9 so that we have a
bit less patches to worry about when submitting our PCIe driver for
3.10.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com