On Thursday 28 March 2013 10:02 PM, Mike Turquette wrote:
> 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
But isn't this applicable only to the _current_ parent? The bypass_clk isn't
the parent of DPLL USB when I try a .set_rate on it. Its the ref_clk.
> 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.
hmm, I didn't bother to check either. Will check, it would be strange though
if such a sequence is indeed needed.
regards,
Rajendra
>
> 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
