Re: Common clock framework API vs RT patchset

[Michael Turquette <mturquette@baylibre.com>]

Quoting Russell King - ARM Linux (2015-08-11 12:25:15)
> On Tue, Aug 11, 2015 at 10:23:46PM +0300, Grygorii Strashko wrote:
> > Hi All,
> > =

> > On 08/04/2015 06:36 PM, Russell King - ARM Linux wrote:
> > > On Tue, Aug 04, 2015 at 10:23:31AM -0500, Nishanth Menon wrote:
> > >> Consider clk_enable/disable/set_parent/setfreq operations. none of t=
hese
> > >> operations are "atomic" from hardware point of view. instead, they a=
re a
> > >> set of steps which culminates to moving from state A to state B of t=
he
> > >> clock tree configuration.
> > > =

> > > There's a world of difference between clk_enable()/clk_disable() and
> > > the rest of the clk API.
> > > =

> > > clk_enable()/clk_disable() _should_ be callable from any context, sin=
ce
> > > you may need to enable or disable a clock from any context.  The rema=
inder
> > > of the clk API is callable only from contexts where sleeping is permi=
ssible.
> > > =

> > > The reason we have this split is because clk_enable()/clk_disable() h=
ave
> > > historically been used in interrupt handlers, and they're specifically
> > > not supposed to impose big delays.
> > > =

> > > Things like waiting for a PLL to re-lock is time-consuming, so it's n=
ot
> > > something I'd expect to see behind a clk_enable() implementation (the
> > > fact you can't sleep in there is a big hint.)  Such waits should be in
> > > the clk_prepare() stage instead.
> > > =

> > > Now, as for clk_enable() being interrupted - if clk_enable() is inter=
rupted
> > > and another clk_enable() comes along for the same clock, that second
> > > clk_enable() should not return until the clock has actually been enab=
led,
> > > and it's up to the implementation to decode how to achieve that.  If =
that
> > > means a RT implementation using a raw spinlock, then that's one option
> > > (which basically would have the side effect of blocking until the pre=
empted
> > > clk_enable() finishes its business.)  Alternatively, if we can preempt
> > > inside clk_enable(), then the clk_enable() implementation should be w=
ritten
> > > to cope with that (eg, by the second clk_enable() fiddling with the h=
ardware,
> > > and the first thread noticing that it has nothing to do.)
> > > =

> > =

> > Thanks a lot for your comments and explanations.
> > =

> > Now lock object in CCF is not a raw spinlock, so, seems, I have to upda=
te =

> > code and try to move clk_enable()/clk_disable() out of atomic context.
> =

> clk_enable/clk_disable _should_ be usable from atomic contexts.

Grygorii,

Note that the common clk implementation allows for the same thread to
re-enter the clock framework even while the lock is held. For instance
if calling clk_enable(foo) resulted in a call to clk_enable(bar), this
would not deadlock. However this re-entrant behavior is ONLY for the
same thread that is already holding the lock.

I doubt that the above bit of trivial will solve your problem and it
probably does not add any new complexity for you either, but it seems
relevant enough for me to add here.

Regards,
Mike

> =

> -- =

> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html