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

[Patrick DELAUNAY]

Hi Simon,

> From: Simon Glass <sjg at chromium.org>
> Sent: mercredi 30 octobre 2019 02:48
> 
> 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

Yes I will it,

 
> [..]
> 
> > 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

ok

> > + * @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;
>    };
> }

Ok, I will do it in v2

The API become:

int ofnode_get_first_property(ofnode node, struct ofprop *prop);
int ofnode_get_next_property(struct ofprop *prop);

same prop is used as input parameter and ouput parameter.

const void *ofnode_get_property_by_prop(struct ofprop *prop,
					const char **propname, int *lenp);

same for dev_ function 


> > + * @propname: place to property name on success
> > + * @lenp: place to put length on success
> 
> These two above can be NULL so please document that

OK

> > + * @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