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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
