On Mon, Apr 24, 2017 at 1:12 PM, Sylwester Nawrocki
<s.nawrocki@xxxxxxxxxxx> wrote:
> On 04/22/2017 05:22 PM, Krzysztof Kozlowski wrote:
>>> @@ -354,10 +359,12 @@ static int samsung_pll36xx_set_rate(struct clk_hw
>>> *hw, unsigned long drate,
>>> writel_relaxed(pll_con1, pll->con_reg + 4);
>>>
>>> /* wait_lock_time */
>>> - do {
>>> - cpu_relax();
>>> - tmp = readl_relaxed(pll->con_reg);
>>> - } while (!(tmp & (1 << PLL36XX_LOCK_STAT_SHIFT)));
>
>
> I will add a comment here like:
> /* Wait until the PLL is locked if it is enabled. */
>
>>> + if (pll_con0 & BIT(pll->enable_offs)) {
>>
>>
>> Why this additional if() is needed?
>
>
> The PLL will never transition to a locked state if it is disabled, without
> this test we would end up with an indefinite loop below.
Hmmm... shouldn't this be split from this change? Or maybe this is
needed only because you added enable/disable for pl36xx and previously
this was not existing?
Best regards,
Krzysztof
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
