RE: [PATCH 1/2] of: property: add of_property_for_each_u64

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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))
> 

Sure, I will fix in v2.

Thanks,
Peng.

> See this discussion for context[1].
> 
> Rob
> 
> [1] https://lore.kernel.org/all/20240624232122.3cfe03f8@booty/






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux