[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqLM7xO1WEjL+OOLansYUHFRZUWmSawAzsqsfH_F+pP8tA@mail.gmail.com>
Date: Mon, 19 Oct 2015 11:27:03 -0500
From: Rob Herring <robh+dt@...nel.org>
To: Mark Brown <broonie@...nel.org>
Cc: Alexander Holler <holler@...oftware.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Russell King <linux@....linux.org.uk>,
Grant Likely <grant.likely@...aro.org>,
Frank Rowand <frowand.list@...il.com>,
David Gibson <david@...son.dropbear.id.au>
Subject: Re: [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies
provided by the DT too
On Mon, Oct 19, 2015 at 7:37 AM, Mark Brown <broonie@...nel.org> 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@...oftware.de>
>> ---
>> 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@...r.kernel.org
>> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists