On 04/22/2017 05:22 PM, Krzysztof Kozlowski wrote: [...]
+static void samsung_pll3xxx_disable(struct clk_hw *hw) +{ + struct samsung_clk_pll *pll = to_clk_pll(hw); + u32 tmp; + + tmp = readl_relaxed(pll->con_reg); + tmp |= BIT(pll->enable_offs);I think you meant here: tmp &= ~BIT()
Yes, I messed it up while copy/pasting from samsung_pll3xxx_enable().
+ writel_relaxed(tmp, pll->con_reg); +}
static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) @@ -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.
+ do { + cpu_relax(); + tmp = readl_relaxed(pll->con_reg); + } while (!(tmp & BIT(PLL36XX_LOCK_STAT_SHIFT)));To be consistent: BIT(pll->lock_offs)?
Yes, fixed. Thanks for your review! -- Regards, Sylwester -- 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
