* [PATCH] ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate
@ 2014-07-23 10:44 Paul Walmsley
2014-07-30 12:18 ` Tomi Valkeinen
2014-09-17 13:02 ` Tomi Valkeinen
0 siblings, 2 replies; 3+ messages in thread
From: Paul Walmsley @ 2014-07-23 10:44 UTC (permalink / raw)
To: linux-arm-kernel
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(-)
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;
--
2.0.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate
2014-07-23 10:44 [PATCH] ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate Paul Walmsley
@ 2014-07-30 12:18 ` Tomi Valkeinen
2014-09-17 13:02 ` Tomi Valkeinen
1 sibling, 0 replies; 3+ messages in thread
From: Tomi Valkeinen @ 2014-07-30 12:18 UTC (permalink / raw)
To: linux-arm-kernel
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
-------------- 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>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate
2014-07-23 10:44 [PATCH] ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate Paul Walmsley
2014-07-30 12:18 ` Tomi Valkeinen
@ 2014-09-17 13:02 ` Tomi Valkeinen
1 sibling, 0 replies; 3+ messages in thread
From: Tomi Valkeinen @ 2014-09-17 13:02 UTC (permalink / raw)
To: linux-arm-kernel
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
-------------- 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>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-17 13:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-23 10:44 [PATCH] ARM: OMAP2+: clock: allow omap2_dpll_round_rate() to round to next-lowest rate Paul Walmsley
2014-07-30 12:18 ` Tomi Valkeinen
2014-09-17 13:02 ` Tomi Valkeinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).