24 Apr
2017
24 Apr
'17
1:18 p.m.
On Mon, Apr 24, 2017 at 1:12 PM, Sylwester Nawrocki s.nawrocki@samsung.com 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