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

[Rob Herring]

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