Subject: Re: [PATCH 2/6] Consolidate of_device_is_compatible

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 25 Apr 2007 09:59:24 +1000

On Tue, 2007-04-24 at 22:38 +1000, Stephen Rothwell wrote:
> The only difference here is that Sparc uses strncmp to match compatibility
> names while PowerPC uses strncasecmp.

We should settle for a single implementation. In theory, strncmp would
be the way to go but there have been "bugs" here or there, especially in
Apple DTs, that made me use strncasecmp instead in the past.

Dave, what do you reckon ? I should try to find out the bogus machines
and add workarounds in the various drivers etc... instead or we can just
go common on strncasecmp ?

Ben.

> Signed-off-by: Stephen Rothwell <[email-masked]>
> ---
>  arch/powerpc/kernel/prom.c |   25 -------------------------
>  arch/sparc/kernel/prom.c   |   21 ---------------------
>  arch/sparc64/kernel/prom.c |   21 ---------------------
>  drivers/openfw/base.c      |   24 ++++++++++++++++++++++++
>  include/asm-powerpc/prom.h |    2 ++
>  include/asm-sparc/prom.h   |    2 ++
>  include/asm-sparc64/prom.h |    2 ++
>  7 files changed, 30 insertions(+), 67 deletions(-)
> 
> -- 
> Cheers,
> Stephen Rothwell                    [email-masked]
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index d701e89..723df55 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -1042,31 +1042,6 @@ void __init early_init_devtree(void *params)
>  
>  #undef printk
>  
> -/** Checks if the given "compat" string matches one of the strings in
> - * the device's "compatible" property
> - */
> -int of_device_is_compatible(const struct device_node *device,
> -		const char *compat)
> -{
> -	const char* cp;
> -	int cplen, l;
> -
> -	cp = of_get_property(device, "compatible", &cplen);
> -	if (cp == NULL)
> -		return 0;
> -	while (cplen > 0) {
> -		if (strncasecmp(cp, compat, strlen(compat)) == 0)
> -			return 1;
> -		l = strlen(cp) + 1;
> -		cp += l;
> -		cplen -= l;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(of_device_is_compatible);
> -
> -
>  /**
>   * Indicates whether the root node has a given value in its
>   * compatible property.
> diff --git a/arch/sparc/kernel/prom.c b/arch/sparc/kernel/prom.c
> index ac3f3c2..f2ce0d4 100644
> --- a/arch/sparc/kernel/prom.c
> +++ b/arch/sparc/kernel/prom.c
> @@ -32,27 +32,6 @@ static struct device_node *allnodes;
>   */
>  static DEFINE_RWLOCK(devtree_lock);
>  
> -int of_device_is_compatible(const struct device_node *device,
> -			    const char *compat)
> -{
> -	const char* cp;
> -	int cplen, l;
> -
> -	cp = of_get_property(device, "compatible", &cplen);
> -	if (cp == NULL)
> -		return 0;
> -	while (cplen > 0) {
> -		if (strncmp(cp, compat, strlen(compat)) == 0)
> -			return 1;
> -		l = strlen(cp) + 1;
> -		cp += l;
> -		cplen -= l;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(of_device_is_compatible);
> -
>  struct device_node *of_get_parent(const struct device_node *node)
>  {
>  	struct device_node *np;
> diff --git a/arch/sparc64/kernel/prom.c b/arch/sparc64/kernel/prom.c
> index 7ef5488..d6dd972 100644
> --- a/arch/sparc64/kernel/prom.c
> +++ b/arch/sparc64/kernel/prom.c
> @@ -36,27 +36,6 @@ static struct device_node *allnodes;
>   */
>  static DEFINE_RWLOCK(devtree_lock);
>  
> -int of_device_is_compatible(const struct device_node *device,
> -			    const char *compat)
> -{
> -	const char* cp;
> -	int cplen, l;
> -
> -	cp = of_get_property(device, "compatible", &cplen);
> -	if (cp == NULL)
> -		return 0;
> -	while (cplen > 0) {
> -		if (strncmp(cp, compat, strlen(compat)) == 0)
> -			return 1;
> -		l = strlen(cp) + 1;
> -		cp += l;
> -		cplen -= l;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(of_device_is_compatible);
> -
>  struct device_node *of_get_parent(const struct device_node *node)
>  {
>  	struct device_node *np;
> diff --git a/drivers/openfw/base.c b/drivers/openfw/base.c
> index 04a8cc2..0751313 100644
> --- a/drivers/openfw/base.c
> +++ b/drivers/openfw/base.c
> @@ -63,3 +63,27 @@ const void *of_get_property(const struct device_node *np, const char *name,
>  	return pp ? pp->value : NULL;
>  }
>  EXPORT_SYMBOL(of_get_property);
> +
> +/** Checks if the given "compat" string matches one of the strings in
> + * the device's "compatible" property
> + */
> +int of_device_is_compatible(const struct device_node *device,
> +		const char *compat)
> +{
> +	const char* cp;
> +	int cplen, l;
> +
> +	cp = of_get_property(device, "compatible", &cplen);
> +	if (cp == NULL)
> +		return 0;
> +	while (cplen > 0) {
> +		if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> +			return 1;
> +		l = strlen(cp) + 1;
> +		cp += l;
> +		cplen -= l;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(of_device_is_compatible);
> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
> index 30cea54..1f3e8cd 100644
> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -24,6 +24,8 @@
>  #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	1
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
>  
> +#define of_compat_cmp(s1, s2, l)	strncasecmp((s1), (s2), (l))
> +
>  /* Definitions used by the flattened device tree */
>  #define OF_DT_HEADER		0xd00dfeed	/* marker */
>  #define OF_DT_BEGIN_NODE	0x1		/* Start of node, full name */
> diff --git a/include/asm-sparc/prom.h b/include/asm-sparc/prom.h
> index 411018d..cc049fd 100644
> --- a/include/asm-sparc/prom.h
> +++ b/include/asm-sparc/prom.h
> @@ -23,6 +23,8 @@
>  #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	2
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
>  
> +#define of_compat_cmp(s1, s2, l)	strncmp((s1), (s2), (l))
> +
>  typedef u32 phandle;
>  typedef u32 ihandle;
>  
> diff --git a/include/asm-sparc64/prom.h b/include/asm-sparc64/prom.h
> index 1097e66..843f9e8 100644
> --- a/include/asm-sparc64/prom.h
> +++ b/include/asm-sparc64/prom.h
> @@ -23,6 +23,8 @@
>  #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	2
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
>  
> +#define of_compat_cmp(s1, s2, l)	strncmp((s1), (s2), (l))
> +
>  typedef u32 phandle;
>  typedef u32 ihandle;
>