Quoting Rajendra Nayak (2013-03-28 03:59:41)
> omap3_noncore_dpll_set_rate() attempts an enable of bypass clk as well
> as ref clk for every .set_rate attempt on a noncore DPLL, regardless of
> whether the .set_rate results in the DPLL being locked or put in bypass.
> Early at boot, while some of these DPLLs are programmed and locked
> (using .set_rate for the DPLL), this causes an ordering issue.
>
> For instance, on OMAP5, the USB DPLL derives its bypass clk from ABE DPLL.
> If a .set_rate of USB DPLL which programmes the M,N and locks it is called
> before the one for ABE, the enable of USB bypass clk (derived from ABE DPLL)
> then attempts to lock the ABE DPLL and fails as the M,N values for ABE
> are yet to be programmed.
>
> To get rid of this ordering needs, enable bypass clk for a DPLL as part
> of its .set_rate only when its being put in bypass, and only enable the
> ref clk when its locked.
>
Hi Rajendra,
Another way to solve this problem would be to model the DPLLs as mux
clocks with a list of possible parents (e.g. clk->parents[2]). Then set
the CLK_SET_RATE_PARENT flag on the USB DPLL which will allow to
propagate the rate request up to the ABE DPLL. This should set the MN
dividers appropriately.
> Reported-by: Roger Quadros <rogerq@xxxxxx>
> Signed-off-by: Rajendra Nayak <rnayak@xxxxxx>
> ---
> arch/arm/mach-omap2/dpll3xxx.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c
> index 3aed4b0..6e9873f 100644
> --- a/arch/arm/mach-omap2/dpll3xxx.c
> +++ b/arch/arm/mach-omap2/dpll3xxx.c
> @@ -480,20 +480,22 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
> if (!dd)
> return -EINVAL;
>
> - __clk_prepare(dd->clk_bypass);
> - clk_enable(dd->clk_bypass);
> - __clk_prepare(dd->clk_ref);
> - clk_enable(dd->clk_ref);
Is this safe? I always thought that the programming sequence in the TRM
was to enable both the ref and bypass clocks during DPLL reprogramming.
Maybe I am misremembering or assuming that the code strictly followed
the TRM.
Regards,
Mike
> -
> if (__clk_get_rate(dd->clk_bypass) == rate &&
> (dd->modes & (1 << DPLL_LOW_POWER_BYPASS))) {
> pr_debug("%s: %s: set rate: entering bypass.\n",
> __func__, __clk_get_name(hw->clk));
>
> + __clk_prepare(dd->clk_bypass);
> + clk_enable(dd->clk_bypass);
> ret = _omap3_noncore_dpll_bypass(clk);
> if (!ret)
> new_parent = dd->clk_bypass;
> + clk_disable(dd->clk_bypass);
> + __clk_unprepare(dd->clk_bypass);
> } else {
> + __clk_prepare(dd->clk_ref);
> + clk_enable(dd->clk_ref);
> +
> if (dd->last_rounded_rate != rate)
> rate = __clk_round_rate(hw->clk, rate);
>
> @@ -514,6 +516,8 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
> ret = omap3_noncore_dpll_program(clk, freqsel);
> if (!ret)
> new_parent = dd->clk_ref;
> + clk_disable(dd->clk_ref);
> + __clk_unprepare(dd->clk_ref);
> }
> /*
> * FIXME - this is all wrong. common code handles reparenting and
> @@ -525,11 +529,6 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
> if (!ret)
> __clk_reparent(hw->clk, new_parent);
>
> - clk_disable(dd->clk_ref);
> - __clk_unprepare(dd->clk_ref);
> - clk_disable(dd->clk_bypass);
> - __clk_unprepare(dd->clk_bypass);
> -
> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
