[PATCH 08/13] ARM: LPC32XX: clock tree support

[u.kleine-koenig@pengutronix.de (Uwe Kleine-König)]

On Wed, Feb 03, 2010 at 07:51:36PM +0100, Kevin Wells wrote:
> > > +/*
> > > + * clk_get_rate - obtain the current clock rate (in Hz) for a clock
> > source
> > > + */
> > > +unsigned long clk_get_rate(struct clk *clk)
> > > +{
> > > +	if (clk->get_rate)
> > > +		return (clk->get_rate)(clk);
> > > +
> > > +	/* If a clocks rate is 0, it uses the parent's rate instead. */
> > > +	while (clk->rate == 0)
> > > +		clk = clk->parent;
> > > +
> > > +	return clk->rate;
> > > +}
> > > +EXPORT_SYMBOL(clk_get_rate);
> > doesn't that need locking to protect against a race with clk_set_rate?
> > Hmmm, you never need to call get_rate for a parent clock?
> > 
> 
> I'll add a lock for this, its needed. The get_rate() function will just
> continue to fall back to the first non-0Hz parent clock (all clocks
> eventually go to a non-0Hz parent clock). Most clocks don't even have
> a get_rate() function.
still, if a clock with a get_rate function has a child, clk_get_rate for
the child might return a wrong result.

Actually you need something like:

	while (!(rate = clk->rate) && !(clk->get_rate && (rate = clk->get_rate(clk)))) {
		clk = clk->parent;
		if (!clk)
			break;
	}	
	return rate;

Maybe with a nicer coding style by not using an assignment in the while
condition.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |