Hello Peng,
On Fri, 28 Jun 2024 13:10:34 +0000
Peng Fan <peng.fan@xxxxxxx> wrote:
> > Subject: RE: [PATCH 1/2] of: property: add of_property_for_each_u64
> >
> > > Subject: Re: [PATCH 1/2] of: property: add of_property_for_each_u64
> > >
> > > +Luca
> > >
> > > On Fri, Jun 21, 2024 at 08:36:39PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@xxxxxxx>
> > > >
> > > > Preparing for assigned-clock-rates-u64 support, add function
> > > > of_property_for_each_u64 to iterate each u64 value
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > > > ---
> > > > drivers/of/property.c | 23 +++++++++++++++++++++++
> > > > include/linux/of.h | 24 ++++++++++++++++++++++++
> > > > 2 files changed, 47 insertions(+)
> > > >
> > > > diff --git a/drivers/of/property.c b/drivers/of/property.c index
> > > > 164d77cb9445..b89c3ab01d44 100644
> > > > --- a/drivers/of/property.c
> > > > +++ b/drivers/of/property.c
> > > > @@ -548,6 +548,29 @@ const __be32 *of_prop_next_u32(struct
> > > property
> > > > *prop, const __be32 *cur, }
> > > EXPORT_SYMBOL_GPL(of_prop_next_u32);
> > > >
> > > > +const __be32 *of_prop_next_u64(struct property *prop, const
> > > __be32 *cur,
> > > > + u64 *pu)
> > >
> > > struct property can be const
> >
> > Fix in v2. BTW, I am thinking something as below:
> >
> > const __be64 *of_prop_next_u64(const struct property *prop, const
> > __be64 *cur,
> > u64 *pu)
> > {
> > const void *curv = cur;
> >
> > if (!prop)
> > return NULL;
> >
> > if (!cur) {
> > curv = prop->value;
> > goto out_val;
> > }
> >
> > curv += sizeof(*cur);
> > if (curv >= prop->value + prop->length)
> > return NULL;
> >
> > out_val:
> > *pu = be64_to_cpup(curv);
> > return curv;
> > }
> > EXPORT_SYMBOL_GPL(of_prop_next_u64);
> >
> > >
> > > > +{
> > > > + const void *curv = cur;
> > > > +
> > > > + if (!prop)
> > > > + return NULL;
> > > > +
> > > > + if (!cur) {
> > > > + curv = prop->value;
> > > > + goto out_val;
> > > > + }
> > > > +
> > > > + curv += sizeof(*cur) * 2;
> > > > + if (curv >= prop->value + prop->length)
> > > > + return NULL;
> > > > +
> > > > +out_val:
> > > > + *pu = of_read_number(curv, 2);
> > > > + return curv;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(of_prop_next_u64);
> > > > +
> > > > const char *of_prop_next_string(struct property *prop, const char
> > > > *cur) {
> > > > const void *curv = cur;
> > > > diff --git a/include/linux/of.h b/include/linux/of.h index
> > > > 13cf7a43b473..464eca6a4636 100644
> > > > --- a/include/linux/of.h
> > > > +++ b/include/linux/of.h
> > > > @@ -439,6 +439,18 @@ extern int of_detach_node(struct
> > > device_node *);
> > > > */
> > > > const __be32 *of_prop_next_u32(struct property *prop, const
> > > __be32 *cur,
> > > > u32 *pu);
> > > > +
> > > > +/*
> > > > + * struct property *prop;
> > > > + * const __be32 *p;
> > > > + * u64 u;
> > > > + *
> > > > + * of_property_for_each_u64(np, "propname", prop, p, u)
> > > > + * printk("U64 value: %llx\n", u);
> > > > + */
> > > > +const __be32 *of_prop_next_u64(struct property *prop, const
> > > __be32 *cur,
> > > > + u64 *pu);
> > > > +
> > > > /*
> > > > * struct property *prop;
> > > > * const char *s;
> > > > @@ -834,6 +846,12 @@ static inline const __be32
> > > *of_prop_next_u32(struct property *prop,
> > > > return NULL;
> > > > }
> > > >
> > > > +static inline const __be32 *of_prop_next_u64(struct property
> > *prop,
> > > > + const __be32 *cur, u64 *pu)
> > > > +{
> > > > + return NULL;
> > > > +}
> > > > +
> > > > static inline const char *of_prop_next_string(struct property *prop,
> > > > const char *cur)
> > > > {
> > > > @@ -1437,6 +1455,12 @@ static inline int
> > > of_property_read_s32(const struct device_node *np,
> > > > p; \
> > > > p = of_prop_next_u32(prop, p, &u))
> > > >
> > > > +#define of_property_for_each_u64(np, propname, prop, p, u) \
> > > > + for (prop = of_find_property(np, propname, NULL), \
> > > > + p = of_prop_next_u64(prop, NULL, &u); \
> > > > + p; \
> > > > + p = of_prop_next_u64(prop, p, &u))
> > >
> > > I think we want to define this differently to avoid exposing struct
> > > property and the property data directly. Like this:
> > >
> > > #define of_property_for_each_u64(np, propname, u) \
> > > for (struct property *_prop = of_find_property(np, propname, NULL),
> > > const __be32 *_p = of_prop_next_u64(_prop, NULL, &u);
> > > _p;
> > > _p = of_prop_next_u64(_prop, _p, &u))
>
> This will trigger a compilation error, because C not allow
> declare two variables with different types as for loop expression 1.
> Need to think about other methods.
I have a working draft here where I solved it somehow, let me just find
the proper branch and send it. Perhaps next week, but I'm striving to do
that by Mon-Tue.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
