Skip to contentSkip to conversation

ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate

PATCH New Paul Walmsley 2 participants 3 messages Jul 23, 2014 – Sep 17, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate

Change the behavior of omap2_dpll_round_rate() to round to either the exact rate requested, or the next lowest rate that the clock is able to provide.

This is not an ideal fix, but is intended to provide a relatively safe way for drivers to set PLL rates, until a better solution can be implemented.

For the time being, omap3_noncore_dpll_set_rate() is still allowed to set its rate to something other than what the caller requested; but will warn when this occurs.

Cc Tomi Valkeinen <[email protected]>
Cc Mike Turquette <[email protected]>
---
 arch/arm/mach-omap2/clkt_dpll.c | 28 +++++++++++++++++++++-------
 arch/arm/mach-omap2/dpll3xxx.c  | 12 ++++++++++--
 2 files changed, 31 insertions(+), 9 deletions(-)
arch/arm/mach-omap2/clkt_dpll.c +21 -7
index 332af927f4d3..85701142c5fc 100644
@@ -293,10 +293,13 @@ long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate,
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	int m, n, r, scaled_max_m;
+	int min_delta_m = INT_MAX, min_delta_n = INT_MAX;
 	unsigned long scaled_rt_rp;
 	unsigned long new_rate = 0;
 	struct dpll_data *dd;
 	unsigned long ref_rate;
+	long delta;
+	long prev_min_delta = LONG_MAX;
 	const char *clk_name;
 
 	if (!clk || !clk->dpll_data)
@@ -342,23 +345,34 @@ long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate,
 		if (r == DPLL_MULT_UNDERFLOW)
 			continue;
 
+		/* skip rates above our target rate */
+		delta = target_rate - new_rate;
+		if (delta < 0)
+			continue;
+
+		if (delta < prev_min_delta) {
+			prev_min_delta = delta;
+			min_delta_m = m;
+			min_delta_n = n;
+		}
+
 		pr_debug("clock: %s: m = %d: n = %d: new_rate = %lu\n",
 			 clk_name, m, n, new_rate);
 
-		if (target_rate == new_rate) {
-			dd->last_rounded_m = m;
-			dd->last_rounded_n = n;
-			dd->last_rounded_rate = target_rate;
+		if (delta == 0)
 			break;
-		}
 	}
 
-	if (target_rate != new_rate) {
+	if (prev_min_delta == LONG_MAX) {
 		pr_debug("clock: %s: cannot round to rate %lu\n",
 			 clk_name, target_rate);
 		return ~0;
 	}
 
-	return target_rate;
+	dd->last_rounded_m = min_delta_m;
+	dd->last_rounded_n = min_delta_n;
+	dd->last_rounded_rate = target_rate - prev_min_delta;
+
+	return dd->last_rounded_rate;
 }
 
arch/arm/mach-omap2/dpll3xxx.c +10 -2
index fcd8036af910..839a13e1208b 100644
@@ -469,6 +469,7 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	struct clk *new_parent = NULL;
+	unsigned long rrate;
 	u16 freqsel = 0;
 	struct dpll_data *dd;
 	int ret;
@@ -496,8 +497,15 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,
 		__clk_prepare(dd->clk_ref);
 		clk_enable(dd->clk_ref);
 
-		if (dd->last_rounded_rate != rate)
-			rate = __clk_round_rate(hw->clk, rate);
+		if (dd->last_rounded_rate != rate) {
+			rrate = __clk_round_rate(hw->clk, rate);
+			if (rrate != rate) {
+				pr_warn("%s: %s: final rate %lu does not match desired rate %lu\n",
+					__func__, __clk_get_name(hw->clk),
+					rrate, rate);
+				rate = rrate;
+			}
+		}
 
 		if (dd->last_rounded_rate == 0)
 			return -EINVAL;
Signature · 1 line
2.0.1
#1 lore raw

PATCH 1/1 ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate

Quoted 9 lines · Paul Walmsley

Change the behavior of omap2_dpll_round_rate() to round to either the exact rate requested, or the next lowest rate that the clock is able to provide.

This is not an ideal fix, but is intended to provide a relatively safe way for drivers to set PLL rates, until a better solution can be implemented.

Quoted 7 lines

For the time being, omap3_noncore_dpll_set_rate() is still allowed to set its rate to something other than what the caller requested; but will warn when this occurs.

Cc Tomi Valkeinen <[email protected]>
Cc Mike Turquette <[email protected]>

Tested on AM437x GP EVM with today's linux-next + this patch. Without the patch I was only able to use certain pixel clocks, but with this patch I can select the pixel clock freely (they end up rounded, of course).

So looks good to me. Thanks for working on this!

Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140730/a6768c2e/attachment.sig>

#2 lore raw

PATCH 1/1 ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate

In reply to Paul Walmsley

Hi,

Quoted 13 lines · Paul Walmsley

Change the behavior of omap2_dpll_round_rate() to round to either the exact rate requested, or the next lowest rate that the clock is able to provide.

This is not an ideal fix, but is intended to provide a relatively safe way for drivers to set PLL rates, until a better solution can be implemented.

For the time being, omap3_noncore_dpll_set_rate() is still allowed to set its rate to something other than what the caller requested; but will warn when this occurs.

Quoted 7 lines
Cc Tomi Valkeinen <[email protected]>
Cc Mike Turquette <[email protected]>
---
 arch/arm/mach-omap2/clkt_dpll.c | 28 +++++++++++++++++++++-------
 arch/arm/mach-omap2/dpll3xxx.c  | 12 ++++++++++--
 2 files changed, 31 insertions(+), 9 deletions(-)

This patch improved things quite a bit, but we still have problems. I noticed that on AM43xx:

clk_round_rate(dss_fclk, 199990846) = 199967213
clk_set_rate(dss_fclk, 199967213) -> 199966387

So similar to the issue that 7e50e7e176634e8cc0335778915be75df41043dc fixed.

Above you say that "omap3_noncore_dpll_set_rate() is still allowed to set its rate to something other than what the caller requested; but will warn when this occurs.", but I see no warning printed.

I didn't find out where the error comes from, but I (again) noticed the two issues we have:

- The omap dpll code has no maximum values, so omap2_dpll_round_rate() goes on to return ~4GHz rates as valid, which I believe it can't do.

- clk-divider.c driver doesn't handle errors from __clk_round_rate(). At least omap2_dpll_round_rate() returns ~0 on error.

Any ideas where that round/set issue might come from?

Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140917/c3a21e5a/attachment.sig>