Re: [PATCH v1 1/5] DT: add infra for finding CPU id from phandle.

On Tue, Apr 22, 2025 at 3:44 AM Alireza Sanaee
<[email protected]> 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 making driver
> code easier to cleaner.

easier to cleaner?

In the subject, use the prefix of the sub-system (of), and drop the period.

>
> The API interface requires three parameters, 1) node, 2) pointer to CPU
> node, and 3) property which includes the phandle. API sets the pointer
> to the CPU node and allows the driver to play with the CPU itself, for
> logging purposes for instance.
> ---
>  drivers/of/cpu.c   | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  3 +++
>  2 files changed, 43 insertions(+)
>
> diff --git a/drivers/of/cpu.c b/drivers/of/cpu.c
> index 5214dc3d05ae..c2d729999a4e 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.
> + * @prop: String of property holding the phandle.
> + *
> + * Return: The logical CPU number of the given CPU device_node or -ENODEV if
> + * the CPU is not found. If the property is not found, it returns -1. On
> + * success, cpu_np will always point to the retrieved CPU device_node.
> + */
> +int of_cpu_phandle_to_id(const struct device_node *node,
> +                        const struct device_node *cpu_np,

This needs to 'struct device_node **cpu_np' to actually work.

> +                        const char * prop)

I think prop here is always 'cpus' or 'cpu', so we can just try both
rather than passing it in. A node is not going to have both. With
'cpus', you need to provide the index.

> +{
> +       bool found = false;
> +       int cpu, ret;
> +       struct device_node *np;
> +       struct of_phandle_args args;
> +
> +       if (!node || !prop)
> +               return -1;
> +
> +       ret = of_parse_phandle_with_args(node, prop, NULL, 0, &args);
> +       if (ret < 0)
> +               return ret;
> +
> +       cpu_np = args.np;
> +       for_each_possible_cpu(cpu) {
> +               np = of_cpu_device_node_get(cpu);
> +               found = (cpu_np == np);
> +               of_node_put(np);
> +               if (found)
> +                       return cpu;
> +       }

This loop is just of_cpu_node_to_id(), use it here.


> +
> +       return -ENODEV;
> +}
> +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 eaf0e2a2b75c..5e51e57478b1 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -360,6 +360,9 @@ 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,
> +                               const struct device_node *cpu_np,
> +                               const char *propname);

You need an empty static inline version.

>  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);
> --
> 2.43.0
>