| Message ID | 1407452515-2390-2-git-send-email-ttynkkynen@nvidia.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Fri, Aug 08, 2014 at 02:01:53AM +0300, Tuomas Tynkkynen wrote: > Add of_match_machine function to test the device tree root for an > of_match array. This can be useful when testing SoC versions at runtime, > for example. > > Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com> > --- > drivers/of/base.c | 21 +++++++++++++++++++++ > include/linux/of.h | 3 +++ > 2 files changed, 24 insertions(+) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index d8574ad..37798ea 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -977,6 +977,27 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from, > EXPORT_SYMBOL(of_find_matching_node_and_match); > > /** > + * of_match_machine - Tell if root of device tree has a matching of_match struct > + * @matches: array of of device match structures to search in > + * > + * Returns the result of of_match_node for the root node. > + */ I was going to say that this kerneldoc is weirdly formatted, but when I looked at the other kerneldoc comments in this file it seems that they use similarly weird formatting... > +const struct of_device_id *of_match_machine(const struct of_device_id *matches) > +{ > + const struct of_device_id *match; > + struct device_node *root; > + > + root = of_find_node_by_path("/"); > + if (!root) > + return NULL; > + > + match = of_match_node(matches, root); > + of_node_put(root); > + return match; > +} > +EXPORT_SYMBOL(of_match_machine); I wonder if of_find_node_by_path("/") is somewhat overkill here. Perhaps simply of_node_get(of_allnodes) would be more appropriate here since the function is implemented in the core? Thierry
On 08/08/14 12:41, Thierry Reding wrote: > >> +const struct of_device_id *of_match_machine(const struct of_device_id *matches) >> +{ >> + const struct of_device_id *match; >> + struct device_node *root; >> + >> + root = of_find_node_by_path("/"); >> + if (!root) >> + return NULL; >> + >> + match = of_match_node(matches, root); >> + of_node_put(root); >> + return match; >> +} >> +EXPORT_SYMBOL(of_match_machine); > > I wonder if of_find_node_by_path("/") is somewhat overkill here. Perhaps > simply of_node_get(of_allnodes) would be more appropriate here since the > function is implemented in the core? of_machine_is_compatible() uses of_find_node_by_path("/") as well, of_allnodes seems to be only used when during iterating. So I'd prefer to have them consistent.
On Fri, Aug 8, 2014 at 8:23 AM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: > > > On 08/08/14 12:41, Thierry Reding wrote: >> >>> +const struct of_device_id *of_match_machine(const struct of_device_id *matches) >>> +{ >>> + const struct of_device_id *match; >>> + struct device_node *root; >>> + >>> + root = of_find_node_by_path("/"); >>> + if (!root) >>> + return NULL; >>> + >>> + match = of_match_node(matches, root); >>> + of_node_put(root); >>> + return match; >>> +} >>> +EXPORT_SYMBOL(of_match_machine); >> >> I wonder if of_find_node_by_path("/") is somewhat overkill here. Perhaps >> simply of_node_get(of_allnodes) would be more appropriate here since the >> function is implemented in the core? > > of_machine_is_compatible() uses of_find_node_by_path("/") as well, of_allnodes > seems to be only used when during iterating. So I'd prefer to have them > consistent. Agreed. For the series: Acked-by: Rob Herring <robh@kernel.org> Rob -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 8 Aug 2014 02:01:53 +0300, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: > Add of_match_machine function to test the device tree root for an > of_match array. This can be useful when testing SoC versions at runtime, > for example. > > Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com> > --- > drivers/of/base.c | 21 +++++++++++++++++++++ > include/linux/of.h | 3 +++ > 2 files changed, 24 insertions(+) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index d8574ad..37798ea 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -977,6 +977,27 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from, > EXPORT_SYMBOL(of_find_matching_node_and_match); > > /** > + * of_match_machine - Tell if root of device tree has a matching of_match struct > + * @matches: array of of device match structures to search in > + * > + * Returns the result of of_match_node for the root node. > + */ > +const struct of_device_id *of_match_machine(const struct of_device_id *matches) > +{ > + const struct of_device_id *match; > + struct device_node *root; > + > + root = of_find_node_by_path("/"); > + if (!root) > + return NULL; > + > + match = of_match_node(matches, root); > + of_node_put(root); > + return match; > +} > +EXPORT_SYMBOL(of_match_machine); Too wordy... return of_match_node(matches, of_allnodes); :-) It could be a static inline, but I don't think it's even worth having a helper. The callers could just open code the above. g. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 8 Aug 2014 14:01:57 -0500, Rob Herring <robherring2@gmail.com> wrote: > On Fri, Aug 8, 2014 at 8:23 AM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: > > > > > > On 08/08/14 12:41, Thierry Reding wrote: > >> > >>> +const struct of_device_id *of_match_machine(const struct of_device_id *matches) > >>> +{ > >>> + const struct of_device_id *match; > >>> + struct device_node *root; > >>> + > >>> + root = of_find_node_by_path("/"); > >>> + if (!root) > >>> + return NULL; > >>> + > >>> + match = of_match_node(matches, root); > >>> + of_node_put(root); > >>> + return match; > >>> +} > >>> +EXPORT_SYMBOL(of_match_machine); > >> > >> I wonder if of_find_node_by_path("/") is somewhat overkill here. Perhaps > >> simply of_node_get(of_allnodes) would be more appropriate here since the > >> function is implemented in the core? > > > > of_machine_is_compatible() uses of_find_node_by_path("/") as well, of_allnodes > > seems to be only used when during iterating. So I'd prefer to have them > > consistent. > > Agreed. Disagreed. of_machine_is_compatible should be simplified. g. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17/08/14 18:28, Grant Likely wrote: > On Fri, 8 Aug 2014 02:01:53 +0300, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: [...] >> +EXPORT_SYMBOL(of_match_machine); > > Too wordy... > > return of_match_node(matches, of_allnodes); > > :-) > > It could be a static inline, but I don't think it's even worth having a > helper. The callers could just open code the above. > Do you mean all the drivers should be referring to of_allnodes directly? I see that it's indeed exported, but to me that sounds like relying too much on an implementation detail. In fact, Documentation/devicetree/todo.txt even seems to have a TODO entry for its removal ("Remove of_allnodes list and iterate using list of child nodes alone").
diff --git a/drivers/of/base.c b/drivers/of/base.c index d8574ad..37798ea 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -977,6 +977,27 @@ struct device_node *of_find_matching_node_and_match(struct device_node *from, EXPORT_SYMBOL(of_find_matching_node_and_match); /** + * of_match_machine - Tell if root of device tree has a matching of_match struct + * @matches: array of of device match structures to search in + * + * Returns the result of of_match_node for the root node. + */ +const struct of_device_id *of_match_machine(const struct of_device_id *matches) +{ + const struct of_device_id *match; + struct device_node *root; + + root = of_find_node_by_path("/"); + if (!root) + return NULL; + + match = of_match_node(matches, root); + of_node_put(root); + return match; +} +EXPORT_SYMBOL(of_match_machine); + +/** * of_modalias_node - Lookup appropriate modalias for a device node * @node: pointer to a device tree node * @modalias: Pointer to buffer that modalias value will be copied into diff --git a/include/linux/of.h b/include/linux/of.h index 6c4363b..b7a8817 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -289,6 +289,8 @@ extern int of_n_addr_cells(struct device_node *np); extern int of_n_size_cells(struct device_node *np); extern const struct of_device_id *of_match_node( const struct of_device_id *matches, const struct device_node *node); +extern const struct of_device_id *of_match_machine( + const struct of_device_id *matches); extern int of_modalias_node(struct device_node *node, char *modalias, int len); extern void of_print_phandle_args(const char *msg, const struct of_phandle_args *args); extern struct device_node *of_parse_phandle(const struct device_node *np, @@ -584,6 +586,7 @@ static inline const char *of_prop_next_string(struct property *prop, #define of_match_ptr(_ptr) NULL #define of_match_node(_matches, _node) NULL +#define of_match_machine(_matches) NULL #endif /* CONFIG_OF */ #if defined(CONFIG_OF) && defined(CONFIG_NUMA)
Add of_match_machine function to test the device tree root for an of_match array. This can be useful when testing SoC versions at runtime, for example. Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com> --- drivers/of/base.c | 21 +++++++++++++++++++++ include/linux/of.h | 3 +++ 2 files changed, 24 insertions(+)