[01/21] of: Add cpu node iterator for_each_of_cpu_node()

Message ID 20180905193738.19325-2-robh@kernel.org
State New
Headers show
Series
  • [01/21] of: Add cpu node iterator for_each_of_cpu_node()
Related show

Commit Message

Rob Herring Sept. 5, 2018, 7:37 p.m.
Iterating thru cpu nodes is a common pattern. Create a common iterator
which can find child nodes either by node name or device_type == cpu.
Using the former will allow for eventually dropping device_type
properties which are deprecated for FDT.

Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>

---
 drivers/of/base.c  | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/of.h | 11 +++++++++++
 2 files changed, 50 insertions(+)

--
2.17.1

Comments

Geert Uytterhoeven Sept. 6, 2018, 8:45 a.m. | #1
On Wed, Sep 5, 2018 at 9:38 PM Rob Herring <robh@kernel.org> wrote:
> Iterating thru cpu nodes is a common pattern. Create a common iterator

> which can find child nodes either by node name or device_type == cpu.

> Using the former will allow for eventually dropping device_type

> properties which are deprecated for FDT.

>

> Cc: Frank Rowand <frowand.list@gmail.com>

> Signed-off-by: Rob Herring <robh@kernel.org>


Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Michael Ellerman Oct. 30, 2018, 2:18 p.m. | #2
Hi Rob,

Sorry I missed this when you posted it.

Rob Herring <robh@kernel.org> writes:
> Iterating thru cpu nodes is a common pattern. Create a common iterator

> which can find child nodes either by node name or device_type == cpu.

> Using the former will allow for eventually dropping device_type

> properties which are deprecated for FDT.


Device trees we see on powerpc generally don't (never?) use "cpu" as the
node name for CPU nodes. And many of those device trees come from
firmware, so we can't update them.

So dropping support for device_type is a non-starter from our POV.

cheers

> Cc: Frank Rowand <frowand.list@gmail.com>

> Signed-off-by: Rob Herring <robh@kernel.org>

> ---

>  drivers/of/base.c  | 39 +++++++++++++++++++++++++++++++++++++++

>  include/linux/of.h | 11 +++++++++++

>  2 files changed, 50 insertions(+)

>

> diff --git a/drivers/of/base.c b/drivers/of/base.c

> index a055cd1ef96d..4807db0a35b3 100644

> --- a/drivers/of/base.c

> +++ b/drivers/of/base.c

> @@ -741,6 +741,45 @@ struct device_node *of_get_next_available_child(const struct device_node *node,

>  }

>  EXPORT_SYMBOL(of_get_next_available_child);

>

> +/**

> + *	of_get_next_cpu_node - Iterate on cpu nodes

> + *	@prev:	previous child of the /cpus node, or NULL to get first

> + *

> + *	Returns a cpu node pointer with refcount incremented, use of_node_put()

> + *	on it when done. Returns NULL when prev is the last child. Decrements

> + *	the refcount of prev.

> + */

> +struct device_node *of_get_next_cpu_node(struct device_node *prev)

> +{

> +	struct device_node *next = NULL;

> +	unsigned long flags;

> +	struct device_node *node;

> +

> +	if (!prev)

> +		node = of_find_node_by_path("/cpus");

> +

> +	raw_spin_lock_irqsave(&devtree_lock, flags);

> +	if (prev)

> +		next = prev->sibling;

> +	else if (node) {

> +		next = node->child;

> +		of_node_put(node);

> +	}

> +	for (; next; next = next->sibling) {

> +		if (!(of_node_name_eq(next, "cpu") ||

> +		      (next->type && !of_node_cmp(next->type, "cpu"))))

> +			continue;

> +		if (!__of_device_is_available(next))

> +			continue;

> +		if (of_node_get(next))

> +			break;

> +	}

> +	of_node_put(prev);

> +	raw_spin_unlock_irqrestore(&devtree_lock, flags);

> +	return next;

> +}

> +EXPORT_SYMBOL(of_get_next_cpu_node);

> +

>  /**

>   * of_get_compatible_child - Find compatible child node

>   * @parent:	parent node

> diff --git a/include/linux/of.h b/include/linux/of.h

> index 99b0ebf49632..1aca0dbd35df 100644

> --- a/include/linux/of.h

> +++ b/include/linux/of.h

> @@ -353,6 +353,8 @@ extern const void *of_get_property(const struct device_node *node,

>  				const char *name,

>  				int *lenp);

>  extern struct device_node *of_get_cpu_node(int cpu, unsigned int *thread);

> +extern struct device_node *of_get_next_cpu_node(struct device_node *prev);

> +

>  #define for_each_property_of_node(dn, pp) \

>  	for (pp = dn->properties; pp != NULL; pp = pp->next)

>

> @@ -754,6 +756,11 @@ static inline struct device_node *of_get_cpu_node(int cpu,

>  	return NULL;

>  }

>

> +static inline struct device_node *of_get_next_cpu_node(struct device_node *prev)

> +{

> +	return NULL;

> +}

> +

>  static inline int of_n_addr_cells(struct device_node *np)

>  {

>  	return 0;

> @@ -1217,6 +1224,10 @@ static inline int of_property_read_s32(const struct device_node *np,

>  	for (child = of_get_next_available_child(parent, NULL); child != NULL; \

>  	     child = of_get_next_available_child(parent, child))

>

> +#define for_each_of_cpu_node(cpu) \

> +	for (cpu = of_get_next_cpu_node(NULL); cpu != NULL; \

> +	     cpu = of_get_next_cpu_node(cpu))

> +

>  #define for_each_node_with_property(dn, prop_name) \

>  	for (dn = of_find_node_with_property(NULL, prop_name); dn; \

>  	     dn = of_find_node_with_property(dn, prop_name))

> --

> 2.17.1
Michael Ellerman Oct. 30, 2018, 2:20 p.m. | #3
Michael Ellerman <mpe@ellerman.id.au> writes:
> Hi Rob,

>

> Sorry I missed this when you posted it.

>

> Rob Herring <robh@kernel.org> writes:

>> Iterating thru cpu nodes is a common pattern. Create a common iterator

>> which can find child nodes either by node name or device_type == cpu.

>> Using the former will allow for eventually dropping device_type

>> properties which are deprecated for FDT.

>

> Device trees we see on powerpc generally don't (never?) use "cpu" as the

> node name for CPU nodes. And many of those device trees come from

> firmware, so we can't update them.

>

> So dropping support for device_type is a non-starter from our POV.


ps. presumably that's what you meant by deprecated *for FDT*.

But anyway just wanted to make sure we are on the same page.

cheers
Rob Herring Oct. 30, 2018, 3 p.m. | #4
On Tue, Oct 30, 2018 at 9:20 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>

> Michael Ellerman <mpe@ellerman.id.au> writes:

> > Hi Rob,

> >

> > Sorry I missed this when you posted it.

> >

> > Rob Herring <robh@kernel.org> writes:

> >> Iterating thru cpu nodes is a common pattern. Create a common iterator

> >> which can find child nodes either by node name or device_type == cpu.

> >> Using the former will allow for eventually dropping device_type

> >> properties which are deprecated for FDT.

> >

> > Device trees we see on powerpc generally don't (never?) use "cpu" as the

> > node name for CPU nodes. And many of those device trees come from

> > firmware, so we can't update them.

> >

> > So dropping support for device_type is a non-starter from our POV.

>

> ps. presumably that's what you meant by deprecated *for FDT*.


Right.

> But anyway just wanted to make sure we are on the same page.


Yes, I was aware at least older powerpc DTs don't use 'cpu' for node names.

Rob
Michael Ellerman Nov. 1, 2018, 10:52 a.m. | #5
Rob Herring <robh@kernel.org> writes:
> On Tue, Oct 30, 2018 at 9:20 AM Michael Ellerman <mpe@ellerman.id.au> wrote:

>>

>> Michael Ellerman <mpe@ellerman.id.au> writes:

>> > Hi Rob,

>> >

>> > Sorry I missed this when you posted it.

>> >

>> > Rob Herring <robh@kernel.org> writes:

>> >> Iterating thru cpu nodes is a common pattern. Create a common iterator

>> >> which can find child nodes either by node name or device_type == cpu.

>> >> Using the former will allow for eventually dropping device_type

>> >> properties which are deprecated for FDT.

>> >

>> > Device trees we see on powerpc generally don't (never?) use "cpu" as the

>> > node name for CPU nodes. And many of those device trees come from

>> > firmware, so we can't update them.

>> >

>> > So dropping support for device_type is a non-starter from our POV.

>>

>> ps. presumably that's what you meant by deprecated *for FDT*.

>

> Right.

>

>> But anyway just wanted to make sure we are on the same page.

>

> Yes, I was aware at least older powerpc DTs don't use 'cpu' for node names.


Actually newer ones too, see below :)

And there's code out there that expects this, so we can't realistically
change it any time soon :/

  https://github.com/ibm-power-utilities/powerpc-utils/blob/master/src/drmgr/common_cpu.c#L186
  https://github.com/ibm-power-utilities/powerpc-utils/blob/master/src/ppc64_cpu.c#L344

cheers

$ ls -d1 /proc/device-tree/cpus/PowerPC\,POWER9@*
/proc/device-tree/cpus/PowerPC,POWER9@14
/proc/device-tree/cpus/PowerPC,POWER9@1c
/proc/device-tree/cpus/PowerPC,POWER9@34
/proc/device-tree/cpus/PowerPC,POWER9@3c
/proc/device-tree/cpus/PowerPC,POWER9@4
/proc/device-tree/cpus/PowerPC,POWER9@48
/proc/device-tree/cpus/PowerPC,POWER9@54
/proc/device-tree/cpus/PowerPC,POWER9@804
/proc/device-tree/cpus/PowerPC,POWER9@80c
/proc/device-tree/cpus/PowerPC,POWER9@814
/proc/device-tree/cpus/PowerPC,POWER9@81c
/proc/device-tree/cpus/PowerPC,POWER9@834
/proc/device-tree/cpus/PowerPC,POWER9@83c
/proc/device-tree/cpus/PowerPC,POWER9@844
/proc/device-tree/cpus/PowerPC,POWER9@84c
/proc/device-tree/cpus/PowerPC,POWER9@c
Segher Boessenkool Nov. 1, 2018, 3:12 p.m. | #6
On Thu, Nov 01, 2018 at 09:52:57PM +1100, Michael Ellerman wrote:
> Rob Herring <robh@kernel.org> writes:

> > Yes, I was aware at least older powerpc DTs don't use 'cpu' for node names.

> 

> Actually newer ones too, see below :)


Good, because that is required by the Open Firmware standard (the PowerPC
binding, to be exact):

http://www.openbios.org/data/docs/ppc-2_1.ps

(see 5.1.4, "name").


Segher

Patch

diff --git a/drivers/of/base.c b/drivers/of/base.c
index a055cd1ef96d..4807db0a35b3 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -741,6 +741,45 @@  struct device_node *of_get_next_available_child(const struct device_node *node,
 }
 EXPORT_SYMBOL(of_get_next_available_child);

+/**
+ *	of_get_next_cpu_node - Iterate on cpu nodes
+ *	@prev:	previous child of the /cpus node, or NULL to get first
+ *
+ *	Returns a cpu node pointer with refcount incremented, use of_node_put()
+ *	on it when done. Returns NULL when prev is the last child. Decrements
+ *	the refcount of prev.
+ */
+struct device_node *of_get_next_cpu_node(struct device_node *prev)
+{
+	struct device_node *next = NULL;
+	unsigned long flags;
+	struct device_node *node;
+
+	if (!prev)
+		node = of_find_node_by_path("/cpus");
+
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	if (prev)
+		next = prev->sibling;
+	else if (node) {
+		next = node->child;
+		of_node_put(node);
+	}
+	for (; next; next = next->sibling) {
+		if (!(of_node_name_eq(next, "cpu") ||
+		      (next->type && !of_node_cmp(next->type, "cpu"))))
+			continue;
+		if (!__of_device_is_available(next))
+			continue;
+		if (of_node_get(next))
+			break;
+	}
+	of_node_put(prev);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+	return next;
+}
+EXPORT_SYMBOL(of_get_next_cpu_node);
+
 /**
  * of_get_compatible_child - Find compatible child node
  * @parent:	parent node
diff --git a/include/linux/of.h b/include/linux/of.h
index 99b0ebf49632..1aca0dbd35df 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -353,6 +353,8 @@  extern const void *of_get_property(const struct device_node *node,
 				const char *name,
 				int *lenp);
 extern struct device_node *of_get_cpu_node(int cpu, unsigned int *thread);
+extern struct device_node *of_get_next_cpu_node(struct device_node *prev);
+
 #define for_each_property_of_node(dn, pp) \
 	for (pp = dn->properties; pp != NULL; pp = pp->next)

@@ -754,6 +756,11 @@  static inline struct device_node *of_get_cpu_node(int cpu,
 	return NULL;
 }

+static inline struct device_node *of_get_next_cpu_node(struct device_node *prev)
+{
+	return NULL;
+}
+
 static inline int of_n_addr_cells(struct device_node *np)
 {
 	return 0;
@@ -1217,6 +1224,10 @@  static inline int of_property_read_s32(const struct device_node *np,
 	for (child = of_get_next_available_child(parent, NULL); child != NULL; \
 	     child = of_get_next_available_child(parent, child))

+#define for_each_of_cpu_node(cpu) \
+	for (cpu = of_get_next_cpu_node(NULL); cpu != NULL; \
+	     cpu = of_get_next_cpu_node(cpu))
+
 #define for_each_node_with_property(dn, prop_name) \
 	for (dn = of_find_node_with_property(NULL, prop_name); dn; \
 	     dn = of_find_node_with_property(dn, prop_name))