Patchwork [v3,09/10] of: Add of_pci_parse_ranges()

login
register
mail settings
Submitter Thierry Reding
Date July 26, 2012, 7:55 p.m.
Message ID <1343332512-28762-10-git-send-email-thierry.reding@avionic-design.de>
Download mbox | patch
Permalink /patch/173481/
State Superseded, archived
Headers show

Comments

Thierry Reding - July 26, 2012, 7:55 p.m.
This new function parses the ranges property of PCI device nodes into
an array of struct resource elements. It is useful in multiple-port PCI
host controller drivers to collect information about the ranges that it
needs to forward to the respective ports.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
Changes in v3:
- new patch

 drivers/of/of_pci.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/of_pci.h |  2 ++
 2 files changed, 85 insertions(+), 1 deletion(-)
Rob Herring - July 31, 2012, 8:07 p.m.
On 07/26/2012 02:55 PM, Thierry Reding wrote:
> This new function parses the ranges property of PCI device nodes into
> an array of struct resource elements. It is useful in multiple-port PCI
> host controller drivers to collect information about the ranges that it
> needs to forward to the respective ports.

It seems to me that some of the DT PCI code in arch/powerpc/kernel/pci*
like pci_process_bridge_OF_ranges() should apply for ARM as well.

Each arch defining their own pci controller structs complicates this,
but I would think at least the DT parsing can be common.

> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
> Changes in v3:
> - new patch
> 
>  drivers/of/of_pci.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/of_pci.h |  2 ++
>  2 files changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 13e37e2..bcff301 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -1,7 +1,8 @@
>  #include <linux/kernel.h>
>  #include <linux/export.h>
> -#include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/of_pci.h>
> +#include <linux/slab.h>
>  #include <asm/prom.h>
>  
>  static inline int __of_pci_pci_compare(struct device_node *node,
> @@ -40,3 +41,84 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
> +
> +struct resource *of_pci_parse_ranges(struct device_node *node,
> +				     unsigned int *countp)
> +{
> +	unsigned int count, i = 0;
> +	struct resource *ranges;

I think res or pci_res would be clearer than ranges that this is a
struct resource.

> +	const __be32 *values;
> +	int len, pna, np;
> +
> +	values = of_get_property(node, "ranges", &len);
> +	if (!values)
> +		return NULL;
> +
> +	pna = of_n_addr_cells(node);
> +	np = pna + 5;
> +
> +	count = len / (np * sizeof(*values));
> +
> +	ranges = kzalloc(sizeof(*ranges) * count, GFP_KERNEL);
> +	if (!ranges)
> +		return NULL;
> +
> +	pr_debug("PCI ranges:\n");
> +
> +	while ((len -= np * sizeof(*values)) >= 0) {
> +		u64 addr = of_translate_address(node, values + 3);
> +		u64 size = of_read_number(values + 3 + pna, 2);
> +		u32 type = be32_to_cpup(values);
> +		const char *suffix = "";
> +		struct resource range;

Same here.

Rob

> +
> +		memset(&range, 0, sizeof(range));
> +		range.start = addr;
> +		range.end = addr + size - 1;
> +
> +		switch ((type >> 24) & 0x3) {
> +		case 0:
> +			range.flags = IORESOURCE_MEM | IORESOURCE_PCI_CS;
> +			pr_debug("  CS  %#x-%#x\n", range.start, range.end);
> +			break;
> +
> +		case 1:
> +			range.flags = IORESOURCE_IO;
> +			pr_debug("  IO  %#x-%#x\n", range.start, range.end);
> +			break;
> +
> +		case 2:
> +			range.flags = IORESOURCE_MEM;
> +
> +			if (type & 0x40000000) {
> +				range.flags |= IORESOURCE_PREFETCH;
> +				suffix = "prefetch";
> +			}
> +
> +			pr_debug("  MEM %#x-%#x %s\n", range.start, range.end,
> +				 suffix);
> +			break;
> +
> +		case 3:
> +			range.flags = IORESOURCE_MEM | IORESOURCE_MEM_64;
> +
> +			if (type & 0x40000000) {
> +				range.flags |= IORESOURCE_PREFETCH;
> +				suffix = "prefetch";
> +			}
> +
> +			pr_debug("  MEM %#x-%#x 64-bit %s\n", range.start,
> +				 range.end, suffix);
> +			break;
> +		}
> +
> +		memcpy(&ranges[i++], &range, sizeof(range));
> +		values += np;
> +	}
> +
> +	if (countp)
> +		*countp = count;
> +
> +	return ranges;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_parse_ranges);
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index bb115de..c0db6ea 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -10,5 +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);
> +struct resource *of_pci_parse_ranges(struct device_node *node,
> +				     unsigned int *countp);
>  
>  #endif
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - Aug. 1, 2012, 6:54 a.m.
On Tue, Jul 31, 2012 at 03:07:31PM -0500, Rob Herring wrote:
> On 07/26/2012 02:55 PM, Thierry Reding wrote:
> > This new function parses the ranges property of PCI device nodes into
> > an array of struct resource elements. It is useful in multiple-port PCI
> > host controller drivers to collect information about the ranges that it
> > needs to forward to the respective ports.
> 
> It seems to me that some of the DT PCI code in arch/powerpc/kernel/pci*
> like pci_process_bridge_OF_ranges() should apply for ARM as well.
> 
> Each arch defining their own pci controller structs complicates this,
> but I would think at least the DT parsing can be common.

Yes, there's quite a lot of room for refactoring. When I first started
work on this there had been some discussion about whether it would make
sense to move PCI controller drivers into a common location to make it
easier to refactor but the consensus at the time was that this should
not be done.

I still think this might be worthwhile, but I have other things that I
need to finish first.

> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> > Changes in v3:
> > - new patch
> > 
> >  drivers/of/of_pci.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  include/linux/of_pci.h |  2 ++
> >  2 files changed, 85 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> > index 13e37e2..bcff301 100644
> > --- a/drivers/of/of_pci.c
> > +++ b/drivers/of/of_pci.c
> > @@ -1,7 +1,8 @@
> >  #include <linux/kernel.h>
> >  #include <linux/export.h>
> > -#include <linux/of.h>
> > +#include <linux/of_address.h>
> >  #include <linux/of_pci.h>
> > +#include <linux/slab.h>
> >  #include <asm/prom.h>
> >  
> >  static inline int __of_pci_pci_compare(struct device_node *node,
> > @@ -40,3 +41,84 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
> >  	return NULL;
> >  }
> >  EXPORT_SYMBOL_GPL(of_pci_find_child_device);
> > +
> > +struct resource *of_pci_parse_ranges(struct device_node *node,
> > +				     unsigned int *countp)
> > +{
> > +	unsigned int count, i = 0;
> > +	struct resource *ranges;
> 
> I think res or pci_res would be clearer than ranges that this is a
> struct resource.

"range" is the term used by the PCI specifications to denote these
regions. I don't see what's wrong with using it as a variable name.

Thierry
Stephen Warren - Aug. 1, 2012, 4:07 p.m.
On 08/01/2012 12:54 AM, Thierry Reding wrote:
> On Tue, Jul 31, 2012 at 03:07:31PM -0500, Rob Herring wrote:
>> On 07/26/2012 02:55 PM, Thierry Reding wrote:
>>> This new function parses the ranges property of PCI device
>>> nodes into an array of struct resource elements. It is useful
>>> in multiple-port PCI host controller drivers to collect
>>> information about the ranges that it needs to forward to the
>>> respective ports.
>> 
>> It seems to me that some of the DT PCI code in
>> arch/powerpc/kernel/pci* like pci_process_bridge_OF_ranges()
>> should apply for ARM as well.
>> 
>> Each arch defining their own pci controller structs complicates
>> this, but I would think at least the DT parsing can be common.
> 
> Yes, there's quite a lot of room for refactoring. When I first
> started work on this there had been some discussion about whether
> it would make sense to move PCI controller drivers into a common
> location to make it easier to refactor but the consensus at the
> time was that this should not be done.

In the long term, if we end up with a separate arch/aarch64/, I think
we'll have to move the PCIe (and any other) drivers to a common
location. Still, I imagine there is plenty of time until that's mandatory.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 13e37e2..bcff301 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -1,7 +1,8 @@ 
 #include <linux/kernel.h>
 #include <linux/export.h>
-#include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/slab.h>
 #include <asm/prom.h>
 
 static inline int __of_pci_pci_compare(struct device_node *node,
@@ -40,3 +41,84 @@  struct device_node *of_pci_find_child_device(struct device_node *parent,
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(of_pci_find_child_device);
+
+struct resource *of_pci_parse_ranges(struct device_node *node,
+				     unsigned int *countp)
+{
+	unsigned int count, i = 0;
+	struct resource *ranges;
+	const __be32 *values;
+	int len, pna, np;
+
+	values = of_get_property(node, "ranges", &len);
+	if (!values)
+		return NULL;
+
+	pna = of_n_addr_cells(node);
+	np = pna + 5;
+
+	count = len / (np * sizeof(*values));
+
+	ranges = kzalloc(sizeof(*ranges) * count, GFP_KERNEL);
+	if (!ranges)
+		return NULL;
+
+	pr_debug("PCI ranges:\n");
+
+	while ((len -= np * sizeof(*values)) >= 0) {
+		u64 addr = of_translate_address(node, values + 3);
+		u64 size = of_read_number(values + 3 + pna, 2);
+		u32 type = be32_to_cpup(values);
+		const char *suffix = "";
+		struct resource range;
+
+		memset(&range, 0, sizeof(range));
+		range.start = addr;
+		range.end = addr + size - 1;
+
+		switch ((type >> 24) & 0x3) {
+		case 0:
+			range.flags = IORESOURCE_MEM | IORESOURCE_PCI_CS;
+			pr_debug("  CS  %#x-%#x\n", range.start, range.end);
+			break;
+
+		case 1:
+			range.flags = IORESOURCE_IO;
+			pr_debug("  IO  %#x-%#x\n", range.start, range.end);
+			break;
+
+		case 2:
+			range.flags = IORESOURCE_MEM;
+
+			if (type & 0x40000000) {
+				range.flags |= IORESOURCE_PREFETCH;
+				suffix = "prefetch";
+			}
+
+			pr_debug("  MEM %#x-%#x %s\n", range.start, range.end,
+				 suffix);
+			break;
+
+		case 3:
+			range.flags = IORESOURCE_MEM | IORESOURCE_MEM_64;
+
+			if (type & 0x40000000) {
+				range.flags |= IORESOURCE_PREFETCH;
+				suffix = "prefetch";
+			}
+
+			pr_debug("  MEM %#x-%#x 64-bit %s\n", range.start,
+				 range.end, suffix);
+			break;
+		}
+
+		memcpy(&ranges[i++], &range, sizeof(range));
+		values += np;
+	}
+
+	if (countp)
+		*countp = count;
+
+	return ranges;
+}
+EXPORT_SYMBOL_GPL(of_pci_parse_ranges);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bb115de..c0db6ea 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -10,5 +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);
+struct resource *of_pci_parse_ranges(struct device_node *node,
+				     unsigned int *countp);
 
 #endif