Re: [PATCH 3/6] Consolidate of_find_property

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2007-04-24 at 22:39 +1000, Stephen Rothwell wrote:
> The only change here is that a readlock is taken while the property list
> is being traversed on Sparc where it was not taken previously.
> 
> Also, Sparc uses strcasecmp to compare property names while PowerPC
> uses strcmp.

Ok, so that's the opposite as the previous one... property names are
supposed to be case sensitive though, no ? Dave, how do you think we
should converge here ? I don't think we would take much risk on ppc by
using strncasecmp but it shouldn't be necessary...

Ben.

> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  arch/powerpc/kernel/prom.c |   24 +-----------------------
>  arch/sparc/kernel/prom.c   |   22 +---------------------
>  arch/sparc64/kernel/prom.c |   22 +---------------------
>  drivers/openfw/base.c      |   26 ++++++++++++++++++++++++++
>  include/asm-powerpc/prom.h |    1 +
>  include/asm-sparc/prom.h   |    1 +
>  include/asm-sparc64/prom.h |    1 +
>  7 files changed, 32 insertions(+), 65 deletions(-)
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 723df55..939d5e4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -79,10 +79,7 @@ struct boot_param_header *initial_boot_params;
>  
>  static struct device_node *allnodes = NULL;
>  
> -/* use when traversing tree through the allnext, child, sibling,
> - * or parent members of struct device_node.
> - */
> -static DEFINE_RWLOCK(devtree_lock);
> +extern rwlock_t devtree_lock;	/* temporary while merging */
>  
>  /* export that to outside world */
>  struct device_node *of_chosen;
> @@ -1454,25 +1451,6 @@ static int __init prom_reconfig_setup(void)
>  __initcall(prom_reconfig_setup);
>  #endif
>  
> -struct property *of_find_property(const struct device_node *np,
> -				  const char *name,
> -				  int *lenp)
> -{
> -	struct property *pp;
> -
> -	read_lock(&devtree_lock);
> -	for (pp = np->properties; pp != 0; pp = pp->next)
> -		if (strcmp(pp->name, name) == 0) {
> -			if (lenp != 0)
> -				*lenp = pp->length;
> -			break;
> -		}
> -	read_unlock(&devtree_lock);
> -
> -	return pp;
> -}
> -EXPORT_SYMBOL(of_find_property);
> -
>  /*
>   * Add a property to a node
>   */
> diff --git a/arch/sparc/kernel/prom.c b/arch/sparc/kernel/prom.c
> index f2ce0d4..0f5aab4 100644
> --- a/arch/sparc/kernel/prom.c
> +++ b/arch/sparc/kernel/prom.c
> @@ -27,10 +27,7 @@
>  
>  static struct device_node *allnodes;
>  
> -/* use when traversing tree through the allnext, child, sibling,
> - * or parent members of struct device_node.
> - */
> -static DEFINE_RWLOCK(devtree_lock);
> +extern rwlock_t devtree_lock;	/* temporary while merging */
>  
>  struct device_node *of_get_parent(const struct device_node *node)
>  {
> @@ -130,23 +127,6 @@ struct device_node *of_find_compatible_node(struct device_node *from,
>  }
>  EXPORT_SYMBOL(of_find_compatible_node);
>  
> -struct property *of_find_property(const struct device_node *np,
> -				  const char *name,
> -				  int *lenp)
> -{
> -	struct property *pp;
> -
> -	for (pp = np->properties; pp != 0; pp = pp->next) {
> -		if (strcasecmp(pp->name, name) == 0) {
> -			if (lenp != 0)
> -				*lenp = pp->length;
> -			break;
> -		}
> -	}
> -	return pp;
> -}
> -EXPORT_SYMBOL(of_find_property);
> -
>  int of_getintprop_default(struct device_node *np, const char *name, int def)
>  {
>  	struct property *prop;
> diff --git a/arch/sparc64/kernel/prom.c b/arch/sparc64/kernel/prom.c
> index d6dd972..c759660 100644
> --- a/arch/sparc64/kernel/prom.c
> +++ b/arch/sparc64/kernel/prom.c
> @@ -31,10 +31,7 @@
>  
>  static struct device_node *allnodes;
>  
> -/* use when traversing tree through the allnext, child, sibling,
> - * or parent members of struct device_node.
> - */
> -static DEFINE_RWLOCK(devtree_lock);
> +extern rwlock_t devtree_lock;	/* temporary while merging */
>  
>  struct device_node *of_get_parent(const struct device_node *node)
>  {
> @@ -134,23 +131,6 @@ struct device_node *of_find_compatible_node(struct device_node *from,
>  }
>  EXPORT_SYMBOL(of_find_compatible_node);
>  
> -struct property *of_find_property(const struct device_node *np,
> -				  const char *name,
> -				  int *lenp)
> -{
> -	struct property *pp;
> -
> -	for (pp = np->properties; pp != 0; pp = pp->next) {
> -		if (strcasecmp(pp->name, name) == 0) {
> -			if (lenp != 0)
> -				*lenp = pp->length;
> -			break;
> -		}
> -	}
> -	return pp;
> -}
> -EXPORT_SYMBOL(of_find_property);
> -
>  int of_getintprop_default(struct device_node *np, const char *name, int def)
>  {
>  	struct property *prop;
> diff --git a/drivers/openfw/base.c b/drivers/openfw/base.c
> index 0751313..2597d29 100644
> --- a/drivers/openfw/base.c
> +++ b/drivers/openfw/base.c
> @@ -18,6 +18,12 @@
>   */
>  #include <linux/module.h>
>  #include <linux/openfw.h>
> +#include <linux/spinlock.h>
> +
> +/* use when traversing tree through the allnext, child, sibling,
> + * or parent members of struct device_node.
> + */
> +DEFINE_RWLOCK(devtree_lock);
>  
>  int of_n_addr_cells(struct device_node *np)
>  {
> @@ -51,6 +57,26 @@ int of_n_size_cells(struct device_node *np)
>  }
>  EXPORT_SYMBOL(of_n_size_cells);
>  
> +struct property *of_find_property(const struct device_node *np,
> +				  const char *name,
> +				  int *lenp)
> +{
> +	struct property *pp;
> +
> +	read_lock(&devtree_lock);
> +	for (pp = np->properties; pp != 0; pp = pp->next) {
> +		if (of_prop_cmp(pp->name, name) == 0) {
> +			if (lenp != 0)
> +				*lenp = pp->length;
> +			break;
> +		}
> +	}
> +	read_unlock(&devtree_lock);
> +
> +	return pp;
> +}
> +EXPORT_SYMBOL(of_find_property);
> +
>  /*
>   * Find a property with a given name for a given node
>   * and return the value.
> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
> index 1f3e8cd..a0679be 100644
> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -25,6 +25,7 @@
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
>  
>  #define of_compat_cmp(s1, s2, l)	strncasecmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2)		strcmp((s1), (s2))
>  
>  /* Definitions used by the flattened device tree */
>  #define OF_DT_HEADER		0xd00dfeed	/* marker */
> diff --git a/include/asm-sparc/prom.h b/include/asm-sparc/prom.h
> index cc049fd..d3dc05b 100644
> --- a/include/asm-sparc/prom.h
> +++ b/include/asm-sparc/prom.h
> @@ -24,6 +24,7 @@
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
>  
>  #define of_compat_cmp(s1, s2, l)	strncmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2)		strcasecmp((s1), (s2))
>  
>  typedef u32 phandle;
>  typedef u32 ihandle;
> diff --git a/include/asm-sparc64/prom.h b/include/asm-sparc64/prom.h
> index 843f9e8..a1b64e7 100644
> --- a/include/asm-sparc64/prom.h
> +++ b/include/asm-sparc64/prom.h
> @@ -24,6 +24,7 @@
>  #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT	1
>  
>  #define of_compat_cmp(s1, s2, l)	strncmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2)		strcasecmp((s1), (s2))
>  
>  typedef u32 phandle;
>  typedef u32 ihandle;