Locking in the clk API

[Nicolas Pitre]

On Fri, 21 Jan 2011, Colin Cross wrote:

> On Fri, Jan 21, 2011 at 2:02 PM, Russell King - ARM Linux
> <linux at arm.linux.org.uk> wrote:
> > On Fri, Jan 21, 2011 at 04:53:44PM -0500, Nicolas Pitre wrote:
> >> So I think that the API must be augmented with more methods, such as:
> >>
> >> clk_slow_enable():
> >>   - may sleep
> >>   - may be a no-op if the clk_fast_enable() is supported
> >>
> >> clk_fast_enable():
> >>   - may not sleep, used in atomic context
> >>   - may be a no-op if controlling the clock takes time, in which case
> >>     clk_slow_enable() must have set the clock up entirely
> >>
> >> ... and similar for clk_slow_disable() and clk_fast_disable().
> >
> > Isn't this along the same lines as my clk_prepare() vs clk_enable()
> > suggestion?
> >
> > I suggested that clk_prepare() be callable only from non-atomic contexts,
> > and do whatever's required to ensure that the clock is available.  That
> > may end up enabling the clock as a result.
> >
> > clk_enable() callable from atomic contexts, and turns the clock on if
> > the hardware supports such an operation.
> >
> > So, if you have something like:
> >
> > Xtal--->PLL--->Routing/Masking--->Device
> >
> > clk = clk_get() returns the clock for the device.
> >
> > clk_prepare(clk) would walk up the clock tree, selecting the routing and
> > preparing each clock.  Clocks prior to _and_ including the PLL would need
> > to be enabled.
> >
> > clk_enable(clk) would walk up the tree if the clock isn't already enabled,
> > calling clk_enable() on the parent clock.  As we require prepared clocks
> > to already be enabled, this automatically stops at the PLL.
> >
> > To encourage correct usage, we just need to make sure that clk_prepare()
> > has a might_sleep() thing, and clk_enable() throws a fit if it's used
> > on a clk without prepare being used first.  The second point is not easy
> > to do in a foolproof manner though, but doing _something_ is better than
> > nothing.
> 
> I like this proposal, and I prefer the clk_prepare naming over
> clk_slow_enable - too many people would call clk_slow_enable instead
> of, and not as well as, clk_fast_enable.

I do prefer those names too.  My clk_slow/fast_enable() was just an 
example for illustration purpose.

> On Tegra, I currently use the ugly conditional mutex or spinlock
> method to deal with voltage scaling based on clock frequency.  Clocks
> that have a voltage dependency, or depend on a clock that has a
> voltage dependency, are non-atomic, everything else is atomic.  PLLs
> are atomic because they lock very fast (300 uS or 1ms) and are shared
> by so many clocks that they realistically don't get turned off very
> often, and if I made them non-atomic, all clocks would be non-atomic.

That's of course implementation details.  The API should define purpose, 
and then you get to decide how your hardware will behave.  If you 
consider that 300us is short enough to busy wait for, and that the 
likelyhood for that PLL to be disabled is low, then you can make it into 
the clk_enable() and make clk_prepare() empty.  But if you are going to 
use a driver with aggressive power saving through frequent usage of 
clk_enable()/clk_disable() then it is probably best to lock the PLLs in 
clk_prepare() instead.


Nicolas