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

Reply via email to