Re: [PATCH] OF: kobj node lifecycle fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Fri, 13 Dec 2013 20:08:59 +0200, Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> After the move to having device nodes be proper kobjects the lifecycle
> of the node needs to be controlled better.
> 
> At first convert of_add_node() in the unflattened functions to
> of_init_node() which initializes the kobject so that of_node_get/put
> work correctly even before of_init is called.
> 
> Afterwards introduce of_node_is_initialized & of_node_is_attached that
> query the underlying kobject about the state (attached means kobj
> is visible in sysfs)
> 
> Using that make sure the lifecycle of the tree is correct at all
> times.
> 
> Signed-off-by: Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx>

Merged with some rework, thanks

g.

> ---
>  drivers/of/base.c  | 96 ++++++++++++++++++++++++++++++++++++++++++++----------
>  drivers/of/fdt.c   |  3 +-
>  include/linux/of.h | 15 +++++++++
>  3 files changed, 95 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 78cd546..39dda4c 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -245,45 +245,104 @@ static int __of_node_add(struct device_node *np)
>  int of_node_add(struct device_node *np)
>  {
>  	int rc = 0;
> -	kobject_init(&np->kobj, &of_node_ktype);
> +
> +	BUG_ON(!of_node_is_initialized(np));
> +
> +	if (!of_kset) {
> +		pr_warn("%s: of_node_add before of_init on %s\n",
> +				__func__, np->full_name);
> +		return 0;
> +	}
> +
>  	mutex_lock(&of_aliases_mutex);
> -	if (of_kset)
> -		rc = __of_node_add(np);
> +	rc = __of_node_add(np);
>  	mutex_unlock(&of_aliases_mutex);
>  	return rc;
>  }
>  
> +/*
> + * Initialize a new device node
> + *
> + * At the moment it is just initializing the kobj of the node.
> + * This occurs during unflattening and when creating dynamic nodes.
> + */
> +void of_node_init(struct device_node *np)
> +{
> +	kobject_init(&np->kobj, &of_node_ktype);
> +}
> +
>  #if defined(CONFIG_OF_DYNAMIC)
>  static void of_node_remove(struct device_node *np)
>  {
>  	struct property *pp;
>  
> -	for_each_property_of_node(np, pp)
> -		sysfs_remove_bin_file(&np->kobj, &pp->attr);
> +	BUG_ON(!of_node_is_initialized(np));
> +
> +	/* only remove properties if on sysfs */
> +	if (of_node_is_attached(np)) {
> +		for_each_property_of_node(np, pp)
> +			sysfs_remove_bin_file(&np->kobj, &pp->attr);
> +		/* delete from sysfs */
> +		kobject_del(&np->kobj);
> +	}
>  
> -	kobject_del(&np->kobj);
> +	/* finally remove the kobj_init ref */
> +	of_node_put(np);
>  }
>  #endif
>  
> +/* recursively attach the tree */
> +static __init int __of_populate(struct device_node *np)
> +{
> +	struct device_node *child;
> +	int rc;
> +
> +	/* add the parent first */
> +	rc = __of_node_add(np);
> +	if (rc)
> +		return rc;
> +
> +	/* the children afterwards */
> +	__for_each_child_of_node(np, child) {
> +		rc = __of_populate(child);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
>  static int __init of_init(void)
>  {
>  	struct device_node *np;
> +	int rc;
>  
>  	of_kset = kset_create_and_add("devicetree", NULL, firmware_kobj);
>  	if (!of_kset)
>  		return -ENOMEM;
>  
> -	/* Make sure all nodes added before this time get added to sysfs */
>  	mutex_lock(&of_aliases_mutex);
> -	for_each_of_allnodes(np)
> -		__of_node_add(np);
> -	mutex_unlock(&of_aliases_mutex);
> +
> +	/* find root */
> +	np = of_find_node_by_path("/");
> +	if (np == NULL) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +	/* populate */
> +	rc = __of_populate(np);
> +	of_node_put(np);
>  
>  	/* Symlink in /proc as required by userspace ABI */
> -	if (of_allnodes)
> -		proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
> +	if (rc != 0)
> +		goto out;
>  
> -	return 0;
> +	proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
> +
> +out:
> +	mutex_unlock(&of_aliases_mutex);
> +
> +	return rc;
>  }
>  core_initcall(of_init);
>  
> @@ -1587,6 +1646,10 @@ static int of_property_notify(int action, struct device_node *np,
>  {
>  	struct of_prop_reconfig pr;
>  
> +	/* only call notifiers if the node is attached */
> +	if (!of_node_is_attached(np))
> +		return 0;
> +
>  	pr.dn = np;
>  	pr.prop = prop;
>  	return of_reconfig_notify(action, &pr);
> @@ -1626,11 +1689,8 @@ int of_add_property(struct device_node *np, struct property *prop)
>  	*next = prop;
>  	raw_spin_unlock_irqrestore(&devtree_lock, flags);
>  
> -	/* at early boot, bail hear and defer setup to of_init() */
> -	if (!of_kset)
> -		return 0;
> -
> -	__of_add_property(np, prop);
> +	if (of_node_is_attached(np))
> +		__of_add_property(np, prop);
>  
>  	return 0;
>  }
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d5d62cd..3ca75fb 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -327,7 +327,8 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
>  		if (!np->type)
>  			np->type = "<NULL>";
>  
> -		of_node_add(np);
> +		/* initialize node (do not add) */
> +		of_node_init(np);
>  	}
>  	while (tag == OF_DT_BEGIN_NODE || tag == OF_DT_NOP) {
>  		if (tag == OF_DT_NOP)
> diff --git a/include/linux/of.h b/include/linux/of.h
> index f285222..f13e773 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -76,6 +76,21 @@ struct of_phandle_args {
>  
>  extern int of_node_add(struct device_node *node);
>  
> +/* initialize a node */
> +extern void of_node_init(struct device_node *node);
> +
> +/* true when node is initialized */
> +static inline int of_node_is_initialized(struct device_node *node)
> +{
> +	return node && node->kobj.state_initialized;
> +}
> +
> +/* true when node is attached (i.e. present on sysfs) */
> +static inline int of_node_is_attached(struct device_node *node)
> +{
> +	return node && node->kobj.state_in_sysfs;
> +}
> +
>  #ifdef CONFIG_OF_DYNAMIC
>  extern struct device_node *of_node_get(struct device_node *node);
>  extern void of_node_put(struct device_node *node);
> -- 
> 1.7.12
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux