Re: [PATCH 01/14] of/pci: Provide support for parsing PCI DT ranges property
On Thu, Jan 10, 2013 at 05:06:48PM -0700, Stephen Warren wrote:
> On 01/09/2013 01:43 PM, Thierry Reding wrote:
> > From: Andrew Murray
> >
> > 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.
> ...
>
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
>
> > +const __be32 *of_pci_process_ranges(struct device_node *node,
>
> > + 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;
>
> Hmmm. That seems to just ignore bad entries in the ranges property. Is
> that really the right thing to do? At least printing a diagnostic might
> be a good idea, even if the code does just soldier on in the hope
> everything still works.
That's true. However, erroring out here wouln't be useful either since a
NULL return value is used to mark the end of the iteration and the
caller would have to assume that no more ranges are present. Maybe
that's better than continuing anyway, even if a message is printed.
Alternatively, the of_pci_process_ranges() could be changed to return an
ERR_PTR() encoded errno. This is one case where it makes a lot of sense.
I have a feeling that Grant won't like that very much, though.
Another possibility would be to change away from an iterator-based
approach and return an integer for the number of ranges and negative
error code on failure while returning an allocated array of resources
through an output parameter.
Thierry
pgp8xVxoYUvnT.pgp
Description: PGP signature
Re: [PATCH 01/14] of/pci: Provide support for parsing PCI DT ranges property
On 01/09/2013 01:43 PM, Thierry Reding wrote:
> From: Andrew Murray
>
> 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.
...
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> +const __be32 *of_pci_process_ranges(struct device_node *node,
> + 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;
Hmmm. That seems to just ignore bad entries in the ranges property. Is
that really the right thing to do? At least printing a diagnostic might
be a good idea, even if the code does just soldier on in the hope
everything still works.
> + 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;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/14] of/pci: Provide support for parsing PCI DT ranges property
On 01/09/2013 01:43 PM, Thierry Reding wrote:
From: Andrew Murray andrew.mur...@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.
...
diff --git a/drivers/of/address.c b/drivers/of/address.c
+const __be32 *of_pci_process_ranges(struct device_node *node,
+ 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;
Hmmm. That seems to just ignore bad entries in the ranges property. Is
that really the right thing to do? At least printing a diagnostic might
be a good idea, even if the code does just soldier on in the hope
everything still works.
+ 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;
+}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/14] of/pci: Provide support for parsing PCI DT ranges property
On Thu, Jan 10, 2013 at 05:06:48PM -0700, Stephen Warren wrote:
On 01/09/2013 01:43 PM, Thierry Reding wrote:
From: Andrew Murray andrew.mur...@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.
...
diff --git a/drivers/of/address.c b/drivers/of/address.c
+const __be32 *of_pci_process_ranges(struct device_node *node,
+ 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;
Hmmm. That seems to just ignore bad entries in the ranges property. Is
that really the right thing to do? At least printing a diagnostic might
be a good idea, even if the code does just soldier on in the hope
everything still works.
That's true. However, erroring out here wouln't be useful either since a
NULL return value is used to mark the end of the iteration and the
caller would have to assume that no more ranges are present. Maybe
that's better than continuing anyway, even if a message is printed.
Alternatively, the of_pci_process_ranges() could be changed to return an
ERR_PTR() encoded errno. This is one case where it makes a lot of sense.
I have a feeling that Grant won't like that very much, though.
Another possibility would be to change away from an iterator-based
approach and return an integer for the number of ranges and negative
error code on failure while returning an allocated array of resources
through an output parameter.
Thierry
pgp8xVxoYUvnT.pgp
Description: PGP signature
[PATCH 01/14] of/pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray
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
Signed-off-by: Liviu Dudau
Signed-off-by: Thierry Reding
---
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 0125524..d659527 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, , );
+ 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", );
+ 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 _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 @@
[PATCH 01/14] of/pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray andrew.mur...@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.mur...@arm.com
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Signed-off-by: Thierry Reding thierry.red...@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 0125524..d659527 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
