On Tue, Mar 15, 2011 at 01:54:05AM -0600, Grant Likely wrote: > On Mon, Mar 14, 2011 at 09:18:42PM +0800, Shawn Guo wrote: > > With the platform clock support, the 'struct clk' should have been > > associated with device_node->data. So the use of function > > __of_clk_get_from_provider can be eliminated. > > > > Signed-off-by: Shawn Guo <shawn....@linaro.org> > > --- > > drivers/of/clock.c | 23 ++--------------------- > > 1 files changed, 2 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/of/clock.c b/drivers/of/clock.c > > index 7b5ea67..f124d0a 100644 > > --- a/drivers/of/clock.c > > +++ b/drivers/of/clock.c > > @@ -71,24 +71,6 @@ void of_clk_del_provider(struct device_node *np, > > mutex_unlock(&of_clk_lock); > > } > > > > -static struct clk *__of_clk_get_from_provider(struct device_node *np, > > const char *clk_output) > > -{ > > - struct of_clk_provider *provider; > > - struct clk *clk = NULL; > > - > > - /* Check if we have such a provider in our array */ > > - mutex_lock(&of_clk_lock); > > - list_for_each_entry(provider, &of_clk_providers, link) { > > - if (provider->node == np) > > - clk = provider->get(np, clk_output, provider->data); > > - if (clk) > > - break; > > - } > > - mutex_unlock(&of_clk_lock); > > - > > - return clk; > > -} > > - > > struct clk *of_clk_get(struct device *dev, const char *id) > > { > > struct device_node *provnode; > > @@ -123,9 +105,8 @@ struct clk *of_clk_get(struct device *dev, const char > > *id) > > __func__, prop_name, dev->of_node->full_name); > > return NULL; > > } > > - clk = __of_clk_get_from_provider(provnode, prop); > > - if (clk) > > - dev_dbg(dev, "Using clock from %s\n", provnode->full_name); > > + > > + clk = provnode->data; > > Where is the device_node->data pointer getting set? > +#define ADD_CLK_LOOKUP() \ + do { \ + node->data = clk; \ ^^^^^^^^^^^^^^^^^ + \ + cl->dev_id = dev_id; \ + cl->clk = clk; \ + clkdev_add(cl); \ + \ + return 0; \ + \ + out_kfree: \ + kfree(cl); \ + return ret; \ + } while (0)
> In general the ->data pointer of struct device_node should be avoided. > There are no strong rules about its usage which means there is a very > real risk that another driver or subsystem will try to use it for a > different purpose. > > Iterating over the whole device tree is safer, and it really isn't > very expensive. If you really want to store the struct_clk pointer in > the device node, then it would be better to add a struct clk * field > to struct device_node. > -- Regards, Shawn _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev