From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752304AbaCMMiz (ORCPT ); Thu, 13 Mar 2014 08:38:55 -0400 Received: from mail-ea0-f176.google.com ([209.85.215.176]:47680 "EHLO mail-ea0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751194AbaCMMiw (ORCPT ); Thu, 13 Mar 2014 08:38:52 -0400 From: Grant Likely Subject: Re: [PATCH] OF: kobj node lifecycle fixes To: Pantelis Antoniou Cc: Rob Herring , Stephen Warren , Matt Porter , Koen Kooi , Alison Chaiken , Dinh Nguyen , Jan Lubbe , Alexander Sverdlin , Michael Stickel , Guenter Roeck , Dirk Behme , Alan Tull , Sascha Hauer , Michael Bohan , Ionut Nicu , Michal Simek , Matt Ranostay , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Pantelis Antoniou In-Reply-To: <1386958139-6130-1-git-send-email-panto@antoniou-consulting.com> References: <1386958139-6130-1-git-send-email-panto@antoniou-consulting.com > Date: Thu, 13 Mar 2014 12:38:36 +0000 Message-Id: <20140313123836.CF666C40E11@trevor.secretlab.ca> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 13 Dec 2013 20:08:59 +0200, Pantelis Antoniou 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 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 = ""; > > - 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Subject: Re: [PATCH] OF: kobj node lifecycle fixes Date: Thu, 13 Mar 2014 12:38:36 +0000 Message-ID: <20140313123836.CF666C40E11@trevor.secretlab.ca> References: <1386958139-6130-1-git-send-email-panto@antoniou-consulting.com > Return-path: In-Reply-To: <1386958139-6130-1-git-send-email-panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: Rob Herring , Stephen Warren , Matt Porter , Koen Kooi , Alison Chaiken , Dinh Nguyen , Jan Lubbe , Alexander Sverdlin , Michael Stickel , Guenter Roeck , Dirk Behme , Alan Tull , Sascha Hauer , Michael Bohan , Ionut Nicu , Michal Simek , Matt Ranostay , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pantelis Antoniou List-Id: devicetree@vger.kernel.org On Fri, 13 Dec 2013 20:08:59 +0200, Pantelis Antoniou 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 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 = ""; > > - 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html