| Message ID | 20131121.191720.1487772262083864095.hdoyu@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/21/2013 10:17 AM, Hiroshi Doyu wrote: > Iterating over a property containing a list of phandles with arguments > is a common operation for device drivers. This patch adds a new > of_property_for_each_phandle_with_args() macro to make the iteration > simpler. > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> > --- > v6+: > Use the description, which Grant Likely proposed, to be full enough > that a future reader can figure out why a patch was written. > http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007062.html This new version only addresses one of the concerns that Grant had, namely the commit message. > diff --git a/include/linux/of.h b/include/linux/of.h > +#define of_property_for_each_phandle_with_args(np, list, cells, i, args) \ > + for (i = 0; !of_parse_phandle_with_args(np, list, cells, i, args); i++) > + Grant also wanted the actual implementation fixed so that it wasn't so inefficient. What this current patch does is basically: for every entry in the property: for every entry in the property before the current index: parse the phandle+specifier That's roughly O(n^2). (n is # entries in the property) Instead, what should happen is: for every entry in the property: parse the phandle+specifier yield the result That's roughly O(n). In other words, an implementation more along the lines of include/linux/of.h's: #define of_property_for_each_u32(np, propname, prop, p, u) \ for (prop = of_find_property(np, propname, NULL), \ p = of_prop_next_u32(prop, NULL, &u); \ p; \ p = of_prop_next_u32(prop, p, &u)) ... so you'd need functions like of_prop_first_specifier() and of_prop_next_specifier(), and perhaps some associated set of state variables, perhaps with all the state wrapped into a single struct for simplicity.
On Thu, 21 Nov 2013 18:17:20 +0100, Hiroshi Doyu <hdoyu@nvidia.com> wrote: > Iterating over a property containing a list of phandles with arguments > is a common operation for device drivers. This patch adds a new > of_property_for_each_phandle_with_args() macro to make the iteration > simpler. > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> Acked-by: Grant Likely <grant.likely@linaro.org> This patch can be merged with the rest of the series. g. > --- > v6+: > Use the description, which Grant Likely proposed, to be full enough > that a future reader can figure out why a patch was written. > http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007062.html > > v5: > New patch for v5. > --- > include/linux/of.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/linux/of.h b/include/linux/of.h > index 276c546..131fef5 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -613,6 +613,9 @@ static inline int of_property_read_u32(const struct device_node *np, > s; \ > s = of_prop_next_string(prop, s)) > > +#define of_property_for_each_phandle_with_args(np, list, cells, i, args) \ > + for (i = 0; !of_parse_phandle_with_args(np, list, cells, i, args); i++) > + > #if defined(CONFIG_PROC_FS) && defined(CONFIG_PROC_DEVICETREE) > extern void proc_device_tree_add_node(struct device_node *, struct proc_dir_entry *); > extern void proc_device_tree_add_prop(struct proc_dir_entry *pde, struct property *prop); > -- > 1.8.1.5
On Thu, 21 Nov 2013 11:57:00 -0700, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 11/21/2013 10:17 AM, Hiroshi Doyu wrote: > > Iterating over a property containing a list of phandles with arguments > > is a common operation for device drivers. This patch adds a new > > of_property_for_each_phandle_with_args() macro to make the iteration > > simpler. > > > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> > > --- > > v6+: > > Use the description, which Grant Likely proposed, to be full enough > > that a future reader can figure out why a patch was written. > > http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007062.html > > This new version only addresses one of the concerns that Grant had, > namely the commit message. > > > diff --git a/include/linux/of.h b/include/linux/of.h > > > +#define of_property_for_each_phandle_with_args(np, list, cells, i, args) \ > > + for (i = 0; !of_parse_phandle_with_args(np, list, cells, i, args); i++) > > + > > Grant also wanted the actual implementation fixed so that it wasn't so > inefficient. > > What this current patch does is basically: > > for every entry in the property: > for every entry in the property before the current index: > parse the phandle+specifier > > That's roughly O(n^2). (n is # entries in the property) > > Instead, what should happen is: > > for every entry in the property: > parse the phandle+specifier > yield the result > > That's roughly O(n). > > In other words, an implementation more along the lines of > include/linux/of.h's: > > #define of_property_for_each_u32(np, propname, prop, p, u) \ > for (prop = of_find_property(np, propname, NULL), \ > p = of_prop_next_u32(prop, NULL, &u); \ > p; \ > p = of_prop_next_u32(prop, p, &u)) > > ... so you'd need functions like of_prop_first_specifier() and > of_prop_next_specifier(), and perhaps some associated set of state > variables, perhaps with all the state wrapped into a single struct for > simplicity. That's right, I forgot I said that. Yes please fix the implementation. g.
diff --git a/include/linux/of.h b/include/linux/of.h index 276c546..131fef5 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -613,6 +613,9 @@ static inline int of_property_read_u32(const struct device_node *np, s; \ s = of_prop_next_string(prop, s)) +#define of_property_for_each_phandle_with_args(np, list, cells, i, args) \ + for (i = 0; !of_parse_phandle_with_args(np, list, cells, i, args); i++) + #if defined(CONFIG_PROC_FS) && defined(CONFIG_PROC_DEVICETREE) extern void proc_device_tree_add_node(struct device_node *, struct proc_dir_entry *); extern void proc_device_tree_add_prop(struct proc_dir_entry *pde, struct property *prop);
Iterating over a property containing a list of phandles with arguments is a common operation for device drivers. This patch adds a new of_property_for_each_phandle_with_args() macro to make the iteration simpler. Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> --- v6+: Use the description, which Grant Likely proposed, to be full enough that a future reader can figure out why a patch was written. http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007062.html v5: New patch for v5. --- include/linux/of.h | 3 +++ 1 file changed, 3 insertions(+)