Re: [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

[Rob Herring]

On Mon, Oct 19, 2015 at 7:37 AM, Mark Brown <broonie@xxxxxxxxxx> wrote:
> On Sat, Oct 17, 2015 at 07:14:16PM +0200, Alexander Holler wrote:
>> This patch adds dependencies provided by the hardware description in
>> the used DT. This avoids the use of the deferred probe mechanism
>> on most (if not all) DT based kernels.
>>
>> Drawback is that the binary DT blob has to be enhanced with type
>> information for phandles (which are used as dependencies) which
>> needs a modified dtc.
>
> You probably want to loop the DT and DTC maintainers in on this - adding
> Frank, Rob and David and leaving context for their reference.  It would
> probably help if you could explicitly say why the DTB needs to be
> annotated and why this annotiation is best done via a DTC modification
> (rather than doing something like add new properties, or just guessing
> that any phandle reference is a dependency).

My (and Grant's) prior responses can be found here[1]. I don't have
any context whether anything has changed since the previous version,
but it seems similar. So I have nothing new to add. I think I've made
it clear what solution I do like[2].

There is a need to annotate DTB's with type information, but that
needs to be looked at in context of all types, not just one specific
type.

Rob

[1] https://lkml.org/lkml/2014/5/14/839
[2] https://lkml.org/lkml/2015/10/15/337

>
>>
>> Signed-off-by: Alexander Holler <holler@xxxxxxxxxxxxx>
>> ---
>>  drivers/of/base.c   | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/of.h  |   3 ++
>>  init/dependencies.c |   4 ++
>>  3 files changed, 121 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 8b5a187..423ddff 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -12,6 +12,8 @@
>>   *  Reconsolidated from arch/x/kernel/prom.c by Stephen Rothwell and
>>   *  Grant Likely.
>>   *
>> + *  The dependency related stuff was done by Alexander Holler.
>> + *
>>   *      This program is free software; you can redistribute it and/or
>>   *      modify it under the terms of the GNU General Public License
>>   *      as published by the Free Software Foundation; either version
>> @@ -2308,3 +2310,115 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node)
>>       return of_get_next_parent(np);
>>  }
>>  EXPORT_SYMBOL(of_graph_get_remote_port);
>> +
>> +#ifdef CONFIG_DEPENDENCIES
>> +
>> +static const struct _annotated_initcall * __init find_matching_driver(
>> +     const struct _annotated_initcall *from, const struct device_node *node)
>> +{
>> +     while (++from != __annotated_initcall_end)
>> +             if (from->driver &&
>> +                             __of_match_node(from->driver->of_match_table,
>> +                                     node))
>> +                     return from;
>> +     return NULL;
>> +}
>> +
>> +static int __init add_dep_list(const struct device_node *node, unsigned drvid)
>> +{
>> +     const __be32 *list, *list_end;
>> +     uint32_t ph;
>> +     int size = 0;
>> +     int rc = 0;
>> +     const struct device_node *dep;
>> +     const struct _annotated_initcall *ac;
>> +
>> +     list = __of_get_property(node, "dependencies", &size);
>> +     if (!list || !size || size % sizeof(*list))
>> +             return 0;
>> +     list_end = list + size / sizeof(*list);
>> +     while (list < list_end) {
>> +             ph = be32_to_cpup(list++);
>> +             if (unlikely(!ph)) {
>> +                     /* Should never happen */
>> +                     if (node->name)
>> +                             pr_warn("phandle == 0 for %s\n", node->name);
>> +                     continue;
>> +             }
>> +             dep = of_find_node_by_phandle(ph);
>> +             if (unlikely(!dep)) {
>> +                     pr_err("No DT node for dependency with phandle 0x%x found\n",
>> +                             ph);
>> +                     continue;
>> +             }
>> +             ac = __annotated_initcall_start - 1;
>> +             while ((ac = find_matching_driver(ac, dep))) {
>> +                     if (!ac->id)
>> +                             continue;
>> +                     rc = add_initcall_dependency(drvid, ac->id);
>> +                     if (rc)
>> +                             return rc;
>> +             }
>> +     }
>> +
>> +     return rc;
>> +}
>> +
>> +static int __init add_deps(unsigned parent, const struct device_node *node)
>> +{
>> +     struct device_node *child;
>> +     const struct _annotated_initcall *ac;
>> +     int rc = 0;
>> +     bool found_one_driver = false;
>> +
>> +     if (!__of_device_is_available(node))
>> +             return 0;
>> +     if (__of_get_property(node, "compatible", NULL)) {
>> +             ac = __annotated_initcall_start - 1;
>> +             while ((ac = find_matching_driver(ac, node))) {
>> +                     if (!ac->id)
>> +                             continue;
>> +                     found_one_driver = true;
>> +                     rc = add_initcall_dependency(ac->id, parent);
>> +                     if (unlikely(rc))
>> +                             return rc;
>> +                     rc = add_dep_list(node, ac->id);
>> +                     if (unlikely(rc))
>> +                             return rc;
>> +                     for_each_child_of_node(node, child) {
>> +                             rc = add_deps(ac->id, child);
>> +                             if (unlikely(rc))
>> +                                     return rc;
>> +                     }
>> +             }
>> +             if (found_one_driver)
>> +                     return rc;
>> +     }
>> +     for_each_child_of_node(node, child) {
>> +             rc = add_deps(parent, child);
>> +             if (unlikely(rc))
>> +                     break;
>> +     }
>> +
>> +     return rc;
>> +}
>> +
>> +int __init of_add_dependencies(void)
>> +{
>> +     int rc = 0;
>> +     struct device_node *child;
>> +     struct device_node *root =  of_find_node_by_path("/");
>> +
>> +     if (unlikely(!root))
>> +             return -EINVAL;
>> +
>> +     for_each_child_of_node(root, child) {
>> +             rc = add_deps(0, child);
>> +             if (unlikely(rc))
>> +                     break;
>> +     }
>> +     of_node_put(root);
>> +
>> +     return rc;
>> +}
>> +#endif /* CONFIG_DEPENDENCIES */
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index edc068d..e3b65c8 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -1101,4 +1101,7 @@ static inline int of_overlay_destroy_all(void)
>>
>>  #endif
>>
>> +/* Inserts dependencies for drivers referenced in the loaded DT. */
>> +int __init of_add_dependencies(void);
>> +
>>  #endif /* _LINUX_OF_H */
>> diff --git a/init/dependencies.c b/init/dependencies.c
>> index c47817c..b484f67 100644
>> --- a/init/dependencies.c
>> +++ b/init/dependencies.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/device.h>
>>  #include <linux/mod_devicetable.h>
>>  #include <linux/init.h>
>> +#include <linux/of.h>
>>
>>  #if defined(CONFIG_DEPENDENCIES_PRINT_INIT_ORDER) \
>>       || defined(CONFIG_DEPENDENCIES_PRINT_CALLS)
>> @@ -335,6 +336,9 @@ static int __init build_order(void)
>>
>>       build_inventory();
>>       add_dependencies();
>> +#ifdef CONFIG_OF
>> +     of_add_dependencies();
>> +#endif
>>       if (topological_sort())
>>               return -EINVAL; /* cycle found */
>>       pr_debug("init: vertices: %u edges %u count %u\n",
>> --
>> 2.1.0
>>
>> --
>> 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/
>>
--
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/