diff mbox

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

Message ID 866c9edccdd89805f6a0c0aa92f8a78ae616ed61.1466421714.git.Adam.Thomson.Opensource@diasemi.com (mailing list archive)
State New, archived
Headers show

Commit Message

Adam Thomson June 20, 2016, 11:38 a.m. UTC
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(-)

--
1.9.3

Comments

Adam Thomson June 21, 2016, 10:20 a.m. UTC | #1
On 20 June 2016 12:39, 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;
> +}
> +
>  static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
>  {
>  	return &adev->fwnode;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 288fac5..03039c4 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -568,6 +568,12 @@ static inline struct acpi_data_node
> *to_acpi_data_node(struct fwnode_handle *fwn
>  	return NULL;
>  }
> 
> +static inline bool acpi_data_node_match(struct fwnode_handle *fwnode,
> +					const char *name)
> +{
> +	return false;
> +}
> +
>  static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
>  {
>  	return NULL;
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 74eb28c..310e32f 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -238,13 +238,6 @@ static inline unsigned long of_read_ulong(const __be32
> *cell, int size)
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>  #endif
> 
> -/* Default string compare functions, Allow arch asm/prom.h to override */
> -#if !defined(of_compat_cmp)
> -#define of_compat_cmp(s1, s2, l)	strcasecmp((s1), (s2))
> -#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
> -#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
> -#endif
> -
>  #define OF_IS_DYNAMIC(x) test_bit(OF_DYNAMIC, &x->_flags)
>  #define OF_MARK_DYNAMIC(x) set_bit(OF_DYNAMIC, &x->_flags)
> 
> @@ -726,6 +719,13 @@ static inline void of_property_clear_flag(struct property *p,
> unsigned long flag
>  #define of_match_node(_matches, _node)	NULL
>  #endif /* CONFIG_OF */
> 
> +/* Default string compare functions, Allow arch asm/prom.h to override */
> +#if !defined(of_compat_cmp)
> +#define of_compat_cmp(s1, s2, l)	strcasecmp((s1), (s2))
> +#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
> +#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
> +#endif
> +
>  #if defined(CONFIG_OF) && defined(CONFIG_NUMA)
>  extern int of_node_to_nid(struct device_node *np);
>  #else
> diff --git a/include/linux/property.h b/include/linux/property.h
> index ecab11e..3a2f9ae 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -77,6 +77,9 @@ struct fwnode_handle *device_get_next_child_node(struct
> device *dev,
>  	for (child = device_get_next_child_node(dev, NULL); child;	\
>  	     child = device_get_next_child_node(dev, child))
> 
> +struct fwnode_handle *device_get_named_child_node(struct device *dev,
> +						  const char *childname);
> +
>  void fwnode_handle_put(struct fwnode_handle *fwnode);
> 
>  unsigned int device_get_child_node_count(struct device *dev);
> --
> 1.9.3

There still seems to be an issue with this patch set reaching the
linux-acpi mailing list (as well as the other vger.kernel.org lists). Have
spent some time looking into it but so far can't see anything obviously wrong
with the mail format. Also tried to reduce down the number of people on the
To/Cc list, and signed up my e-mail address to the linux-acpi list, but still
not having any joy.

Rafael, Mika, et al. Can you please have a look at this patch set and provide
any feedback on the ACPI part (assuming you still have it in your inbox)?

Thanks.
Andy Shevchenko June 21, 2016, 10:22 a.m. UTC | #2
On Tue, 2016-06-21 at 10:20 +0000, Opensource [Adam Thomson] wrote:
> There still seems to be an issue with this patch set reaching the
> linux-acpi mailing list (as well as the other vger.kernel.org lists).
> Have
> spent some time looking into it but so far can't see anything
> obviously wrong
> with the mail format. Also tried to reduce down the number of people
> on the
> To/Cc list

There is still _a lot_ of addresses in the Cc and To fields.
Adam Thomson June 21, 2016, 10:39 a.m. UTC | #3
On 21 June 2016 11:23, Andy Shevchenko wrote:

> > There still seems to be an issue with this patch set reaching the
> > linux-acpi mailing list (as well as the other vger.kernel.org lists).
> > Have
> > spent some time looking into it but so far can't see anything
> > obviously wrong
> > with the mail format. Also tried to reduce down the number of people
> > on the
> > To/Cc list
> 
> There is still _a lot_ of addresses in the Cc and To fields.

There are a reasonable number, but the patch set does cross ALSA, DT and ACPI,
and those addresses are retrieved from the getmaintainer.pl script as
maintainers/supporters of those subsystems. I've also seen e-mails with larger
To and Cc fields appear on the various mailing lists without issue, so am
struggling to figure out why my initial patches aren't making it.

I can of course try again with a further reduced number of To/Cc, but I don't
want to miss off important reviewers, and I really don't want to spam the lists
unnecessarily.
Mika Westerberg June 21, 2016, 11:11 a.m. UTC | #4
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?
Rafael J. Wysocki June 21, 2016, 11:42 a.m. UTC | #5
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
Adam Thomson June 21, 2016, 11:50 a.m. UTC | #6
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?
Mika Westerberg June 21, 2016, 11:54 a.m. UTC | #7
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.
Mika Westerberg June 21, 2016, 11:59 a.m. UTC | #8
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).
Adam Thomson June 21, 2016, 12:27 p.m. UTC | #9
21 June 2016 13:00, Mika Westerberg 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).

Understood. For DT they are case insensitive (except for Sparc platforms) so
having ACPI match seems to make sense. I can add a comment indicating this,
just for clarity.
Adam Thomson June 21, 2016, 2:07 p.m. UTC | #10
On 21 June 2016 13:27, Adam Thomson 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).
> 
> Understood. For DT they are case insensitive (except for Sparc platforms) so
> having ACPI match seems to make sense. I can add a comment indicating this,
> just for clarity.

Actually, I'm going to back-track. Having seen the following discussion thread
on node name matching, it would seem the expected behaviour is that names should
be case sensitive for DT:

http://www.spinics.net/lists/devicetree/msg130870.html

Will follow that lead, and do the same for ACPI.
Rafael J. Wysocki June 21, 2016, 3:11 p.m. UTC | #11
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
diff mbox

Patch

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;
+}
+
 static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
 {
 	return &adev->fwnode;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 288fac5..03039c4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -568,6 +568,12 @@  static inline struct acpi_data_node *to_acpi_data_node(struct fwnode_handle *fwn
 	return NULL;
 }

+static inline bool acpi_data_node_match(struct fwnode_handle *fwnode,
+					const char *name)
+{
+	return false;
+}
+
 static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
 {
 	return NULL;
diff --git a/include/linux/of.h b/include/linux/of.h
index 74eb28c..310e32f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -238,13 +238,6 @@  static inline unsigned long of_read_ulong(const __be32 *cell, int size)
 #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
 #endif

-/* Default string compare functions, Allow arch asm/prom.h to override */
-#if !defined(of_compat_cmp)
-#define of_compat_cmp(s1, s2, l)	strcasecmp((s1), (s2))
-#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
-#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
-#endif
-
 #define OF_IS_DYNAMIC(x) test_bit(OF_DYNAMIC, &x->_flags)
 #define OF_MARK_DYNAMIC(x) set_bit(OF_DYNAMIC, &x->_flags)

@@ -726,6 +719,13 @@  static inline void of_property_clear_flag(struct property *p, unsigned long flag
 #define of_match_node(_matches, _node)	NULL
 #endif /* CONFIG_OF */

+/* Default string compare functions, Allow arch asm/prom.h to override */
+#if !defined(of_compat_cmp)
+#define of_compat_cmp(s1, s2, l)	strcasecmp((s1), (s2))
+#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
+#define of_node_cmp(s1, s2)		strcasecmp((s1), (s2))
+#endif
+
 #if defined(CONFIG_OF) && defined(CONFIG_NUMA)
 extern int of_node_to_nid(struct device_node *np);
 #else
diff --git a/include/linux/property.h b/include/linux/property.h
index ecab11e..3a2f9ae 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -77,6 +77,9 @@  struct fwnode_handle *device_get_next_child_node(struct device *dev,
 	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))

+struct fwnode_handle *device_get_named_child_node(struct device *dev,
+						  const char *childname);
+
 void fwnode_handle_put(struct fwnode_handle *fwnode);

 unsigned int device_get_child_node_count(struct device *dev);