linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Refactoring finding CPU phandles in DT
@ 2025-07-08 15:14 Alireza Sanaee
  2025-07-08 15:14 ` [PATCH v2 1/5] of: add infra for finding CPU id from phandle Alireza Sanaee
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Alireza Sanaee @ 2025-07-08 15:14 UTC (permalink / raw)
  To: krzk, robh
  Cc: coresight, devicetree, dianders, james.clark, jonathan.cameron,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

This series refactors the way CPU IDs are retrieved from the device
tree.

Usually, there is a for loop that goes over every single CPU that can be
avoided. This also reduces the amount of NULL pointer checks in drivers.
I have abstracted away that loop and introduced a new function
(of_cpu_node_to_id) for this.

This patchset is a subset of [1], where I removed content and patches
relevant to hyper-threaded cores for DT. Based on the discussion, the
code refactor is still useful, hence this patchset.

Changes since v1 [2]:
    - Rebased on top of the latest mainline.
    - Addressed Krzysztof Kozlowski's comments -- Hopefully :-)
    - Addressed Jonathan Cameron's comments.

[1] https://lore.kernel.org/all/20250512080715.82-1-alireza.sanaee@huawei.com/
[2] https://lore.kernel.org/all/20250707150414.620-1-alireza.sanaee@huawei.com/

Alireza Sanaee (5):
  of: add infra for finding CPU id from phandle
  arch_topology: update CPU map to use the new API
  coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id
  coresight: Use of_cpu_phandle_to_id for grabbing CPU id
  perf/arm-dsu: refactor cpu id retrieval via new API
    of_cpu_phandle_to_id

 drivers/base/arch_topology.c                  | 16 ++++----
 .../coresight/coresight-cti-platform.c        | 14 +------
 .../hwtracing/coresight/coresight-platform.c  | 15 +------
 drivers/of/cpu.c                              | 40 +++++++++++++++++++
 drivers/perf/arm_dsu_pmu.c                    |  8 +---
 include/linux/of.h                            |  9 +++++
 6 files changed, 61 insertions(+), 41 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/5] of: add infra for finding CPU id from phandle
  2025-07-08 15:14 [PATCH v2 0/5] Refactoring finding CPU phandles in DT Alireza Sanaee
@ 2025-07-08 15:14 ` Alireza Sanaee
  2025-07-11  9:17   ` Jonathan Cameron
  2025-07-11  9:35   ` Jonathan Cameron
  2025-07-08 15:14 ` [PATCH v2 2/5] arch_topology: update CPU map to use the new API Alireza Sanaee
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Alireza Sanaee @ 2025-07-08 15:14 UTC (permalink / raw)
  To: krzk, robh
  Cc: coresight, devicetree, dianders, james.clark, jonathan.cameron,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

Get CPU id from phandle. Many drivers get do this by getting hold of CPU
node first through a phandle and then find the CPU ID using the relevant
function. This commit encapsulates cpu node finding and improves
readability.

The API interface requires two parameters, 1) node, 2) pointer to
pointer of CPU node, 3) cpu node index. API sets the pointer to the CPU
node and allows the driver to play with the CPU itself, for logging
purposes for instance.

Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 drivers/of/cpu.c   | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |  9 +++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/of/cpu.c b/drivers/of/cpu.c
index 5214dc3d05ae..494d47470f94 100644
--- a/drivers/of/cpu.c
+++ b/drivers/of/cpu.c
@@ -173,6 +173,46 @@ int of_cpu_node_to_id(struct device_node *cpu_node)
 }
 EXPORT_SYMBOL(of_cpu_node_to_id);
 
+/**
+ * of_cpu_phandle_to_id: Get the logical CPU number for a given device_node
+ *
+ * @node: Pointer to the device_node containing CPU phandle.
+ * @cpu_np: Pointer to the device_node for CPU.
+ * @cpu_idx: The index of the CPU in the list of CPUs.
+ *
+ * Return: The logical CPU number of the given CPU device_node or -ENODEV if
+ * the CPU is not found, or if the node is NULL, it returns -1. On success,
+ * cpu_np will always point to the retrieved CPU device_node with refcount
+ * incremented, use of_node_put() on it when done.
+ */
+int of_cpu_phandle_to_id(const struct device_node *node,
+			 struct device_node **cpu_np,
+			 uint8_t cpu_idx)
+{
+	struct device_node *local_cpu_node;
+	int cpu;
+
+	if (!node)
+		return -1;
+
+	local_cpu_node = of_parse_phandle(node, "cpu", 0);
+	if (!local_cpu_node)
+		local_cpu_node = of_parse_phandle(node, "cpus", cpu_idx);
+
+	if (!local_cpu_node)
+		return -ENODEV;
+
+	cpu = of_cpu_node_to_id(local_cpu_node);
+
+	if (cpu_np)
+		*cpu_np = local_cpu_node;
+	else
+		of_node_put(local_cpu_node);
+
+	return cpu;
+}
+EXPORT_SYMBOL(of_cpu_phandle_to_id);
+
 /**
  * of_get_cpu_state_node - Get CPU's idle state node at the given index
  *
diff --git a/include/linux/of.h b/include/linux/of.h
index a62154aeda1b..717e55065d99 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -365,6 +365,8 @@ extern const void *of_get_property(const struct device_node *node,
 extern struct device_node *of_get_cpu_node(int cpu, unsigned int *thread);
 extern struct device_node *of_cpu_device_node_get(int cpu);
 extern int of_cpu_node_to_id(struct device_node *np);
+extern int of_cpu_phandle_to_id(const struct device_node *np,
+				struct device_node **cpu_np, uint8_t cpu_idx);
 extern struct device_node *of_get_next_cpu_node(struct device_node *prev);
 extern struct device_node *of_get_cpu_state_node(const struct device_node *cpu_node,
 						 int index);
@@ -680,6 +682,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
 	return -ENODEV;
 }
 
+static inline int of_cpu_phandle_to_id(const struct device_node *np,
+				       struct device_node **cpu_np,
+				       uint8_t cpu_idx)
+{
+	return -ENODEV;
+}
+
 static inline struct device_node *of_get_next_cpu_node(struct device_node *prev)
 {
 	return NULL;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/5] arch_topology: update CPU map to use the new API
  2025-07-08 15:14 [PATCH v2 0/5] Refactoring finding CPU phandles in DT Alireza Sanaee
  2025-07-08 15:14 ` [PATCH v2 1/5] of: add infra for finding CPU id from phandle Alireza Sanaee
@ 2025-07-08 15:14 ` Alireza Sanaee
  2025-07-11  9:25   ` Jonathan Cameron
  2025-07-08 15:15 ` [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id Alireza Sanaee
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Alireza Sanaee @ 2025-07-08 15:14 UTC (permalink / raw)
  To: krzk, robh
  Cc: coresight, devicetree, dianders, james.clark, jonathan.cameron,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

Cleans up the cpu-map generation using the created API.

Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 drivers/base/arch_topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 1037169abb45..ccc73bfae90d 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -473,19 +473,19 @@ static unsigned int max_smt_thread_num = 1;
  */
 static int __init get_cpu_for_node(struct device_node *node)
 {
+	struct device_node *cpu_node;
 	int cpu;
-	struct device_node *cpu_node __free(device_node) =
-		of_parse_phandle(node, "cpu", 0);
 
-	if (!cpu_node)
-		return -1;
-
-	cpu = of_cpu_node_to_id(cpu_node);
-	if (cpu >= 0)
+	cpu = of_cpu_phandle_to_id(node, &cpu_node, 0);
+	if (cpu >= 0) {
 		topology_parse_cpu_capacity(cpu_node, cpu);
-	else
+		of_node_put(cpu_node);
+	} else if (cpu == -ENODEV) {
 		pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
 			cpu_node, cpumask_pr_args(cpu_possible_mask));
+		of_node_put(cpu_node);
+	} else
+		return -1;
 
 	return cpu;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id
  2025-07-08 15:14 [PATCH v2 0/5] Refactoring finding CPU phandles in DT Alireza Sanaee
  2025-07-08 15:14 ` [PATCH v2 1/5] of: add infra for finding CPU id from phandle Alireza Sanaee
  2025-07-08 15:14 ` [PATCH v2 2/5] arch_topology: update CPU map to use the new API Alireza Sanaee
@ 2025-07-08 15:15 ` Alireza Sanaee
  2025-07-11  9:28   ` Jonathan Cameron
  2025-07-11 15:55   ` Mike Leach
  2025-07-08 15:15 ` [PATCH v2 4/5] coresight: " Alireza Sanaee
  2025-07-08 15:15 ` [PATCH v2 5/5] perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id Alireza Sanaee
  4 siblings, 2 replies; 14+ messages in thread
From: Alireza Sanaee @ 2025-07-08 15:15 UTC (permalink / raw)
  To: krzk, robh
  Cc: coresight, devicetree, dianders, james.clark, jonathan.cameron,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

Use the newly created API to grab CPU id.

Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 .../hwtracing/coresight/coresight-cti-platform.c   | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
index d0ae10bf6128..e1dc559d54aa 100644
--- a/drivers/hwtracing/coresight/coresight-cti-platform.c
+++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
@@ -41,20 +41,8 @@
  */
 static int of_cti_get_cpu_at_node(const struct device_node *node)
 {
-	int cpu;
-	struct device_node *dn;
+	int cpu = of_cpu_phandle_to_id(node, NULL, 0);
 
-	if (node == NULL)
-		return -1;
-
-	dn = of_parse_phandle(node, "cpu", 0);
-	/* CTI affinity defaults to no cpu */
-	if (!dn)
-		return -1;
-	cpu = of_cpu_node_to_id(dn);
-	of_node_put(dn);
-
-	/* No Affinity  if no cpu nodes are found */
 	return (cpu < 0) ? -1 : cpu;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/5] coresight: Use of_cpu_phandle_to_id for grabbing CPU id
  2025-07-08 15:14 [PATCH v2 0/5] Refactoring finding CPU phandles in DT Alireza Sanaee
                   ` (2 preceding siblings ...)
  2025-07-08 15:15 ` [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id Alireza Sanaee
@ 2025-07-08 15:15 ` Alireza Sanaee
  2025-07-11  9:30   ` Jonathan Cameron
  2025-07-08 15:15 ` [PATCH v2 5/5] perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id Alireza Sanaee
  4 siblings, 1 reply; 14+ messages in thread
From: Alireza Sanaee @ 2025-07-08 15:15 UTC (permalink / raw)
  To: krzk, robh
  Cc: coresight, devicetree, dianders, james.clark, jonathan.cameron,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

Use the newly created API to grab CPU id.

Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 drivers/hwtracing/coresight/coresight-platform.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 0db64c5f4995..95d46ea08936 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -167,20 +167,7 @@ of_coresight_get_output_ports_node(const struct device_node *node)
 
 static int of_coresight_get_cpu(struct device *dev)
 {
-	int cpu;
-	struct device_node *dn;
-
-	if (!dev->of_node)
-		return -ENODEV;
-
-	dn = of_parse_phandle(dev->of_node, "cpu", 0);
-	if (!dn)
-		return -ENODEV;
-
-	cpu = of_cpu_node_to_id(dn);
-	of_node_put(dn);
-
-	return cpu;
+	return of_cpu_phandle_to_id(dev->of_node, NULL, 0);
 }
 
 /*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/5] perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id
  2025-07-08 15:14 [PATCH v2 0/5] Refactoring finding CPU phandles in DT Alireza Sanaee
                   ` (3 preceding siblings ...)
  2025-07-08 15:15 ` [PATCH v2 4/5] coresight: " Alireza Sanaee
@ 2025-07-08 15:15 ` Alireza Sanaee
  2025-07-11  9:36   ` Jonathan Cameron
  4 siblings, 1 reply; 14+ messages in thread
From: Alireza Sanaee @ 2025-07-08 15:15 UTC (permalink / raw)
  To: krzk, robh
  Cc: coresight, devicetree, dianders, james.clark, jonathan.cameron,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

Update arm-dsu to use the new API, where both "cpus" and "cpu"
properties are supported.

Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 drivers/perf/arm_dsu_pmu.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
index cb4fb59fe04b..1014b92c0fd2 100644
--- a/drivers/perf/arm_dsu_pmu.c
+++ b/drivers/perf/arm_dsu_pmu.c
@@ -591,17 +591,13 @@ static struct dsu_pmu *dsu_pmu_alloc(struct platform_device *pdev)
 static int dsu_pmu_dt_get_cpus(struct device *dev, cpumask_t *mask)
 {
 	int i = 0, n, cpu;
-	struct device_node *cpu_node;
 
 	n = of_count_phandle_with_args(dev->of_node, "cpus", NULL);
 	if (n <= 0)
 		return -ENODEV;
+
 	for (; i < n; i++) {
-		cpu_node = of_parse_phandle(dev->of_node, "cpus", i);
-		if (!cpu_node)
-			break;
-		cpu = of_cpu_node_to_id(cpu_node);
-		of_node_put(cpu_node);
+		cpu = of_cpu_phandle_to_id(dev->of_node, NULL, i);
 		/*
 		 * We have to ignore the failures here and continue scanning
 		 * the list to handle cases where the nr_cpus could be capped
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/5] of: add infra for finding CPU id from phandle
  2025-07-08 15:14 ` [PATCH v2 1/5] of: add infra for finding CPU id from phandle Alireza Sanaee
@ 2025-07-11  9:17   ` Jonathan Cameron
  2025-07-11  9:35   ` Jonathan Cameron
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11  9:17 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

On Tue, 8 Jul 2025 16:14:58 +0100
Alireza Sanaee <alireza.sanaee@huawei.com> wrote:


Hi Ali,

Code looks good, just some comments on patch description and one slightly
odd return value.

> Get CPU id from phandle. Many drivers get do this by getting hold of CPU

Avoid term like "many" as something people can argue about. Also where you
can be more specific in patch description.

Drivers do this by get the CPU device_node through a phandle and then find
the CPU ID using using of_cpu_node_to_id().

> node first through a phandle and then find the CPU ID using the relevant
> function. This commit encapsulates cpu node finding and improves
> readability.
> 
> The API interface requires two parameters, 1) node, 2) pointer to
> pointer of CPU node, 3) cpu node index. API sets the pointer to the CPU

Two parameters, then list 3?  Only 2 of which are 'required'.
Talk about that optionality briefly here.

> node and allows the driver to play with the CPU itself, for logging

Playing with the cpu node, not the CPU. :)

> purposes for instance.
> 
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> ---
>  drivers/of/cpu.c   | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  9 +++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/of/cpu.c b/drivers/of/cpu.c
> index 5214dc3d05ae..494d47470f94 100644
> --- a/drivers/of/cpu.c
> +++ b/drivers/of/cpu.c
> @@ -173,6 +173,46 @@ int of_cpu_node_to_id(struct device_node *cpu_node)
>  }
>  EXPORT_SYMBOL(of_cpu_node_to_id);
>  
> +/**
> + * of_cpu_phandle_to_id: Get the logical CPU number for a given device_node
> + *
> + * @node: Pointer to the device_node containing CPU phandle.
> + * @cpu_np: Pointer to the device_node for CPU.
> + * @cpu_idx: The index of the CPU in the list of CPUs.
> + *
> + * Return: The logical CPU number of the given CPU device_node or -ENODEV if
> + * the CPU is not found, or if the node is NULL, it returns -1. On success,

Why the -1?  I guess inherited from one of the usecases. If we can flip
to -EINVAL to indicate an invalid parameter it might be more conventional?

> + * cpu_np will always point to the retrieved CPU device_node with refcount
> + * incremented, use of_node_put() on it when done.

Need to update that *cpu_np is only set if cpu_np is not NULL and make
this refcount bit dependent on that in the description.

> + */
> +int of_cpu_phandle_to_id(const struct device_node *node,
> +			 struct device_node **cpu_np,
> +			 uint8_t cpu_idx)
> +{
> +	struct device_node *local_cpu_node;
> +	int cpu;
> +
> +	if (!node)
> +		return -1;
> +
> +	local_cpu_node = of_parse_phandle(node, "cpu", 0);
> +	if (!local_cpu_node)
> +		local_cpu_node = of_parse_phandle(node, "cpus", cpu_idx);
> +
> +	if (!local_cpu_node)
> +		return -ENODEV;
> +
> +	cpu = of_cpu_node_to_id(local_cpu_node);
> +
> +	if (cpu_np)
> +		*cpu_np = local_cpu_node;
> +	else
> +		of_node_put(local_cpu_node);
> +
> +	return cpu;
> +}
> +EXPORT_SYMBOL(of_cpu_phandle_to_id);
> +
>  /**
>   * of_get_cpu_state_node - Get CPU's idle state node at the given index
>   *
> diff --git a/include/linux/of.h b/include/linux/of.h
> index a62154aeda1b..717e55065d99 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -365,6 +365,8 @@ extern const void *of_get_property(const struct device_node *node,
>  extern struct device_node *of_get_cpu_node(int cpu, unsigned int *thread);
>  extern struct device_node *of_cpu_device_node_get(int cpu);
>  extern int of_cpu_node_to_id(struct device_node *np);
> +extern int of_cpu_phandle_to_id(const struct device_node *np,
> +				struct device_node **cpu_np, uint8_t cpu_idx);
>  extern struct device_node *of_get_next_cpu_node(struct device_node *prev);
>  extern struct device_node *of_get_cpu_state_node(const struct device_node *cpu_node,
>  						 int index);
> @@ -680,6 +682,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
>  	return -ENODEV;
>  }
>  
> +static inline int of_cpu_phandle_to_id(const struct device_node *np,
> +				       struct device_node **cpu_np,
> +				       uint8_t cpu_idx)
> +{
> +	return -ENODEV;
> +}
> +
>  static inline struct device_node *of_get_next_cpu_node(struct device_node *prev)
>  {
>  	return NULL;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/5] arch_topology: update CPU map to use the new API
  2025-07-08 15:14 ` [PATCH v2 2/5] arch_topology: update CPU map to use the new API Alireza Sanaee
@ 2025-07-11  9:25   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11  9:25 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

On Tue, 8 Jul 2025 16:14:59 +0100
Alireza Sanaee <alireza.sanaee@huawei.com> wrote:

> Cleans up the cpu-map generation using the created API.
> 
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> ---
>  drivers/base/arch_topology.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 1037169abb45..ccc73bfae90d 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -473,19 +473,19 @@ static unsigned int max_smt_thread_num = 1;
>   */
>  static int __init get_cpu_for_node(struct device_node *node)
>  {
> +	struct device_node *cpu_node;

Initialize to NULL.  Whilst we can see that it will always be initalized
static analysis tools and compilers probably can't.

>  	int cpu;
> -	struct device_node *cpu_node __free(device_node) =
> -		of_parse_phandle(node, "cpu", 0);
>  
> -	if (!cpu_node)
> -		return -1;
> -
> -	cpu = of_cpu_node_to_id(cpu_node);
> -	if (cpu >= 0)
> +	cpu = of_cpu_phandle_to_id(node, &cpu_node, 0);
> +	if (cpu >= 0) {
>  		topology_parse_cpu_capacity(cpu_node, cpu);
> -	else
> +		of_node_put(cpu_node);
> +	} else if (cpu == -ENODEV) {
>  		pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
>  			cpu_node, cpumask_pr_args(cpu_possible_mask));
> +		of_node_put(cpu_node);
> +	} else
> +		return -1;

I'd be tempted to make this more conventional with error path out of line.

	cpu = of_cpu_phandle_to_id(node, &cpu_node, 0);
	if (cpu == -ENODEV) {
  		pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
  			cpu_node, cpumask_pr_args(cpu_possible_mask));
		return -ENODEV;
	} else if (cpu < 0) {
`		return -1;  /* to keep ABI - it's odd but not something we should touch! */
// note this avoids need for the helper to return -1, it can return -EINVAL to indicate
// an invalid parameter.
	}

	topology_parse_cpu_capacity(cpu_node, cpu);
	of_node_put(cpu_node);

	return cpu;
	

>  
>  	return cpu;
>  }


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id
  2025-07-08 15:15 ` [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id Alireza Sanaee
@ 2025-07-11  9:28   ` Jonathan Cameron
  2025-07-11 15:55   ` Mike Leach
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11  9:28 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi, Suzuki K Poulose

On Tue, 8 Jul 2025 16:15:00 +0100
Alireza Sanaee <alireza.sanaee@huawei.com> wrote:

> Use the newly created API to grab CPU id.
> 
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> ---
>  .../hwtracing/coresight/coresight-cti-platform.c   | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
> index d0ae10bf6128..e1dc559d54aa 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
> @@ -41,20 +41,8 @@
>   */
>  static int of_cti_get_cpu_at_node(const struct device_node *node)
>  {
> -	int cpu;
> -	struct device_node *dn;
> +	int cpu = of_cpu_phandle_to_id(node, NULL, 0);

Could do a single line - kind of down to coresight maintainers preference though!

	return (of_cpu_phandle_to_id(node, NULL, 0) < 0) ? -1; cpu;

+CC Suzuki (you got the two reviewers in the +CC but not the maintainer)


>  
> -	if (node == NULL)
> -		return -1;
> -
> -	dn = of_parse_phandle(node, "cpu", 0);
> -	/* CTI affinity defaults to no cpu */
> -	if (!dn)
> -		return -1;
> -	cpu = of_cpu_node_to_id(dn);
> -	of_node_put(dn);
> -
> -	/* No Affinity  if no cpu nodes are found */
>  	return (cpu < 0) ? -1 : cpu;
>  }
>  


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/5] coresight: Use of_cpu_phandle_to_id for grabbing CPU id
  2025-07-08 15:15 ` [PATCH v2 4/5] coresight: " Alireza Sanaee
@ 2025-07-11  9:30   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11  9:30 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

On Tue, 8 Jul 2025 16:15:01 +0100
Alireza Sanaee <alireza.sanaee@huawei.com> wrote:

> Use the newly created API to grab CPU id.
> 
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

We could just squash this to the call site, however it is in
an if / else if with the much larger acpi equivalent call so
probably worth keeping this trivial helper.
> ---
>  drivers/hwtracing/coresight/coresight-platform.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 0db64c5f4995..95d46ea08936 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -167,20 +167,7 @@ of_coresight_get_output_ports_node(const struct device_node *node)
>  
>  static int of_coresight_get_cpu(struct device *dev)
>  {
> -	int cpu;
> -	struct device_node *dn;
> -
> -	if (!dev->of_node)
> -		return -ENODEV;
> -
> -	dn = of_parse_phandle(dev->of_node, "cpu", 0);
> -	if (!dn)
> -		return -ENODEV;
> -
> -	cpu = of_cpu_node_to_id(dn);
> -	of_node_put(dn);
> -
> -	return cpu;
> +	return of_cpu_phandle_to_id(dev->of_node, NULL, 0);
>  }
>  
>  /*


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/5] of: add infra for finding CPU id from phandle
  2025-07-08 15:14 ` [PATCH v2 1/5] of: add infra for finding CPU id from phandle Alireza Sanaee
  2025-07-11  9:17   ` Jonathan Cameron
@ 2025-07-11  9:35   ` Jonathan Cameron
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11  9:35 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

On Tue, 8 Jul 2025 16:14:58 +0100
Alireza Sanaee <alireza.sanaee@huawei.com> wrote:

> Get CPU id from phandle. Many drivers get do this by getting hold of CPU
> node first through a phandle and then find the CPU ID using the relevant
> function. This commit encapsulates cpu node finding and improves
> readability.
> 
> The API interface requires two parameters, 1) node, 2) pointer to
> pointer of CPU node, 3) cpu node index. API sets the pointer to the CPU
> node and allows the driver to play with the CPU itself, for logging
> purposes for instance.
> 
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> ---
>  drivers/of/cpu.c   | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  9 +++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/of/cpu.c b/drivers/of/cpu.c
> index 5214dc3d05ae..494d47470f94 100644
> --- a/drivers/of/cpu.c
> +++ b/drivers/of/cpu.c
> @@ -173,6 +173,46 @@ int of_cpu_node_to_id(struct device_node *cpu_node)
>  }
>  EXPORT_SYMBOL(of_cpu_node_to_id);
>  
> +/**
> + * of_cpu_phandle_to_id: Get the logical CPU number for a given device_node
> + *
> + * @node: Pointer to the device_node containing CPU phandle.
> + * @cpu_np: Pointer to the device_node for CPU.
> + * @cpu_idx: The index of the CPU in the list of CPUs.
> + *
> + * Return: The logical CPU number of the given CPU device_node or -ENODEV if
> + * the CPU is not found, or if the node is NULL, it returns -1. On success,
> + * cpu_np will always point to the retrieved CPU device_node with refcount
> + * incremented, use of_node_put() on it when done.
> + */
> +int of_cpu_phandle_to_id(const struct device_node *node,
> +			 struct device_node **cpu_np,
> +			 uint8_t cpu_idx)
> +{
> +	struct device_node *local_cpu_node;
> +	int cpu;
> +
> +	if (!node)
> +		return -1;
> +
> +	local_cpu_node = of_parse_phandle(node, "cpu", 0);

Sorry - half asleep in earlier reviews. This is only valid if cpu_idx = 0 I think?

	struct device_node *local_cpu_node = NULL;

...
	if (cpu_idx == 0)
		local_cpu_node = of_parse_phandle(node, "cpu", 0);
	if (!local_cpu_node)
		local_cpu_node = of_parse_phandle(node, "cpus", cpu_idx);

Is probably the simplest implementation.
Very unlikely we'd ever call it with the combination of cpu phandle and an index
as that will be constrained by the user (currently just the DSU I think)
but we should still make the code not doing crazy things if that happens.


> +	if (!local_cpu_node)
> +		local_cpu_node = of_parse_phandle(node, "cpus", cpu_idx);
> +
> +	if (!local_cpu_node)
> +		return -ENODEV;
> +
> +	cpu = of_cpu_node_to_id(local_cpu_node);
> +
> +	if (cpu_np)
> +		*cpu_np = local_cpu_node;
> +	else
> +		of_node_put(local_cpu_node);
> +
> +	return cpu;
> +}
> +EXPORT_SYMBOL(of_cpu_phandle_to_id);
> +
>  /**
>   * of_get_cpu_state_node - Get CPU's idle state node at the given index
>   *
> diff --git a/include/linux/of.h b/include/linux/of.h
> index a62154aeda1b..717e55065d99 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -365,6 +365,8 @@ extern const void *of_get_property(const struct device_node *node,
>  extern struct device_node *of_get_cpu_node(int cpu, unsigned int *thread);
>  extern struct device_node *of_cpu_device_node_get(int cpu);
>  extern int of_cpu_node_to_id(struct device_node *np);
> +extern int of_cpu_phandle_to_id(const struct device_node *np,
> +				struct device_node **cpu_np, uint8_t cpu_idx);
>  extern struct device_node *of_get_next_cpu_node(struct device_node *prev);
>  extern struct device_node *of_get_cpu_state_node(const struct device_node *cpu_node,
>  						 int index);
> @@ -680,6 +682,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
>  	return -ENODEV;
>  }
>  
> +static inline int of_cpu_phandle_to_id(const struct device_node *np,
> +				       struct device_node **cpu_np,
> +				       uint8_t cpu_idx)
> +{
> +	return -ENODEV;
> +}
> +
>  static inline struct device_node *of_get_next_cpu_node(struct device_node *prev)
>  {
>  	return NULL;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/5] perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id
  2025-07-08 15:15 ` [PATCH v2 5/5] perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id Alireza Sanaee
@ 2025-07-11  9:36   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11  9:36 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	linux-arm-kernel, linux-kernel, linux-perf-users, linuxarm,
	mark.rutland, mike.leach, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

On Tue, 8 Jul 2025 16:15:02 +0100
Alireza Sanaee <alireza.sanaee@huawei.com> wrote:

> Update arm-dsu to use the new API, where both "cpus" and "cpu"
> properties are supported.

I'd gloss over that and just not mention support of "cpu" as
it never applies here and we just queried the number of phandles
for cpus a few lines up.

> 
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> ---
>  drivers/perf/arm_dsu_pmu.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index cb4fb59fe04b..1014b92c0fd2 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -591,17 +591,13 @@ static struct dsu_pmu *dsu_pmu_alloc(struct platform_device *pdev)
>  static int dsu_pmu_dt_get_cpus(struct device *dev, cpumask_t *mask)
>  {
>  	int i = 0, n, cpu;
> -	struct device_node *cpu_node;
>  
>  	n = of_count_phandle_with_args(dev->of_node, "cpus", NULL);
>  	if (n <= 0)
>  		return -ENODEV;
> +
Stray change - it's a valid one for readability but not in this patch.

>  	for (; i < n; i++) {
> -		cpu_node = of_parse_phandle(dev->of_node, "cpus", i);
> -		if (!cpu_node)
> -			break;
> -		cpu = of_cpu_node_to_id(cpu_node);
> -		of_node_put(cpu_node);
> +		cpu = of_cpu_phandle_to_id(dev->of_node, NULL, i);
>  		/*
>  		 * We have to ignore the failures here and continue scanning
>  		 * the list to handle cases where the nr_cpus could be capped


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id
  2025-07-08 15:15 ` [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id Alireza Sanaee
  2025-07-11  9:28   ` Jonathan Cameron
@ 2025-07-11 15:55   ` Mike Leach
  2025-07-11 16:08     ` Jonathan Cameron
  1 sibling, 1 reply; 14+ messages in thread
From: Mike Leach @ 2025-07-11 15:55 UTC (permalink / raw)
  To: Alireza Sanaee
  Cc: krzk, robh, coresight, devicetree, dianders, james.clark,
	jonathan.cameron, linux-arm-kernel, linux-kernel,
	linux-perf-users, linuxarm, mark.rutland, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

Hi,

On Tue, 8 Jul 2025 at 16:16, Alireza Sanaee <alireza.sanaee@huawei.com> wrote:
>
> Use the newly created API to grab CPU id.
>
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> ---
>  .../hwtracing/coresight/coresight-cti-platform.c   | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
> index d0ae10bf6128..e1dc559d54aa 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
> @@ -41,20 +41,8 @@
>   */
>  static int of_cti_get_cpu_at_node(const struct device_node *node)
>  {
> -       int cpu;
> -       struct device_node *dn;
> +       int cpu = of_cpu_phandle_to_id(node, NULL, 0);
>
> -       if (node == NULL)
> -               return -1;
> -
> -       dn = of_parse_phandle(node, "cpu", 0);
> -       /* CTI affinity defaults to no cpu */
> -       if (!dn)
> -               return -1;
> -       cpu = of_cpu_node_to_id(dn);
> -       of_node_put(dn);
> -
> -       /* No Affinity  if no cpu nodes are found */

Leave the above comment in place.

>         return (cpu < 0) ? -1 : cpu;
>  }
>
> --
> 2.43.0
>

With that

Reviewed-by: Mike Leach <mike.leach@linro.org>
-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id
  2025-07-11 15:55   ` Mike Leach
@ 2025-07-11 16:08     ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2025-07-11 16:08 UTC (permalink / raw)
  To: Mike Leach
  Cc: Alireza Sanaee, krzk, robh, coresight, devicetree, dianders,
	james.clark, linux-arm-kernel, linux-kernel, linux-perf-users,
	linuxarm, mark.rutland, ruanjinjie, saravanak,
	shameerali.kolothum.thodi

On Fri, 11 Jul 2025 16:55:15 +0100
Mike Leach <mike.leach@linaro.org> wrote:

> Hi,
> 
> On Tue, 8 Jul 2025 at 16:16, Alireza Sanaee <alireza.sanaee@huawei.com> wrote:
> >
> > Use the newly created API to grab CPU id.
> >
> > Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> > ---
> >  .../hwtracing/coresight/coresight-cti-platform.c   | 14 +-------------
> >  1 file changed, 1 insertion(+), 13 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
> > index d0ae10bf6128..e1dc559d54aa 100644
> > --- a/drivers/hwtracing/coresight/coresight-cti-platform.c
> > +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
> > @@ -41,20 +41,8 @@
> >   */
> >  static int of_cti_get_cpu_at_node(const struct device_node *node)
> >  {
> > -       int cpu;
> > -       struct device_node *dn;
> > +       int cpu = of_cpu_phandle_to_id(node, NULL, 0);
> >
> > -       if (node == NULL)
> > -               return -1;
> > -
> > -       dn = of_parse_phandle(node, "cpu", 0);
> > -       /* CTI affinity defaults to no cpu */
> > -       if (!dn)
> > -               return -1;
> > -       cpu = of_cpu_node_to_id(dn);
> > -       of_node_put(dn);
> > -
> > -       /* No Affinity  if no cpu nodes are found */  
> 
> Leave the above comment in place.
> 
> >         return (cpu < 0) ? -1 : cpu;
> >  }
> >
> > --
> > 2.43.0
> >  
> 
> With that
> 
> Reviewed-by: Mike Leach <mike.leach@linro.org>
Just in case Ali doesn't notice - typo in your address Mike!
(usually it is me who manages that ;)



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-07-11 16:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-08 15:14 [PATCH v2 0/5] Refactoring finding CPU phandles in DT Alireza Sanaee
2025-07-08 15:14 ` [PATCH v2 1/5] of: add infra for finding CPU id from phandle Alireza Sanaee
2025-07-11  9:17   ` Jonathan Cameron
2025-07-11  9:35   ` Jonathan Cameron
2025-07-08 15:14 ` [PATCH v2 2/5] arch_topology: update CPU map to use the new API Alireza Sanaee
2025-07-11  9:25   ` Jonathan Cameron
2025-07-08 15:15 ` [PATCH v2 3/5] coresight: cti: Use of_cpu_phandle_to_id for grabbing CPU id Alireza Sanaee
2025-07-11  9:28   ` Jonathan Cameron
2025-07-11 15:55   ` Mike Leach
2025-07-11 16:08     ` Jonathan Cameron
2025-07-08 15:15 ` [PATCH v2 4/5] coresight: " Alireza Sanaee
2025-07-11  9:30   ` Jonathan Cameron
2025-07-08 15:15 ` [PATCH v2 5/5] perf/arm-dsu: refactor cpu id retrieval via new API of_cpu_phandle_to_id Alireza Sanaee
2025-07-11  9:36   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).