diff mbox

ARM: OMAP4: clock: Lock PLLs in the right sequence

Message ID 1375956851-842-1-git-send-email-rnayak@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rajendra Nayak Aug. 8, 2013, 10:14 a.m. UTC
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
    |
    V
dpll_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.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/cclock44xx_data.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Mike Turquette Aug. 9, 2013, 1:18 a.m. UTC | #1
Quoting Rajendra Nayak (2013-08-08 03:14:11)
> 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.
...
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>

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

> ---
>  arch/arm/mach-omap2/cclock44xx_data.c |   20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c
> index 88e37a4..1d5b529 100644
> --- a/arch/arm/mach-omap2/cclock44xx_data.c
> +++ b/arch/arm/mach-omap2/cclock44xx_data.c
> @@ -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;
>  }
> -- 
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley Aug. 9, 2013, 3:47 a.m. UTC | #2
Hi Mike,

On Thu, 8 Aug 2013, Mike Turquette wrote:

> Quoting Rajendra Nayak (2013-08-08 03:14:11)
> > 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.
> ...
> > Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c
index 88e37a4..1d5b529 100644
--- a/arch/arm/mach-omap2/cclock44xx_data.c
+++ b/arch/arm/mach-omap2/cclock44xx_data.c
@@ -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;
 }