Clock API and "maximum rate"

Clock API and "maximum rate"

[Matt Sealey]

Hi guys,

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?

I have a specific issue with the MX5 IPU whereby the IPU clock defines
the upper limit of the pixel clock. On MX51 it's around 133MHz, thus
limiting displays to 133MHz - this means not 1080p60 but 1080p24 would
and does work). On MX53 it's 150MHz which encompasses the CEA 1080p60
(148.25MHz) modes but probably not anything higher like 1600x1200. Of
course this mostly depends on the monitor used (I have one that has
1600x1200 at 50 at 125MHz or so), but in the end the IPU driver should
refuse to accept such a mode (as invalid) and any DRM connector or
subdriver for HDMI, DVI, VGA or whatever should certainly parse these
modes out from standard available mode lists too. This behavior relies
on the idea that we know where we're deriving the pixel clock from and
where we need to do the refusal to set mode or parsing of the mode
lists, so I am not sure if there is even a single place to do so, but
knowing the maximum clock seems like a better idea than perhaps just
checking for a particular running cpu_is_imx51() or so and giving a
hardcoded value (especially since the PLL the IPU clock is derived
from could possibly limit possible clock rates below the maximum).

So, is there a solution for this, or at least a nice way to report
such information without making a guess of the system state?

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Clock API and "maximum rate"

[Jamie Iles]

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.

Jamie

Clock API and "maximum rate"

[Matt Sealey]

On Wed, Jan 11, 2012 at 10:58 AM, Jamie Iles <jamie@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.

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Clock API and "maximum rate"

[Russell King - ARM Linux]

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@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);
}

Clock API and "maximum rate"

[Matt Sealey]

On Thu, Jan 12, 2012 at 8:11 AM, Russell King - ARM Linux
<linux@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@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?

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

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@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@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;