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

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

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@diasemi.com>
> Tested-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> 
> 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?