[U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property

[Simon Glass]

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>
> Add functions to iterate on all property with livetree
> - ofnode_get_first_property
> - ofnode_get_next_property
> - ofnode_get_property_by_prop
>
> For example:
> for (prop = ofnode_get_first_property(dev_ofnode(dev));
>      prop;
>      prop = ofnode_get_next_property(dev_ofnode(dev),prop))
> {
>      value = ofnode_get_property_by_prop(dev_ofnode(dev), prop,
>                                          &propname, &len);
> ....
> }
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> ---
>
>  drivers/core/of_access.c | 32 ++++++++++++++++++++++++++++
>  drivers/core/ofnode.c    | 45 ++++++++++++++++++++++++++++++++++++++++
>  include/dm/of_access.h   | 40 +++++++++++++++++++++++++++++++++++
>  include/dm/ofnode.h      | 39 +++++++++++++++++++++++++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)

Please can you add the dev_read() interface too?  Also need to support
CONFIG_DM_DEV_READ_INLINE in read.h

[..]

> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 13fedb7cf5..0418782aa2 100644
> --- a/include/dm/of_access.h
> +++ b/include/dm/of_access.h
> @@ -103,6 +103,46 @@ struct property *of_find_property(const struct device_node *np,
>  const void *of_get_property(const struct device_node *np, const char *name,
>                             int *lenp);
>
> +/**
> + * of_get_first_property()- get to the pointer of the first property
> + *
> + * Get pointer to the first property of the node, it is used to iterate
> + * and read all the property with of_get_next_property_by_prop().
> + *
> + * @p: Pointer to device node

np

> + * @return pointer to property or NULL if not found
> + */
> +const struct property *of_get_first_property(const struct device_node *np);
> +
> +/**
> + * of_get_next_property() - get to the pointer of the next property
> + *
> + * Get pointer to the next property of the node, it is used to iterate
> + * and read all the property with of_get_property_by_prop().
> + *
> + * @p: Pointer to device node

np

> + * @property: pointer of the current property
> + * @return pointer to next property or NULL if not found
> + */
> +const struct property *of_get_next_property(const struct device_node *np,
> +                                           const struct property *property);
> +
> +/**
> + * of_get_property_by_prop() - get a property value of a node property
> + *
> + * Get value for the property identified by node and property pointer.
> + *
> + * @node: node to read
> + * @property: pointer of the property to read
> + * @propname: place to property name on success

This can be NULL so please document that

> + * @lenp: place to put length on success

This can be NULL so please document that

> + * @return pointer to property value or NULL if error
> + */
> +const void *of_get_property_by_prop(const struct device_node *np,
> +                                   const struct property *property,
> +                                   const char **name,
> +                                   int *lenp);
> +
>  /**
>   * of_device_is_compatible() - Check if the node matches given constraints
>   * @device: pointer to node
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 5c4cbf0998..08d684cea0 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -543,7 +543,7 @@ int ofnode_decode_display_timing(ofnode node, int index,
>                                  struct display_timing *config);
>
>  /**
> - * ofnode_get_property()- - get a pointer to the value of a node property
> + * ofnode_get_property() - get a pointer to the value of a node property
>   *
>   * @node: node to read
>   * @propname: property to read
> @@ -552,6 +552,43 @@ int ofnode_decode_display_timing(ofnode node, int index,
>   */
>  const void *ofnode_get_property(ofnode node, const char *propname, int *lenp);
>
> +/**
> + * ofnode_get_first_property()- get to the pointer of the first property
> + *
> + * Get pointer to the first property of the node, it is used to iterate
> + * and read all the property with ofnode_get_property_by_prop().
> + *
> + * @node: node to read
> + * @return pointer or offset to property, used to iterate, or NULL
> + */
> +const void *ofnode_get_first_property(ofnode node);
> +
> +/**
> + * ofnode_get_next_property() - get to the pointer of the next property
> + *
> + * Get pointer to the next property of the node, it is used to iterate
> + * and read all the property with ofnode_get_property_by_prop().
> + *
> + * @node: node to read
> + * @property: pointer or offset of the current property
> + * @return pointer or offset to next property or NULL
> + */
> +const void *ofnode_get_next_property(ofnode node, const void *property);
> +
> +/**
> + * ofnode_get_property_by_prop() - get a pointer to the value of a node property
> + *
> + * Get value for the property identified by node and property.
> + *
> + * @node: node to read
> + * @property: pointer or offset of the property to read

Perhaps you should define an ofprop type for this? It is pretty ugly
to use a pointer.

In fact I wonder if ofprop should be:

struct ofprop {
   ofnode node;
   union {
      int offset;
      struct property *prop;
   };
}


> + * @propname: place to property name on success
> + * @lenp: place to put length on success

These two above can be NULL so please document that

> + * @return pointer to property or NULL if error
> + */
> +const void *ofnode_get_property_by_prop(ofnode node, const void *property,
> +                                       const char **propname, int *lenp);
> +
>  /**
>   * ofnode_is_available() - check if a node is marked available
>   *
> --
> 2.17.1
>

Regards,
Simon