Re: [PATCH 1/3] of: base: convert index to unsigned for of_parse_phandle()

From: Rob Herring
Date: Thu Jan 13 2022 - 21:29:28 EST


On Thu, Jan 13, 2022 at 2:52 AM Michael Walle <michael@xxxxxxxx> wrote:
>
> Since commit 2021bd01ffcc ("of: Remove counting special case from
> __of_parse_phandle_with_args()"), the index is >=0, thus convert the

Ah good, that explains why we had signed in the first place.

> paramenter to unsigned of the of_parse_phandle() and all its variants.

typo.

> Make the smaller variants static inline, too.

This should be a separate patch.

> Suggested-by: Rob Herring <robh@xxxxxxxxxx>
> Signed-off-by: Michael Walle <michael@xxxxxxxx>
> ---
> drivers/of/base.c | 137 +++---------------------------------------
> include/linux/of.h | 147 ++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 142 insertions(+), 142 deletions(-)

A lot of moving around going on...

>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 8a24d37153b4..58b1b6ffc105 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1420,14 +1420,15 @@ int of_phandle_iterator_args(struct of_phandle_iterator *it,
> return count;
> }
>
> -static int __of_parse_phandle_with_args(const struct device_node *np,
> - const char *list_name,
> - const char *cells_name,
> - int cell_count, int index,
> - struct of_phandle_args *out_args)
> +int __of_parse_phandle_with_args(const struct device_node *np,
> + const char *list_name,
> + const char *cells_name,
> + int cell_count, unsigned int index,
> + struct of_phandle_args *out_args)
> {
> struct of_phandle_iterator it;
> - int rc, cur_index = 0;
> + unsigned int cur_index = 0;
> + int rc;
>
> /* Loop over the phandles until all the requested entry is found */
> of_for_each_phandle(&it, rc, np, list_name, cells_name, cell_count) {
> @@ -1471,82 +1472,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
> of_node_put(it.node);
> return rc;
> }
> -
> -/**
> - * of_parse_phandle - Resolve a phandle property to a device_node pointer
> - * @np: Pointer to device node holding phandle property
> - * @phandle_name: Name of property holding a phandle value
> - * @index: For properties holding a table of phandles, this is the index into
> - * the table
> - *
> - * Return: The device_node pointer with refcount incremented. Use
> - * of_node_put() on it when done.
> - */
> -struct device_node *of_parse_phandle(const struct device_node *np,
> - const char *phandle_name, int index)
> -{
> - struct of_phandle_args args;
> -
> - if (index < 0)
> - return NULL;
> -
> - if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
> - index, &args))
> - return NULL;
> -
> - return args.np;
> -}
> -EXPORT_SYMBOL(of_parse_phandle);
> -
> -/**
> - * of_parse_phandle_with_args() - Find a node pointed by phandle in a list
> - * @np: pointer to a device tree node containing a list
> - * @list_name: property name that contains a list
> - * @cells_name: property name that specifies phandles' arguments count
> - * @index: index of a phandle to parse out
> - * @out_args: optional pointer to output arguments structure (will be filled)
> - *
> - * This function is useful to parse lists of phandles and their arguments.
> - * Returns 0 on success and fills out_args, on error returns appropriate
> - * errno value.
> - *
> - * Caller is responsible to call of_node_put() on the returned out_args->np
> - * pointer.
> - *
> - * Example::
> - *
> - * phandle1: node1 {
> - * #list-cells = <2>;
> - * };
> - *
> - * phandle2: node2 {
> - * #list-cells = <1>;
> - * };
> - *
> - * node3 {
> - * list = <&phandle1 1 2 &phandle2 3>;
> - * };
> - *
> - * To get a device_node of the ``node2`` node you may call this:
> - * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
> - */
> -int of_parse_phandle_with_args(const struct device_node *np, const char *list_name,
> - const char *cells_name, int index,
> - struct of_phandle_args *out_args)
> -{
> - int cell_count = -1;
> -
> - if (index < 0)
> - return -EINVAL;
> -
> - /* If cells_name is NULL we assume a cell count of 0 */
> - if (!cells_name)
> - cell_count = 0;
> -
> - return __of_parse_phandle_with_args(np, list_name, cells_name,
> - cell_count, index, out_args);
> -}
> -EXPORT_SYMBOL(of_parse_phandle_with_args);
> +EXPORT_SYMBOL(__of_parse_phandle_with_args);
>
> /**
> * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> @@ -1593,7 +1519,8 @@ EXPORT_SYMBOL(of_parse_phandle_with_args);
> int of_parse_phandle_with_args_map(const struct device_node *np,
> const char *list_name,
> const char *stem_name,
> - int index, struct of_phandle_args *out_args)
> + unsigned int index,
> + struct of_phandle_args *out_args)
> {
> char *cells_name, *map_name = NULL, *mask_name = NULL;
> char *pass_name = NULL;
> @@ -1606,9 +1533,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> int i, ret, map_len, match;
> u32 list_size, new_size;
>
> - if (index < 0)
> - return -EINVAL;
> -
> cells_name = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
> if (!cells_name)
> return -ENOMEM;
> @@ -1732,47 +1656,6 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> }
> EXPORT_SYMBOL(of_parse_phandle_with_args_map);
>
> -/**
> - * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
> - * @np: pointer to a device tree node containing a list
> - * @list_name: property name that contains a list
> - * @cell_count: number of argument cells following the phandle
> - * @index: index of a phandle to parse out
> - * @out_args: optional pointer to output arguments structure (will be filled)
> - *
> - * This function is useful to parse lists of phandles and their arguments.
> - * Returns 0 on success and fills out_args, on error returns appropriate
> - * errno value.
> - *
> - * Caller is responsible to call of_node_put() on the returned out_args->np
> - * pointer.
> - *
> - * Example::
> - *
> - * phandle1: node1 {
> - * };
> - *
> - * phandle2: node2 {
> - * };
> - *
> - * node3 {
> - * list = <&phandle1 0 2 &phandle2 2 3>;
> - * };
> - *
> - * To get a device_node of the ``node2`` node you may call this:
> - * of_parse_phandle_with_fixed_args(node3, "list", 2, 1, &args);
> - */
> -int of_parse_phandle_with_fixed_args(const struct device_node *np,
> - const char *list_name, int cell_count,
> - int index, struct of_phandle_args *out_args)
> -{
> - if (index < 0)
> - return -EINVAL;
> - return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
> - index, out_args);
> -}
> -EXPORT_SYMBOL(of_parse_phandle_with_fixed_args);
> -
> /**
> * of_count_phandle_with_args() - Find the number of phandles references in a property
> * @np: pointer to a device tree node containing a list
> diff --git a/include/linux/of.h b/include/linux/of.h
> index ff143a027abc..df3af6d3cbe3 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -364,17 +364,11 @@ extern const struct of_device_id *of_match_node(
> const struct of_device_id *matches, const struct device_node *node);
> extern int of_modalias_node(struct device_node *node, char *modalias, int len);
> extern void of_print_phandle_args(const char *msg, const struct of_phandle_args *args);
> -extern struct device_node *of_parse_phandle(const struct device_node *np,
> - const char *phandle_name,
> - int index);
> -extern int of_parse_phandle_with_args(const struct device_node *np,
> - const char *list_name, const char *cells_name, int index,
> - struct of_phandle_args *out_args);
> +extern int __of_parse_phandle_with_args(const struct device_node *np, const
> + char *list_name, const char *cells_name, int cell_count,
> + unsigned int index, struct of_phandle_args *out_args);
> extern int of_parse_phandle_with_args_map(const struct device_node *np,
> - const char *list_name, const char *stem_name, int index,
> - struct of_phandle_args *out_args);
> -extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
> - const char *list_name, int cells_count, int index,
> + const char *list_name, const char *stem_name, unsigned int index,
> struct of_phandle_args *out_args);
> extern int of_count_phandle_with_args(const struct device_node *np,
> const char *list_name, const char *cells_name);
> @@ -416,6 +410,117 @@ extern int of_detach_node(struct device_node *);
>
> #define of_match_ptr(_ptr) (_ptr)
>
> +/**
> + * of_parse_phandle - Resolve a phandle property to a device_node pointer
> + * @np: Pointer to device node holding phandle property
> + * @phandle_name: Name of property holding a phandle value
> + * @index: For properties holding a table of phandles, this is the index into
> + * the table
> + *
> + * Return: The device_node pointer with refcount incremented. Use
> + * of_node_put() on it when done.
> + */
> +static inline struct device_node *of_parse_phandle(const struct device_node *np,
> + const char *phandle_name,
> + unsigned int index)
> +{
> + struct of_phandle_args args;
> +
> + if (__of_parse_phandle_with_args(np, phandle_name, NULL, 0,
> + index, &args))
> + return NULL;
> +
> + return args.np;
> +}
> +
> +/**
> + * of_parse_phandle_with_args() - Find a node pointed by phandle in a list
> + * @np: pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cells_name: property name that specifies phandles' arguments count
> + * @index: index of a phandle to parse out
> + * @out_args: optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example::
> + *
> + * phandle1: node1 {
> + * #list-cells = <2>;
> + * };
> + *
> + * phandle2: node2 {
> + * #list-cells = <1>;
> + * };
> + *
> + * node3 {
> + * list = <&phandle1 1 2 &phandle2 3>;
> + * };
> + *
> + * To get a device_node of the ``node2`` node you may call this:
> + * of_parse_phandle_with_args(node3, "list", "#list-cells", 1, &args);
> + */
> +static inline int of_parse_phandle_with_args(const struct device_node *np,
> + const char *list_name,
> + const char *cells_name,
> + unsigned int index,
> + struct of_phandle_args *out_args)
> +{
> + int cell_count = -1;
> +
> + /* If cells_name is NULL we assume a cell count of 0 */
> + if (!cells_name)
> + cell_count = 0;
> +
> + return __of_parse_phandle_with_args(np, list_name, cells_name,
> + cell_count, index, out_args);
> +}
> +
> +/**
> + * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
> + * @np: pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cell_count: number of argument cells following the phandle
> + * @index: index of a phandle to parse out
> + * @out_args: optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example::
> + *
> + * phandle1: node1 {
> + * };
> + *
> + * phandle2: node2 {
> + * };
> + *
> + * node3 {
> + * list = <&phandle1 0 2 &phandle2 2 3>;
> + * };
> + *
> + * To get a device_node of the ``node2`` node you may call this:
> + * of_parse_phandle_with_fixed_args(node3, "list", 2, 1, &args);
> + */
> +static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
> + const char *list_name,
> + int cell_count,
> + unsigned int index,
> + struct of_phandle_args *out_args)
> +{
> + return __of_parse_phandle_with_args(np, list_name, NULL, cell_count,
> + index, out_args);
> +}
> +
> /**
> * of_property_read_u8_array - Find and read an array of u8 from a property.
> *
> @@ -865,9 +970,19 @@ static inline int of_property_read_string_helper(const struct device_node *np,
> return -ENOSYS;
> }
>
> +static inline int __of_parse_phandle_with_args(const struct device_node *np,
> + const char *list_name,
> + const char *cells_name,
> + int cell_count,
> + unsigned int index,
> + struct of_phandle_args *out_args)
> +{
> + return -ENOSYS;
> +};
> +
> static inline struct device_node *of_parse_phandle(const struct device_node *np,
> const char *phandle_name,
> - int index)
> + unsigned int index)
> {
> return NULL;
> }
> @@ -875,7 +990,7 @@ static inline struct device_node *of_parse_phandle(const struct device_node *np,
> static inline int of_parse_phandle_with_args(const struct device_node *np,
> const char *list_name,
> const char *cells_name,
> - int index,
> + unsigned int index,
> struct of_phandle_args *out_args)
> {
> return -ENOSYS;
> @@ -884,15 +999,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
> static inline int of_parse_phandle_with_args_map(const struct device_node *np,

With these as static inlines, you only need them once unconditionally
as long as __of_parse_phandle_with_args() has an empty static inline.

Rob