Clock API and "maximum rate"

[Jamie Iles]

On Thu, Jan 12, 2012 at 08:48:11AM -0600, Matt Sealey wrote:
> On Thu, Jan 12, 2012 at 8:11 AM, Russell King - ARM Linux
> <linux at arm.linux.org.uk> wrote:
> > On Thu, Jan 12, 2012 at 07:49:35AM -0600, Matt Sealey wrote:
> >> On Wed, Jan 11, 2012 at 10:58 AM, Jamie Iles <jamie at jamieiles.com> wrote:
> >> > Hi Matt,
> >> >
> >> > On Wed, Jan 11, 2012 at 10:30:15AM -0600, Matt Sealey wrote:
> >> >> Just a curious question, is there any safe way in the current API or
> >> >> even by peeking a little behind the scenes to find out what the
> >> >> maximum clock rate would be for a given named clock or struct clk?
> >> >
> >> > How about:
> >> >
> >> >        long max = clk_round_rate(clk, ~0LU);
> >> >
> >> > clk_round_rate() is one of the optional API calls though.
> >>
> >> Luckily implemented in this case :) Seems to do the trick, thanks.
> >
> > If it's not implemented, but there is an implemented clk_set_rate(), feel
> > free to complain at whoever created the implementation.
> >
> > If there's a clk_set_rate() implementation, then there's _already_ an
> > implementation of clk_round_rate() internally doing the rounding for the
> > set_rate function, so there's really no excuse not to expose that via
> > clk_round_rate().
> >
> > clk_round_rate() is supposed to tell you what you end up with if you
> > ask clk_set_rate() to set the exact same value you passed in - but
> > clk_round_rate() won't modify the hardware.
> >
> > So, clk_round_rate/clk_set_rate really should be thought of being
> > implemented as this:
> >
> > long clk_round_rate(struct clk *clk, unsigned long rate)
> > {
> >        return __clk_round_rate(clk, rate);
> > }
> >
> > int clk_set_rate(struct clk *clk, unsigned long rate)
> > {
> >        long rate = __clk_round_rate(clk, rate);
> >
> >        if (rate < 0)
> >                return rate;
> >
> >        return __clk_set_rate(clk, rate);
> > }
> 
> Right. On i.MX it seems not to round first, but to assume you passed a
> rounded rate? I assume the upper level clk API does the above, so the
> low level doesn't have to?

Well each platform implements the whole clock API itself at the moment 
so I guess the higher layer is in plat-mxc?  If so then something like 
the (untested) patch below may do the trick.  clk_round_rate() for this 
platform returns 0 when it fails though and that's not right from the 
API:

/**
 * clk_round_rate - adjust a rate to the exact rate a clock can provide
 * @clk: clock source
 * @rate: desired clock rate in Hz
 *
 * Returns rounded clock rate in Hz, or negative errno.
 */

Jamie

8<---

diff --git a/arch/arm/plat-mxc/clock.c b/arch/arm/plat-mxc/clock.c
index 2ed3ab1..64f679b 100644
--- a/arch/arm/plat-mxc/clock.c
+++ b/arch/arm/plat-mxc/clock.c
@@ -134,7 +134,7 @@ EXPORT_SYMBOL(clk_get_rate);
 long clk_round_rate(struct clk *clk, unsigned long rate)
 {
 	if (clk == NULL || IS_ERR(clk) || !clk->round_rate)
-		return 0;
+		return -EOPNOTSUPP;
 
 	return clk->round_rate(clk, rate);
 }
@@ -146,12 +146,17 @@ EXPORT_SYMBOL(clk_round_rate);
 int clk_set_rate(struct clk *clk, unsigned long rate)
 {
 	int ret = -EINVAL;
+	long rounded_rate;
 
 	if (clk == NULL || IS_ERR(clk) || clk->set_rate == NULL || rate == 0)
 		return ret;
 
+	rounded_rate = clock_round_rate(clk, rate);
+	if (rounded_rate < 0)
+		return rounded_rate;
+
 	mutex_lock(&clocks_mutex);
-	ret = clk->set_rate(clk, rate);
+	ret = clk->set_rate(clk, rounded_rate);
 	mutex_unlock(&clocks_mutex);
 
 	return ret;