Re: [RFC PATCH 4/9] of: Add property_ops callback for devices with of_node

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Sun, Aug 17, 2014 at 01:54:16PM +0100, Grant Likely wrote:
> On Sun, 17 Aug 2014 09:04:14 +0300, Mika Westerberg <mika.westerberg@linux.intel.com> wrote:
> > From: Aaron Lu <aaron.lu@intel.com>
> > 
> > With the unified device properties interface in place, add device tree support.
> > By adding the dev_prop_ops for of_node devices, drivers can access properties
> > from ACPI or Device Tree in a generic way.
> > 
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > Reviewed-by: Darren Hart <dvhart@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Hi Mika,
> 
> As commented on patch 3, the new API currently throws away type
> information so the compiler won't warn on an error. That needs to be
> fixed and this patch reworked accordingly.

The compiler should warn when we add static inline property accessor
functions for each type.

> 
> g.
> 
> ---
> >  drivers/of/base.c        | 194 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/of/platform.c    |   4 +-
> >  include/linux/property.h |   4 +
> >  3 files changed, 200 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index b9864806e9b8..527004d01423 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -20,7 +20,7 @@
> >  #include <linux/ctype.h>
> >  #include <linux/cpu.h>
> >  #include <linux/module.h>
> > -#include <linux/of.h>
> > +#include <linux/property.h>
> >  #include <linux/of_graph.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/slab.h>
> > @@ -1343,6 +1343,39 @@ int of_property_read_u64(const struct device_node *np, const char *propname,
> >  EXPORT_SYMBOL_GPL(of_property_read_u64);
> >  
> >  /**
> > + * of_property_read_u64_array - Find and read an array of 64 bit integers
> > + * from a property.
> > + *
> > + * @np:		device node from which the property value is to be read.
> > + * @propname:	name of the property to be searched.
> > + * @out_values:	pointer to return value, modified only if return value is 0.
> > + * @sz:		number of array elements to read
> > + *
> > + * Search for a property in a device node and read 64-bit value(s) from
> > + * it. Returns 0 on success, -EINVAL if the property does not exist,
> > + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> > + * property data isn't large enough.
> > + *
> > + * The out_values is modified only if a valid u64 value can be decoded.
> > + */
> > +int of_property_read_u64_array(const struct device_node *np,
> > +			       const char *propname, u64 *out_values,
> > +			       size_t sz)
> > +{
> > +	const __be32 *val = of_find_property_value_of_size(np, propname,
> > +						(sz * sizeof(*out_values)));
> > +
> > +	if (IS_ERR(val))
> > +		return PTR_ERR(val);
> > +
> > +	while (sz--) {
> > +		*out_values++ = of_read_number(val, 2);
> > +		val += 2;
> > +	};
> > +	return 0;
> > +}
> 
> Considering we don't even have a user of u64 arrays in all of the DT
> drivers, do we even need this type hook in the generic API?

Indeed, I think we can remove it for now.