[PATCH] clk: rockchip: register pll mux before pll itself

[Heiko Stuebner]

Am Montag, 24. August 2015, 16:43:19 schrieb Stephen Boyd:
> On 08/24, Doug Anderson wrote:
> > Heiko
> > 
> > On Wed, Aug 19, 2015 at 6:06 AM, Heiko Stuebner <heiko at sntech.de> wrote:
> > > The structure is xin24m -> pll -> pll-mux (xin24m,pll,xin32k). The pll
> > > does have an init callback to make sure the boot-selected frequency is
> > > using the expected pll settings and resets the same frequency using
> > > the values provided in the driver if necessary.
> > > 
> > > The setting itself also involves remuxing the pll-mux temporarily to
> > > the xin24m source to let the new pll rate settle. Until now this worked
> > > flawlessly, even when it had the flaw of accessing the mux settings
> > > before the mux actually got registered.
> > > 
> > > With the recent clock-core conversions this flaw became apparent in
> > > null pointer dereference in
> > > [<c03fc400>] (clk_hw_get_num_parents) from [<c0400df0>]
> > > (clk_mux_get_parent+0x14/0xc8) [<c0400ddc>] (clk_mux_get_parent) from
> > > [<c040246c>] (rockchip_rk3066_pll_set_rate+0xd8/0x320)
> > > 
> > > So to fix that, simply register the pll-mux before the pll, so that
> > > it will be fully initialized when the pll clock executes its init-
> > > callback and possibly touches the pll-mux clock.
> > > 
> > > Signed-off-by: Heiko Stuebner <heiko at sntech.de>
> > > ---
> > > This only surfaced with the clk_core changes for 4.3, so should
> > > probably just go on top.
> > > 
> > >  drivers/clk/rockchip/clk-pll.c | 63
> > >  +++++++++++++++++++++--------------------- 1 file changed, 32
> > >  insertions(+), 31 deletions(-)
> > 
> > Fixes boot crash on rk3288-veyron-jerry on next-20150824.  It'd be
> > super great to get this landed somewhere so that we can boot linuxnext
> > again.  :)
> > 
> > Tested-by: Douglas Anderson <dianders at chromium.org>
> 
> So I understand the fix, but how could it have ever possibly
> worked flawlessly? clk_mux_get_parent() should have returned
> -EINVAL through that u8 which would have meant that the check in
> rockchip_rk3066_pll_set_rate() for cur_parent == PLL_MODE_NORM
> would never have been true, and we would never have switched the
> PLL mux over. I guess we've been getting away with this because
> we don't need to actually switch the mux at this time?

The manual mandates the switch away from the pll source before touching the 
pll settings, aka to the slow mode from the 24MHz source. I guess we were 
lucky it simply still worked nevertheless.

In normal rate changes both clocks will have been registered already, so only 
the init callback is affected and also only runs on plls that generate the rate 
from a different settings-touple that what we have in the rate-table, so not 
even all plls normally.