All of lore.kernel.org
 help / color / mirror / Atom feed
From: u.kleine-koenig@pengutronix.de (Uwe Kleine-König)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: divider: Use DIV_ROUND_CLOSEST
Date: Thu, 21 Mar 2013 10:15:31 +0100	[thread overview]
Message-ID: <20130321091531.GN20530@pengutronix.de> (raw)
In-Reply-To: <20130320185051.GA28349@pengutronix.de>

Hello,

On Wed, Mar 20, 2013 at 07:50:51PM +0100, Sascha Hauer wrote:
> On Wed, Mar 20, 2013 at 09:32:51AM -0700, S?ren Brinkmann wrote:
> > If the caller
> > doesn't like the returned frequency he can request a different one.
> > And he's eventually happy with the return value he calls
> > clk_set_rate() requesting the frequency clk_round_rate() returned.
> > Always rounding down seems a bit odd to me.
> > 
> > Another issue with the current implmentation:
> > clk_divider_round_rate() calls clk_divider_bestdiv(), which uses the ROUND_UP macro, returning a rather low frequency.
> 
> And that is correct. clk_divider_bestdiv is used to calculate the
> maximum parent frequency for which a given divider value does not
> exceed the desired rate.
The reason for that is that the (more?) usual constraint is like: This
mmc card can handle up to 100 MHz. Or this i2c device can handle up to
this and that frequency. Of course there are different constraints, e.g.
for a UART if the target baud speed is 38400 you better run at 38402
than at 19201.

I wonder if it depends on the clock if you want "best approximation <=
requested value" or "best approximation" or on the caller. In the former
case a flag for the clock would be the right thing (as suggested in this
thread). If however it's the caller of round_rate who knows better which
rounding is preferred than better extend the clk API.

Extending the API could just be a convenience function that doesn't
affect the implementations of the clk API. E.g.:

	long clk_round_rate_nearest(struct clk *clk, unsigned long rate)
	{
		long lower_limit = clk_round_rate(clk, rate);
		long upper_limit = clk_round_rate(clk, rate + (rate - lower_limit));

		if (rate - lower_limit < upper_limit - rate)
			return lower_limit;
		else
			return upper_limit;
	}

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>,
	Mike Turquette <mturquette@linaro.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] clk: divider: Use DIV_ROUND_CLOSEST
Date: Thu, 21 Mar 2013 10:15:31 +0100	[thread overview]
Message-ID: <20130321091531.GN20530@pengutronix.de> (raw)
In-Reply-To: <20130320185051.GA28349@pengutronix.de>

Hello,

On Wed, Mar 20, 2013 at 07:50:51PM +0100, Sascha Hauer wrote:
> On Wed, Mar 20, 2013 at 09:32:51AM -0700, Sören Brinkmann wrote:
> > If the caller
> > doesn't like the returned frequency he can request a different one.
> > And he's eventually happy with the return value he calls
> > clk_set_rate() requesting the frequency clk_round_rate() returned.
> > Always rounding down seems a bit odd to me.
> > 
> > Another issue with the current implmentation:
> > clk_divider_round_rate() calls clk_divider_bestdiv(), which uses the ROUND_UP macro, returning a rather low frequency.
> 
> And that is correct. clk_divider_bestdiv is used to calculate the
> maximum parent frequency for which a given divider value does not
> exceed the desired rate.
The reason for that is that the (more?) usual constraint is like: This
mmc card can handle up to 100 MHz. Or this i2c device can handle up to
this and that frequency. Of course there are different constraints, e.g.
for a UART if the target baud speed is 38400 you better run at 38402
than at 19201.

I wonder if it depends on the clock if you want "best approximation <=
requested value" or "best approximation" or on the caller. In the former
case a flag for the clock would be the right thing (as suggested in this
thread). If however it's the caller of round_rate who knows better which
rounding is preferred than better extend the clk API.

Extending the API could just be a convenience function that doesn't
affect the implementations of the clk API. E.g.:

	long clk_round_rate_nearest(struct clk *clk, unsigned long rate)
	{
		long lower_limit = clk_round_rate(clk, rate);
		long upper_limit = clk_round_rate(clk, rate + (rate - lower_limit));

		if (rate - lower_limit < upper_limit - rate)
			return lower_limit;
		else
			return upper_limit;
	}

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

  reply	other threads:[~2013-03-21  9:15 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-30  1:25 [PATCH] clk: divider: Use DIV_ROUND_CLOSEST Soren Brinkmann
2013-01-30  1:25 ` Soren Brinkmann
2013-02-08  2:17 ` Sören Brinkmann
2013-02-08  2:17   ` Sören Brinkmann
2013-03-20  0:16 ` Mike Turquette
2013-03-20 16:32   ` Sören Brinkmann
2013-03-20 16:32     ` Sören Brinkmann
2013-03-20 18:50     ` Sascha Hauer
2013-03-20 18:50       ` Sascha Hauer
2013-03-21  9:15       ` Uwe Kleine-König [this message]
2013-03-21  9:15         ` Uwe Kleine-König
2013-03-26 22:45         ` Sören Brinkmann
2013-03-26 22:45           ` Sören Brinkmann
2013-03-27  1:37           ` Mike Turquette
2013-03-27  1:37             ` Mike Turquette
2013-04-01 23:24             ` Sören Brinkmann
2013-04-01 23:24               ` Sören Brinkmann
2013-04-03  0:38               ` Mike Turquette
2013-03-21 16:36       ` Sören Brinkmann
2013-03-21 16:36         ` Sören Brinkmann
2013-03-25 10:37         ` Sascha Hauer
2013-03-25 10:37           ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130321091531.GN20530@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.