| Message ID | 1375956851-842-1-git-send-email-rnayak@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
Quoting Paul Walmsley (2013-08-08 20:47:02) > 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. No problem. I had not yet published it to clk-next so this causes no issue. Regards, Mike > > > - Paul
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; }
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(-)