linux-kernel - RE: [PATCH 1/2] of: property: add of_property_for_each

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 28 Jun 2024 13:10:34 +0000
From: Peng Fan <peng.fan@....com>
To: Rob Herring <robh@...nel.org>, "Peng Fan (OSS)" <peng.fan@....nxp.com>,
	Luca Ceresoli <luca.ceresoli@...tlin.com>
CC: Saravana Kannan <saravanak@...gle.com>, Michael Turquette
	<mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
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
> 
> > 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@....com>
> > >
> > > 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@....com>
> > > ---
> > >  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.

Thanks,
Peng.

> >
> 
> Sure, I will fix in v2.
> 
> Thanks,
> Peng.
> 
> > See this discussion for context[1].
> >
> > Rob
> >
> > [1] https://lore.kernel.org/all/20240624232122.3cfe03f8@booty/