Re: [PATCH v1 2/3] of: Add support for reading a s32 from a multi-value property.

[Mark Rutland]

On Fri, Aug 19, 2016 at 09:47:34PM +0100, David Woodhouse wrote:
> On Fri, 2016-08-19 at 22:41 +0200, Heiko Stuebner wrote:
> > 
> > > So no, don't *add* any more of these functions. Only add the generic
> > > version. And if your driver isn't using the generic property
> > > functions... fix it.
> > 
> > As far as I can see, all the device_property_* functions are grounded on their
> > of_property_*, acpi_property_* etc counterparts and functions reading specific
> > elements (the _index variants) are currently not available at all.
> > 
> > drivers/base/property.c:
> > #define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval)ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ\
> > ÂÂÂÂÂÂÂÂ(val) ? of_property_read_##type##_array((node), (propname), (val), (nval))ÂÂÂÂÂÂ\
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂ : of_property_count_elems_of_size((node), (propname), sizeof(type))
> > 
> > So even if you're using the device_property_* functions you'd still need
> > a match in the underlying functions or am I missing something?
> 
> Yes, but the underlying function should never be used directly by
> drivers. 

There are properties which never make sense with ACPI, and there are
drivers which shouldn't exist in the case of ACPI (due to clashes with
core ACPI functionality). So using of_property_* for those, and not
matching any ACPI tables is perfectly fine.

IMO, an of_property_read_s32_index() function is perfectly fine (as is
an ACPI-specific variant, or a unified helper, if that's useful to some
driver).

There is no need to force drivers to superficially appear to support
ACPI when they have not been designed with ACPI in mind, and are
unlikely to function in an ACPI environment.

Thanks,
Mark.