Re: [PATCH RFC 1/7] clk: samsung: Add enable/disable operation for PLL36XX clocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux