On Wednesday, July 01, 2015 11:40:57 AM Tomeu Vizoso wrote:
Adds API that allows callers to find out what other firmware nodes a node depends on.
Implementors of bindings documentation can register callbacks that return the dependencies of a node.
Dependency information can be used to change the order in which devices are probed, or to print a warning when a device node is going to be probed without all its dependencies fulfilled.
Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com
I'd like to see a description of the new API in English in the changelog.
Changes in v2:
- Allow bindings implementations register a function instead of using class callbacks, as not only subsystems implement firmware bindings.
drivers/base/property.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/fwnode.h | 5 +++ include/linux/property.h | 12 +++++++ 3 files changed, 108 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c index 8ead1ba..9d38ede 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -19,7 +19,13 @@ #include <linux/platform_device.h> #include <linux/property.h>
Please add a comment describing this structure. In particular, what it is supposed to be used for and how it is supposed to be used.
+struct dependency_parser {
- struct list_head parser;
I'd rather call this "list_node" or something like that.
- void (*func)(struct fwnode_handle *fwnode, struct list_head *deps);
+};
static bool fwnode_match_enable = false; +static LIST_HEAD(dependency_parsers);
/**
- device_add_property_set - Add a collection of properties to a device object.
@@ -553,6 +559,27 @@ bool device_dma_is_coherent(struct device *dev) EXPORT_SYMBOL_GPL(device_dma_is_coherent);
/**
- fwnode_add_dependency - add firmware node to the passed dependency list
- @fwnode: Firmware node to add to dependency list
- @list: Dependency list to add the fwnode to
- */
+void fwnode_add_dependency(struct fwnode_handle *fwnode,
struct list_head *list)+{
- struct fwnode_dependency *dep;
- dep = kzalloc(sizeof(*dep), GFP_KERNEL);
- if (!dep)
return;- INIT_LIST_HEAD(&dep->dependency);
- dep->fwnode = fwnode;
- list_add_tail(&dep->dependency, list);
+} +EXPORT_SYMBOL_GPL(fwnode_add_dependency);
+/**
- fwnode_get_parent - return the parent node of a device node
- @fwnode: Device node to find the parent node of
*/ @@ -600,6 +627,70 @@ bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible) EXPORT_SYMBOL_GPL(fwnode_is_compatible);
/**
- fwnode_add_dependency_parser - register dependency parser
- @func: Function that will be called to find out dependencies of a node
- Registers a callback that will be called when collecting the dependencies
- of a firmware node. The callback should inspect the properties of the node
- and call fwnode_add_dependency() for each dependency it recognizes, from
- the bindings documentation.
- */
+void fwnode_add_dependency_parser(
- void (*func)(struct fwnode_handle *fwnode, struct list_head *deps))
+{
- struct dependency_parser *parser;
- parser = kzalloc(sizeof(*parser), GFP_KERNEL);
- if (!parser)
return;- INIT_LIST_HEAD(&parser->parser);
- parser->func = func;
- list_add_tail(&parser->parser, &dependency_parsers);
We're modifying a global list here. Any locking needed? RCU? Whatever?
+} +EXPORT_SYMBOL_GPL(fwnode_add_dependency_parser);
+/**
- fwnode_remove_dependency_parser - unregister dependency parser
- @func: Function that was to be called to find out dependencies of a node
- */
+void fwnode_remove_dependency_parser(
- void (*func)(struct fwnode_handle *fwnode, struct list_head *deps))
+{
- struct dependency_parser *parser, *tmp;
- list_for_each_entry_safe(parser, tmp, &dependency_parsers, parser) {
if (parser->func == func) {list_del(&parser->parser);kfree(parser);return;}- }
+} +EXPORT_SYMBOL_GPL(fwnode_remove_dependency_parser);
+/**
- fwnode_get_dependencies - find out what dependencies a firmware node has
- @fwnode: firmware node to find its dependencies
- @deps: list of struct fwnode_dependency in which dependencies will be placed
- */
+void fwnode_get_dependencies(struct fwnode_handle *fwnode,
struct list_head *deps)+{
- struct dependency_parser *parser;
- struct fwnode_handle *child;
- list_for_each_entry(parser, &dependency_parsers, parser)
parser->func(fwnode, deps);- /* Some device nodes will have dependencies in non-device sub-nodes */
- fwnode_for_each_child_node(fwnode, child)
if (!fwnode_property_present(child, "compatible"))
This is a blatant OF-ism. We need to think about a generic way to express that.
fwnode_get_dependencies(child, deps);+}
+/**
- fwnode_driver_match_device - Tell if a driver matches a device.
- @drv: the device_driver structure to test
- @dev: the device structure to match against
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 0408545..68ab558 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -24,4 +24,9 @@ struct fwnode_handle { struct fwnode_handle *secondary; };
+struct fwnode_dependency {
- struct fwnode_handle *fwnode;
- struct list_head dependency;
So this is a node in the list of dependencies, right?
I'd call that field "list_node", then.
+};
And fwnode is a firmware node that the owner of the list depends on, right?
#endif diff --git a/include/linux/property.h b/include/linux/property.h index 4e453c4..b8b86ea 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -86,6 +86,18 @@ bool fwnode_is_compatible(struct fwnode_handle *fwnode, const char *compatible); bool fwnode_driver_match_device(struct device *dev, const struct device_driver *drv);
+void fwnode_add_dependency(struct fwnode_handle *fwnode,
struct list_head *list);+void fwnode_add_dependency_parser(
- void (*func)(struct fwnode_handle *fwnode, struct list_head *deps));
+void fwnode_remove_dependency_parser(
- void (*func)(struct fwnode_handle *fwnode, struct list_head *deps));
+void fwnode_get_dependencies(struct fwnode_handle *fwnode,
struct list_head *list);unsigned int device_get_child_node_count(struct device *dev);
static inline bool device_property_read_bool(struct device *dev,