ARM: OMAP4: clock: Lock PLLs in the right sequence
PATCH 1/1 ARM: OMAP4: clock: Lock PLLs in the right sequence
On OMAP4 we have clk_set_rate()s being done for a few DPLL clock nodes, as part of the clock init code, since the bootloaders no longer locks these DPLLs.
So we have a clk_set_rate() done for a ABE DPLL node (which inturn locks it) followed by a clk_set_rate() for the USB DPLL.
With USB DPLL being in bypass, we have this parent->child relationship thats formed while the clocks get registered.
dpll_abe_ck
|
V
dpll_abe_x2_ck
|
V
dpll_abe_m3x2_ck
|
V
usb_hs_clk_div_ck
|
Vdpll_usb_ck
This is because usb_hs_clk_div_ck is bypass clock for dpll_usb_ck.
So with this parent->child relationship in place, a clk_set_rate() on ABE DPLL results eventually in a clk_set_rate() call on USB DPLL, because CCF does a clk_change_rate() (as part of clk_set_rate()) on all downstream clocks resulting from a rate change on the top clock.
So its important that we lock USB DPLL before we lock ABE DPLL. Without which we see these error logs at boot.
[These error logs will not be seen if using a bootloader that locks USB DPLL]
[ 0.000000] clock: dpll_usb_ck failed transition to 'locked' [ 0.000000] Division by zero in kernel. [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-03445-gfb2af00-dirty #7 [ 0.000000] [<c001bfe8>] (unwind_backtrace+0x0/0xf4) from [<c001868c>] (show_stack+0x10/0x14) [ 0.000000] [<c001868c>] (show_stack+0x10/0x14) from [<c02deb28>] (Ldiv0+0x8/0x10) [ 0.000000] [<c02deb28>] (Ldiv0+0x8/0x10) from [<c0477030>] (clk_divider_set_rate+0x10/0x114) [ 0.000000] [<c0477030>] (clk_divider_set_rate+0x10/0x114) from [<c0476ef4>] (clk_change_rate+0x38/0xb8) [ 0.000000] [<c0476ef4>] (clk_change_rate+0x38/0xb8) from [<c0476f5c>] (clk_change_rate+0xa0/0xb8) [ 0.000000] Division by zero in kernel. [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-03445-gfb2af00-dirty #7 [ 0.000000] [<c001bfe8>] (unwind_backtrace+0x0/0xf4) from [<c001868c>] (show_stack+0x10/0x14) [ 0.000000] [<c001868c>] (show_stack+0x10/0x14) from [<c02deb28>] (Ldiv0+0x8/0x10) [ 0.000000] [<c02deb28>] (Ldiv0+0x8/0x10) from [<c0477030>] (clk_divider_set_rate+0x10/0x114) [ 0.000000] [<c0477030>] (clk_divider_set_rate+0x10/0x114) from [<c0476ef4>] (clk_change_rate+0x38/0xb8) [ 0.000000] [<c0476ef4>] (clk_change_rate+0x38/0xb8) from [<c0476f5c>] (clk_change_rate+0xa0/0xb8) [ 0.000000] Division by zero in kernel. [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-03445-gfb2af00-dirty #7 [ 0.000000] [<c001bfe8>] (unwind_backtrace+0x0/0xf4) from [<c001868c>] (show_stack+0x10/0x14) [ 0.000000] [<c001868c>] (show_stack+0x10/0x14) from [<c02deb28>] (Ldiv0+0x8/0x10) [ 0.000000] [<c02deb28>] (Ldiv0+0x8/0x10) from [<c0477030>] (clk_divider_set_rate+0x10/0x114) [ 0.000000] [<c0477030>] (clk_divider_set_rate+0x10/0x114) from [<c0476ef4>] (clk_change_rate+0x38/0xb8) [ 0.000000] [<c0476ef4>] (clk_change_rate+0x38/0xb8) from [<c0476f5c>] (clk_change_rate+0xa0/0xb8) [ 0.000000] Division by zero in kernel. [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-03445-gfb2af00-dirty #7 [ 0.000000] [<c001bfe8>] (unwind_backtrace+0x0/0xf4) from [<c001868c>] (show_stack+0x10/0x14) [ 0.000000] [<c001868c>] (show_stack+0x10/0x14) from [<c02deb28>] (Ldiv0+0x8/0x10) [ 0.000000] [<c02deb28>] (Ldiv0+0x8/0x10) from [<c0477030>] (clk_divider_set_rate+0x10/0x114) [ 0.000000] [<c0477030>] (clk_divider_set_rate+0x10/0x114) from [<c0476ef4>] (clk_change_rate+0x38/0xb8) [ 0.000000] [<c0476ef4>] (clk_change_rate+0x38/0xb8) from [<c0476f5c>] (clk_change_rate+0xa0/0xb8) [ 0.000000] Division by zero in kernel. [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-03445-gfb2af00-dirty #7 [ 0.000000] [<c001bfe8>] (unwind_backtrace+0x0/0xf4) from [<c001868c>] (show_stack+0x10/0x14) [ 0.000000] [<c001868c>] (show_stack+0x10/0x14) from [<c02deb28>] (Ldiv0+0x8/0x10) [ 0.000000] [<c02deb28>] (Ldiv0+0x8/0x10) from [<c0477030>] (clk_divider_set_rate+0x10/0x114) [ 0.000000] [<c0477030>] (clk_divider_set_rate+0x10/0x114) from [<c0476ef4>] (clk_change_rate+0x38/0xb8) [ 0.000000] [<c0476ef4>] (clk_change_rate+0x38/0xb8) from [<c0476f5c>] (clk_change_rate+0xa0/0xb8) [ 0.000000] Division by zero in kernel. [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0-03445-gfb2af00-dirty #7 [ 0.000000] [<c001bfe8>] (unwind_backtrace+0x0/0xf4) from [<c001868c>] (show_stack+0x10/0x14) [ 0.000000] [<c001868c>] (show_stack+0x10/0x14) from [<c02deb28>] (Ldiv0+0x8/0x10) [ 0.000000] [<c02deb28>] (Ldiv0+0x8/0x10) from [<c0477030>] (clk_divider_set_rate+0x10/0x114) [ 0.000000] [<c0477030>] (clk_divider_set_rate+0x10/0x114) from [<c0476ef4>] (clk_change_rate+0x38/0xb8) [ 0.000000] [<c0476ef4>] (clk_change_rate+0x38/0xb8) from [<c0476f5c>] (clk_change_rate+0xa0/0xb8) [ 0.000000] clock: trace_clk_div_ck: could not find divisor for target rate 0 for parent pmd_trace_clk_mux_ck [ 0.000000] Division by zero in kernel.
--- arch/arm/mach-omap2/cclock44xx_data.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
arch/arm/mach-omap2/cclock44xx_data.c +12 -8
@@ -1707,6 +1707,18 @@ int __init omap4xxx_clk_init(void)omap2_clk_disable_autoidle_all ();/* +* A set rate of ABE DPLL inturn triggers a set rate of USB DPLL +* when its in bypass. So always lock USB before ABE DPLL. +*/ +/* +* Lock USB DPLL on OMAP4 devices so that the L3INIT power +* domain can transition to retention state when not in use. +*/ + rc =clk_set_rate (&dpll_usb_ck,OMAP4_DPLL_USB_DEFFREQ ); +if (rc) +pr_err ("%s: failed to configure USB DPLL!\n" , __func__); + +/* * On OMAP4460 the ABE DPLL fails to turn on if in idle low-power * state when turning the ABE clock domain. Workaround this by * locking the ABE DPLL on boot.
@@ -1718,13 +1730,5 @@ int __init omap4xxx_clk_init(void)if (rc)pr_err ("%s: failed to configure ABE DPLL!\n" , __func__); -/* -* Lock USB DPLL on OMAP4 devices so that the L3INIT power -* domain can transition to retention state when not in use. -*/ - rc =clk_set_rate (&dpll_usb_ck,OMAP4_DPLL_USB_DEFFREQ ); -if (rc) -pr_err ("%s: failed to configure USB DPLL!\n" , __func__); -return 0 ; }
Signature · 1 line
1.7.9.5
PATCH 1/1 ARM: OMAP4: clock: Lock PLLs in the right sequence
Quoting Rajendra Nayak (2013-08-08 03:14:11)
Quoted 3 lines
On OMAP4 we have clk_set_rate()s being done for a few DPLL clock nodes, as part of the clock init code, since the bootloaders no longer locks these DPLLs.
...
Quoted 1 line
Signed-off-by Rajendra Nayak <[email protected]>
Taken into clk-next. Thanks for the fix. Besides the annoying prints is there a functional regression fixed by this? If so I can take into clk-fixes as needed.
I haven't spent much time thinking about this, but I wonder if representing the DPLLs as proper mux clocks with multiple parents coming in would have helped here? Right now the DPLL implementation sort of manages the mux bits behind the clock framework's back, right?
Regards,
Mike
Quoted 43 lines
--- arch/arm/mach-omap2/cclock44xx_data.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)arch/arm/mach-omap2/cclock44xx_data.c +12
-8@@ -1707,6 +1707,18 @@ int __init omap4xxx_clk_init(void)omap2_clk_disable_autoidle_all ();/* +* A set rate of ABE DPLL inturn triggers a set rate of USB DPLL +* when its in bypass. So always lock USB before ABE DPLL. +*/ +/* +* Lock USB DPLL on OMAP4 devices so that the L3INIT power +* domain can transition to retention state when not in use. +*/ + rc =clk_set_rate (&dpll_usb_ck,OMAP4_DPLL_USB_DEFFREQ ); +if (rc) +pr_err ("%s: failed to configure USB DPLL!\n" , __func__); + +/* * On OMAP4460 the ABE DPLL fails to turn on if in idle low-power * state when turning the ABE clock domain. Workaround this by * locking the ABE DPLL on boot. @@ -1718,13 +1730,5 @@ int __init omap4xxx_clk_init(void)if (rc)pr_err ("%s: failed to configure ABE DPLL!\n" , __func__); -/* -* Lock USB DPLL on OMAP4 devices so that the L3INIT power -* domain can transition to retention state when not in use. -*/ - rc =clk_set_rate (&dpll_usb_ck,OMAP4_DPLL_USB_DEFFREQ ); -if (rc) -pr_err ("%s: failed to configure USB DPLL!\n" , __func__); -return 0 ; }Signature · 1 line
1.7.9.5
Paul Walmsley
maintainerPATCH 1/1 ARM: OMAP4: clock: Lock PLLs in the right sequence
Hi Mike,
Quoted 10 lines · Mike Turquette
Quoting Rajendra Nayak (2013-08-08 03:14:11)
Quoted 3 lines
On OMAP4 we have clk_set_rate()s being done for a few DPLL clock nodes, as part of the clock init code, since the bootloaders no longer locks these DPLLs.
...
Quoted 1 line
Signed-off-by Rajendra Nayak <[email protected]>Taken into clk-next. Thanks for the fix. Besides the annoying prints is there a functional regression fixed by this? If so I can take into clk-fixes as needed.
It's best if I take this patch upstream. That way, potential merge conflicts are avoided, since I might have other changes to arch/arm/mach-omap2/cclock44xx_data.c to send upstream at the same time.
- Paul
PATCH 1/1 ARM: OMAP4: clock: Lock PLLs in the right sequence
Quoting Paul Walmsley (2013-08-08 20:47:02)
Quoted 11 lines
Hi Mike,
Quoted 7 lines · Mike Turquette
Quoting Rajendra Nayak (2013-08-08 03:14:11)
Quoted 3 lines
On OMAP4 we have clk_set_rate()s being done for a few DPLL clock nodes, as part of the clock init code, since the bootloaders no longer locks these DPLLs.
...
Quoted 1 line
Signed-off-by Rajendra Nayak <[email protected]>
Quoted 7 lines
Quoted 3 lines
Taken into clk-next. Thanks for the fix. Besides the annoying prints is there a functional regression fixed by this? If so I can take into clk-fixes as needed.
It's best if I take this patch upstream. That way, potential merge conflicts are avoided, since I might have other changes to
arch/arm/mach-omap2/cclock44xx_data.cto send upstream at the same time.
No problem. I had not yet published it to clk-next so this causes no issue.
Regards,
Mike
Quoted 3 lines
- Paul