On Friday, July 10, 2015 03:14:38 PM Tomeu Vizoso wrote: > On 2 July 2015 at 02:02, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > > 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@xxxxxxxxxxxxx> > > > > I'd like to see a description of the new API in English in the changelog. > > Agreed. > > >> --- > >> > >> 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. > > Ok. > > >> +struct dependency_parser { > >> + struct list_head parser; > > > > I'd rather call this "list_node" or something like that. > > Ok. > > >> + 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? > > Oops. > > >> +} > >> +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. > > My impression from reading the existing usage of fwnode in gpiolib and > the examples in the link below was that we were going to share the > bindings between DT and ACPI. Doesn't that extend to the meaning of > the compatible property? > > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf Not entirely. An ACPI device object without the "compatible" property may still represent a valid device (and usually does), so it is not enough to check whether or not the "compatible" property is present for that in general. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html