OF: base: import of_find_matching_node_and_match from Linux OF API
Hi,
on master/next branch,
of_find_matching_node_and_match macro can make infinite loop.
This is because, of_tree_for_each_node traverse all nodes each time. But a callee of of_find_matching_node_and_match,
such as for_each_matching_node, calls it by changing "from" arg.
So if "matches" have two or more nodes, previously matched node can be re-matched.
Below change can fix the issue.
Or is it better to port node->allnext from linux kernel?
--- drivers/of/base.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/of/base.c +10 -4
@@ -36,6 +36,9 @@#define of_tree_for_each_node (node, root) \ list_for_each_entry(node, &root->list, list) +#define of_get_next_node (np) \ + list_first_entry(&np->list, typeof(*np), list) +/** * struct alias_prop - Alias property in 'aliases' node * @link: List node to link the structure in aliases_lookup list
@@ -444,17 +447,20 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from,if (match) *match =NULL ; -if (!from) - from = root_node; + np = from ?of_get_next_node (from) : root_node; -of_tree_for_each_node (np, from) { +if (from !=NULL && np == root_node) +return NULL ; + +do {const struct of_device_id *m =of_match_node (matches, np);if (m) {if (match) *match = m;return np; } - } + np =of_get_next_node (np); + }while (np != root_node);return NULL ; }
Signature · 1 line
1.8.1.2
Quoted 109 lines
This imports
of_find_matching_node_and_matchand corresponding helpers from Linux OF API.Signed-off-by Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx>---
Cc: barebox@xxxxxxxxxxxxxxxxxxx--- drivers/of/base.c | 37 +++++++++++++++++++++++++++++++++++++ include/of.h | 24 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 0 deletions(-)drivers/of/base.c +37
-0@@ -389,6 +389,43 @@ const struct of_device_id *of_match_node(const struct of_device_id *matches,return NULL ; } +/** +* of_find_matching_node_and_match - Find a node based on an of_device_id +* match table. +* @from: The node to start searching from or NULL, the node +* you pass will not be searched, only the next one +* will; typically, you pass what the previous call +* returned. +* @matches: array of of device match structures to search in +* @match Updated to point at the matches entry which matched +* +* Returns a pointer to the node found or NULL. +*/ +struct device_node *of_find_matching_node_and_match (struct device_node *from, +const struct of_device_id *matches, +const struct of_device_id **match) +{ +struct device_node *np; + +if (match) + *match =NULL ; + +if (!from) + from = root_node; + +of_tree_for_each_node (np, from) { +const struct of_device_id *m =of_match_node (matches, np); +if (m) { +if (match) + *match = m; +return np; + } + } + +return NULL ; +} +EXPORT_SYMBOL (of_find_matching_node_and_match); +int of_match (struct device_d *dev,struct driver_d *drv) {const struct of_device_id *id;include/of.h +24
-0@@ -187,6 +187,10 @@ extern struct device_node *of_find_node_by_name(struct device_node *from,extern struct device_node *of_find_node_by_path (const char *path);extern struct device_node *of_find_compatible_node (struct device_node *from,const char *type,const char *compat); +extern struct device_node *of_find_matching_node_and_match ( +struct device_node *from, +const struct of_device_id *matches, +const struct of_device_id **match);extern int of_device_is_available (const struct device_node *device);extern void of_alias_scan (void );@@ -259,6 +263,14 @@ static inline struct device_node *of_find_compatible_node(return NULL ; } +static inline struct device_node *of_find_matching_node_and_match ( +struct device_node *from, +const struct of_device_id *matches, +const struct of_device_id **match) +{ +return NULL ; +} +static inline int of_device_is_available (const struct device_node *device) {return 0 ;@@ -285,5 +297,17 @@ static inline const char *of_alias_get(struct device_node *np)#define for_each_compatible_node (dn, type, compatible) \ for (dn = of_find_compatible_node(NULL, type, compatible); dn; \ dn = of_find_compatible_node(dn, type, compatible)) +static inline struct device_node *of_find_matching_node ( +struct device_node *from, +const struct of_device_id *matches) +{ +return of_find_matching_node_and_match (from, matches,NULL ); +} +#define for_each_matching_node (dn, matches) \ + for (dn = of_find_matching_node(NULL, matches); dn; \ + dn = of_find_matching_node(dn, matches)) +#define for_each_matching_node_and_match (dn, matches, match) \ + for (dn = of_find_matching_node_and_match(NULL, matches, match); \ + dn; dn = of_find_matching_node_and_match(dn, matches, match))#endif /* __OF_H */ Signature · 1 line
1.7.2.5
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Quoted 3 lines · NISHIMOTO Hiroki
on master/next branch,
of_find_matching_node_and_matchmacro can make infinite loop.
Hiroki,
I confirmed that infinite loop but there are more functions that suffer from this infinite loop.
Quoted 6 lines
This is because,
of_tree_for_each_nodetraverse all nodes each time. But a callee ofof_find_matching_node_and_match,
such asfor_each_matching_node, calls it by changing "from" arg.
So if "matches" have two or more nodes, previously matched node can be re-matched.Below change can fix the issue.
@Sascha: please confirm the below. It is what I understand from your last comments about OF API improvements.
Actually, root_node is only the list head for the currently active device tree. There can be other device trees you want to use the of_for_each_.. on, so we need to find another way to check for the list head of the tree the "from" node is in.
From what I can see from linux linked lists API, there is no safe way to determine the start of a linked list without knowing what has been the first list node in the first place.
Sascha knows barebox struct device_node lists for sure, maybe he can point us to the best way to solve this?
Sebastian
Quoted 36 lines
Or is it better to port
node->allnextfrom linux kernel?Signed-off-by NISHIMOTO Hiroki<[email protected]>--- drivers/of/base.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)drivers/of/base.c +8
-3@@ -36,6 +36,9 @@#define of_tree_for_each_node (node, root) \ list_for_each_entry(node,&root->list, list) +#define of_get_next_node (np) \ + list_first_entry(&np->list, typeof(*np), list) +/** * struct alias_prop - Alias property in 'aliases' node * @link: List node to link the structure in aliases_lookup list @@ -444,17 +447,20 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from,if (match) *match =NULL ; -if (!from) - from = root_node; + np = from ?of_get_next_node (from) : root_node; -of_tree_for_each_node (np, from) { +if (from !=NULL && np == root_node) +return NULL ; + + do { const struct of_device_id *m = of_match_node(matches, np); if (m) {
drivers/of/base.c struct device_node *of_find_matching_node_and_match(struct device_node *from, · line 466
Quoted 10 lines
drivers/of/base.c +2
-1if (match) *match = m;return np; } - } + np =of_get_next_node (np); + }while (np != root_node);return NULL ; }
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox