[PATCH 5/5] of/clock: eliminate function __of_clk_get_from_provider

Shawn Guo shawn.guo at freescale.com
Thu Mar 17 01:25:04 EST 2011


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.guo at 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?
> 
> 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

I do not understand how we can get the pointer to the 'struct clk'
from device tree.  The clock code reads nodes from device tree and
creates 'struct clk' dynamically.  If we do not save the pointer
somewhere, the pointer will get lost outside the clock code.  How
can we possibly get it back from device tree later?

> the device node, then it would be better to add a struct clk * field
> to struct device_node.
> 

-- 
Regards,
Shawn



More information about the devicetree-discuss mailing list