Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427614 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RESEND PATCH v3 1/2] device property: Add function to search  for named child of device Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-21 13:20 +0200
    Re: [RESEND PATCH v3 1/2] device property: Add function to search for named child of device "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-21 13:50 +0200
      RE: [RESEND PATCH v3 1/2] device property: Add function to search  for named child of device "Opensource [Adam Thomson]" <Adam.Thomson.Opensource@diasemi.com> - 2016-06-21 14:00 +0200
        Re: [RESEND PATCH v3 1/2] device property: Add function to search  for named child of device Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-21 14:10 +0200
        Re: [RESEND PATCH v3 1/2] device property: Add function to search for named child of device "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-21 17:10 +0200
      Re: [RESEND PATCH v3 1/2] device property: Add function to search  for named child of device Mika Westerberg <mika.westerberg@linux.intel.com> - 2016-06-21 14:10 +0200

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

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?

[toc] | [next] | [standalone]

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

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@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?

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

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1427667

21 June 2016 12:42, Rafael J. Wysocki wrote:

> > > +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.

From my reading of the Hierarchical Data Extension and ACPI Spec, I thought
that was the case (section 19.3.1 ASL Names - ASL names are not case-sensitive
and will be converted to upper case). Am I misreading the documents/missing
something else?

[toc] | [prev] | [next] | [standalone]

#1427683

On Tue, Jun 21, 2016 at 11:50:01AM +0000, Opensource [Adam Thomson] wrote:
> 21 June 2016 12:42, Rafael J. Wysocki wrote:
> 
> > > > +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.
> 
> From my reading of the Hierarchical Data Extension and ACPI Spec, I thought
> that was the case (section 19.3.1 ASL Names - ASL names are not case-sensitive
> and will be converted to upper case). Am I misreading the documents/missing
> something else?

Those are names in the ASL code itself.

What we are talking here are actually just string values (name of the
data node).

[toc] | [prev] | [next] | [standalone]

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

On Tuesday, June 21, 2016 11:50:01 AM Opensource [Adam Thomson] wrote:
> 21 June 2016 12:42, Rafael J. Wysocki wrote:
> 
> > > > +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.
> 
> From my reading of the Hierarchical Data Extension and ACPI Spec, I thought
> that was the case (section 19.3.1 ASL Names - ASL names are not case-sensitive
> and will be converted to upper case).

Section 19.3.1 is applicable to object names and not to the data returned
by those objects.  The link names in the Hierarchical Data Extension are
data returned by _DSD (or other objects related to it).  They are general ACPI
strings and those are case-sensitive.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1427678

On Tue, Jun 21, 2016 at 01:42:16PM +0200, Rafael J. Wysocki wrote:
> > > +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.

OK.

Maybe it is good idea to document that in acpi_data_node_match(). A
comment explaining why we use strcasecmp() for now.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web