Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties

Rafael J. Wysocki
2014-10-19T23:17:04Z

On Saturday, October 18, 2014 11:35:21 AM Arnd Bergmann wrote:
> On Friday 17 October 2014 14:14:53 Rafael J. Wysocki wrote:
> > +/**
> > + * fwnode_property_present - check if a property of a firmware node is present
> > + * @fwnode: Firmware node whose property to check
> > + * @propname: Name of the property
> > + */
> > +bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
> > +{
> > + if (is_of_node(fwnode))
> > + return of_property_read_bool(of_node(fwnode), propname);
> > + else if (is_acpi_node(fwnode))
> > + return !acpi_dev_prop_get(acpi_node(fwnode), propname, NULL);
> > +
> > + return false;
> > +}
> > +EXPORT_SYMBOL_GPL(fwnode_property_present);
> >
>
> Should this be
>
> return acpi_dev_prop_get(acpi_node(fwnode), propname, NULL);
>
> without the '!'?

No, acpi_dev_prop_get() returns 0 on success. :-)

> I'm also unsure about the '_present' vs '_read_bool' naming. IIRC we had
> a long debate about this before we decided on 'read_bool' for DT, and
> I don't really want to start a new debate, but being consistent would
> be nice.
>
> We could of course have
>
> static inline bool fwnode_property_read_bool(struct fwnode_handle *fwnode, const char *propname)
> {
> return fwnode_property_present(fwnode, propname);
> }
>
> which is completely redundant, but would help for drivers using the
> interface to document whether we are checking for bool property that
> we expect to be either empty or absent (_get_bool), vs checking for
> the presence of a non-empty property (_present).

I'm fine with that, so I'll add fwnode_property_read_bool() (and an analogous
wrapper for device_) to patches [02/12] and [09/12].

I'll be sending updates of them shortly, so please have a look at those.

Rafael