| Message ID | alpine.DEB.2.02.1407231043230.32419@utopia.booyaka.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23/07/14 13:44, Paul Walmsley wrote: > > 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 <tomi.valkeinen@ti.com> > Cc: Mike Turquette <mturquette@linaro.org> > Signed-off-by: Paul Walmsley <paul@pwsan.com> 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
Hi, On 23/07/14 13:44, Paul Walmsley wrote: > > 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 <tomi.valkeinen@ti.com> > Cc: Mike Turquette <mturquette@linaro.org> > Signed-off-by: Paul Walmsley <paul@pwsan.com> > --- > 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
diff --git a/arch/arm/mach-omap2/clkt_dpll.c b/arch/arm/mach-omap2/clkt_dpll.c index 332af927f4d3..85701142c5fc 100644 --- a/arch/arm/mach-omap2/clkt_dpll.c +++ b/arch/arm/mach-omap2/clkt_dpll.c @@ -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; } diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c index fcd8036af910..839a13e1208b 100644 --- a/arch/arm/mach-omap2/dpll3xxx.c +++ b/arch/arm/mach-omap2/dpll3xxx.c @@ -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;
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 <tomi.valkeinen@ti.com> Cc: Mike Turquette <mturquette@linaro.org> Signed-off-by: Paul Walmsley <paul@pwsan.com> --- arch/arm/mach-omap2/clkt_dpll.c | 28 +++++++++++++++++++++------- arch/arm/mach-omap2/dpll3xxx.c | 12 ++++++++++-- 2 files changed, 31 insertions(+), 9 deletions(-)