Re: [PATCH 1/3] of: Fix early OF builtup on kobj-ification

[Grant Likely]

On Tue, Dec 10, 2013 at 2:13 PM, Pantelis Antoniou
<panto@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> When booting platforms that do very early OF initialization before
> core_initcalls are performed of_init is called too late.
>
> This results in a hard-hard without getting a chance to output anything.
>
> Fixed by adding a flag that marks when init has been done, and
> performing the per-node init at that time.
>
> Signed-off-by: Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/of/base.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 734689b..a4f3dda 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -98,7 +98,7 @@ int __weak of_node_to_nid(struct device_node *np)
>   */
>  struct device_node *of_node_get(struct device_node *node)
>  {
> -       if (node)
> +       if (node && of_kset)
>                 kobject_get(&node->kobj);
>         return node;
>  }
> @@ -156,7 +156,7 @@ static void of_node_release(struct kobject *kobj)
>   */
>  void of_node_put(struct device_node *node)
>  {
> -       if (node)
> +       if (node && of_kset)
>                 kobject_put(&node->kobj);
>  }

As discussed on IRC, I don't think this is required.

>  EXPORT_SYMBOL(of_node_put);
> @@ -202,9 +202,16 @@ static const char *safe_name(struct kobject *kobj, const char *orig_name)
>  static int __of_add_property(struct device_node *np, struct property *pp)
>  {
>         int rc;
> +       bool secure;
> +
> +       /* note that we don't take a lock here */
> +
> +       /* fake the return while of_init is not yet called */
> +       if (!of_kset)
> +               return 0;

__of_add_property is called from three functions; __of_node_add(),
of_add_property() and of_modify_property(). __of_node_add is never
called on an unattached node. If the other two are getting called
early, then I think they should make the check. I'll modify my patch
to reflect that.

>
>         /* Important: Don't leak passwords */
> -       bool secure = strncmp(pp->name, "security-", 9) == 0;
> +       secure = strncmp(pp->name, "security-", 9) == 0;
>
>         pp->attr.attr.name = safe_name(&np->kobj, pp->name);
>         pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
> @@ -222,6 +229,12 @@ static int __of_node_add(struct device_node *np)
>         struct property *pp;
>         int rc;
>
> +       /* note that we don't take a lock here */
> +
> +       /* fake the return while of_init is not yet called */
> +       if (!of_kset)
> +               return 0;
> +

__of_node_add() never gets called before of_kset is initialized.

>         np->kobj.kset = of_kset;
>         if (!np->parent) {
>                 /* Nodes without parents are new top level trees */
> @@ -245,11 +258,14 @@ 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);
> +
> +       /* fake the return while of_init is not yet called */
>         mutex_lock(&of_aliases_mutex);
> +       kobject_init(&np->kobj, &of_node_ktype);

Why the kobject_init() move?

>         if (of_kset)
>                 rc = __of_node_add(np);
>         mutex_unlock(&of_aliases_mutex);
> +
>         return rc;
>  }
>
> @@ -275,14 +291,16 @@ static int __init of_init(void)
>
>         /* 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);
>
>         /* Symlink in /proc as required by userspace ABI */
>         if (of_allnodes)
>                 proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
>
> +       mutex_unlock(&of_aliases_mutex);
> +
>         return 0;
>  }
>  core_initcall(of_init);
> --
> 1.7.12
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/