On Thu, 2 Jun 2016, Dong Aisheng wrote:
> On Wed, Apr 27, 2016 at 12:15:00PM +0200, Thomas Gleixner wrote:
> > Calling a function which might sleep _BEFORE_ kernel_init() is wrong. Don't
> > try to work around such an issue by doing magic irq_disabled() checks and
> > busy
> > loops. Fix the call site and be done with it.
> >
>
> IRQ chip and clocksource are also initialised before kernel_init()
> which may call clk APIs like clk_prepare_enable().
> We may not be able to guarantee those clocks are unsleepable.
>
> e.g.
> For IRQ chip, the arm,gic documents the optional clocks property in
> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt.
> Although there seems no user currently, not sure there might be
> a exception in the future.
>
> For clocksource driver, it's quite common to call clk APIs to
> enable and get clock rate which may also cause sleep.
> e.g. ARM twd clock in arch/arm/kernel/smp_twd.c.
>
> If we have to avoid the possible sleep caused by these clocks,
> we may need to manually check it and change the related clocks
> (e.g PLLs) from sleepable to busy loops.
> But that is not a good solution and may also not stable when
> clock varies.
>
> It looks like not easy to find a generic solution for them.
> What's your suggestion?
I think we should split the prepare callbacks up and move the handling logic
into the core.
clk_ops.prepare() Legacy callback
clk_ops.prepare_hw() Callback which writes to the hardware
clk_ops.prepare_done() Callback which checks whether the prepare is completed
So now the core can do:
clk_prepare()
{
/* Legacy code should go away */
if (ops->prepare) {
ops->prepare();
return;
}
if (ops->prepare_hw)
ops->prepare_hw();
if (!ops->prepare_done())
return;
if (early_boot_or_suspend_resume()) {
/*
* Busy loop when we can't schedule in early boot,
* suspend and resume.
*/
while (!ops->prepare_done())
;
} else {
while (!ops->prepare_done())
usleep(clk->prepare_delay);
}
}
As a side effect that allows us to remove the gazillion of
while (!hw_ready)
usleep();
copies all over the place and we have a single point of control where we allow
the clocks to busy loop.
Toughts?
Thanks,
tglx
