Re: [RESEND PATCH v3 1/2] device property: Add function to search for named child of device

From: Rafael J. Wysocki
Date: Tue Jun 21 2016 - 07:48:31 EST


On Tuesday, June 21, 2016 02:11:26 PM Mika Westerberg wrote:
> On Mon, Jun 20, 2016 at 12:38:58PM +0100, Adam Thomson wrote:
> > For device nodes in both DT and ACPI, it possible to have named
> > child nodes which contain properties (an existing example being
> > gpio-leds). This adds a function to find a named child node for
> > a device which can be used by drivers for property retrieval.
> >
> > For DT data node name matching, of_node_cmp() and similar functions
> > are made available outside of CONFIG_OF block so the new function
> > can reference these for DT and non-DT builds.
> >
> > For ACPI data node name matching, a helper function is also added
> > which returns false if CONFIG_ACPI is not set, otherwise it
> > performs a string comparison on the data node name. This avoids
> > using the acpi_data_node struct for non CONFIG_ACPI builds,
> > which would otherwise cause a build failure.
> >
> > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx>
> > Tested-by: Sathyanarayana Nujella <sathyanarayana.nujella@xxxxxxxxx>
> > Acked-by: Rob Herring <robh@xxxxxxxxxx>
> > ---
> >
> > Changes in v3:
> > - Move of_*_cmp() functions in of.h outside of CONFIG_OF block so they are
> > available for non-DT builds
> > - In device_get_named_child_node(), use of_node_cmp() helper macro instead of
> > strcasecmp() (node names not alway case insensitive, depending on platform).
> >
> > Changes in v2:
> > - Rebase to v4.7-rc1
> >
> > drivers/base/property.c | 28 ++++++++++++++++++++++++++++
> > include/acpi/acpi_bus.h | 7 +++++++
> > include/linux/acpi.h | 6 ++++++
> > include/linux/of.h | 14 +++++++-------
> > include/linux/property.h | 3 +++
> > 5 files changed, 51 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index f38c21d..43a36d6 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -888,6 +888,34 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
> > EXPORT_SYMBOL_GPL(device_get_next_child_node);
> >
> > /**
> > + * device_get_named_child_node - Return first matching named child node handle
> > + * @dev: Device to find the named child node for.
> > + * @childname: String to match child node name against.
> > + */
> > +struct fwnode_handle *device_get_named_child_node(struct device *dev,
> > + const char *childname)
> > +{
> > + struct fwnode_handle *child;
> > +
> > + /*
> > + * Find first matching named child node of this device.
> > + * For ACPI this will be a data only sub-node.
> > + */
> > + device_for_each_child_node(dev, child) {
> > + if (is_of_node(child)) {
> > + if (!of_node_cmp(to_of_node(child)->name, childname))
> > + return child;
> > + } else if (is_acpi_data_node(child)) {
> > + if (acpi_data_node_match(child, childname))
> > + return child;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(device_get_named_child_node);
> > +
> > +/**
> > * fwnode_handle_put - Drop reference to a device node
> > * @fwnode: Pointer to the device node to drop the reference to.
> > *
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index 788c6c3..993bdd0 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -420,6 +420,13 @@ static inline struct acpi_data_node *to_acpi_data_node(struct fwnode_handle *fwn
> > container_of(fwnode, struct acpi_data_node, fwnode) : NULL;
> > }
> >
> > +static inline bool acpi_data_node_match(struct fwnode_handle *fwnode,
> > + const char *name)
> > +{
> > + return is_acpi_data_node(fwnode) ?
> > + (!strcasecmp(to_acpi_data_node(fwnode)->name, name)) : false;
> > +}
>
> Looks fine to me.
>
> One question - is it expected that matching ACPI data nodes is always
> case insensitive?

That would not be a correct expectation in theory, although I don't think it
really matters in practice.

Thanks,
Rafael