Thread (5 messages) 5 messages, 4 authors, 2015-08-26

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

From: Stephen Boyd <hidden>
Date: 2015-08-24 23:43:19
Also in: linux-clk, linux-rockchip

On 08/24, Doug Anderson wrote:
Heiko

On Wed, Aug 19, 2015 at 6:06 AM, Heiko Stuebner [off-list ref] wrote:
quoted
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@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@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?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help