| Message ID | 1400106655-22465-3-git-send-email-soren.brinkmann@xilinx.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
Hello, it's great you pick that up. On Wed, May 14, 2014 at 03:30:52PM -0700, Soren Brinkmann wrote: > Introduce a new API function to round a rate to the closest possible > rate the HW clock can generate. > In contrast to 'clk_round_rate()' which works similar, but always returns > a frequency <= its input rate. > > The code comes from Uwe and was copied from this LKML thread: > https://lkml.org/lkml/2013/3/21/115 > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com> > --- > > drivers/clk/clk.c | 26 ++++++++++++++++++++++++-- > include/linux/clk.h | 14 ++++++++++++-- > 2 files changed, 36 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index dff0373f53c1..b715f5a9826c 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1011,8 +1011,9 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned long rate) > * @rate: the rate which is to be rounded > * > * Takes in a rate as input and rounds it to a rate that the clk can actually > - * use which is then returned. If clk doesn't support round_rate operation > - * then the parent rate is returned. > + * use and does not exceed the requested frequency, which is then returned. > + * If clk doesn't support round_rate operation then the parent rate > + * is returned. > */ > long clk_round_rate(struct clk *clk, unsigned long rate) > { > @@ -1027,6 +1028,27 @@ long clk_round_rate(struct clk *clk, unsigned long rate) > EXPORT_SYMBOL_GPL(clk_round_rate); > > /** > + * clk_round_rate_nearest - round the given rate for a clk > + * @clk: the clk for which we are rounding a rate > + * @rate: the rate which is to be rounded > + * > + * Takes in a rate as input and rounds it to the closest rate that the clk > + * can actually use which is then returned. If clk doesn't support > + * round_rate operation then the parent rate is returned. > + */ > +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; I wanted to suggest to add some comment to describe why the calculation works here. While trying to proove it, I noticed that this implementation is buggy. Consider a clock that can provide the following frequencies: 38000, 38401, 38600. clk_round_rate_nearest(clk, 38400) lower_limit = clk_round_rate(clk, 38400) -> 38000 upper_limit = clk_round_rate(clk, 38800) -> 38600 return 38600 but 38401 would have been the better/correct answer. I think you cannot implement clk_round_rate_nearest without iteration if you don't want to add specific logic to the clock providers. Best regards Uwe
Hi Uwe, On Thu, 2014-05-15 at 09:38AM +0200, Uwe Kleine-König wrote: > Hello, > > it's great you pick that up. > > On Wed, May 14, 2014 at 03:30:52PM -0700, Soren Brinkmann wrote: > > Introduce a new API function to round a rate to the closest possible > > rate the HW clock can generate. > > In contrast to 'clk_round_rate()' which works similar, but always returns > > a frequency <= its input rate. > > > > The code comes from Uwe and was copied from this LKML thread: > > https://lkml.org/lkml/2013/3/21/115 > > > > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com> > > --- > > > > drivers/clk/clk.c | 26 ++++++++++++++++++++++++-- > > include/linux/clk.h | 14 ++++++++++++-- > > 2 files changed, 36 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index dff0373f53c1..b715f5a9826c 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -1011,8 +1011,9 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned long rate) > > * @rate: the rate which is to be rounded > > * > > * Takes in a rate as input and rounds it to a rate that the clk can actually > > - * use which is then returned. If clk doesn't support round_rate operation > > - * then the parent rate is returned. > > + * use and does not exceed the requested frequency, which is then returned. > > + * If clk doesn't support round_rate operation then the parent rate > > + * is returned. > > */ > > long clk_round_rate(struct clk *clk, unsigned long rate) > > { > > @@ -1027,6 +1028,27 @@ long clk_round_rate(struct clk *clk, unsigned long rate) > > EXPORT_SYMBOL_GPL(clk_round_rate); > > > > /** > > + * clk_round_rate_nearest - round the given rate for a clk > > + * @clk: the clk for which we are rounding a rate > > + * @rate: the rate which is to be rounded > > + * > > + * Takes in a rate as input and rounds it to the closest rate that the clk > > + * can actually use which is then returned. If clk doesn't support > > + * round_rate operation then the parent rate is returned. > > + */ > > +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; > I wanted to suggest to add some comment to describe why the calculation > works here. While trying to proove it, I noticed that this > implementation is buggy. > Consider a clock that can provide the following frequencies: 38000, > 38401, 38600. > > clk_round_rate_nearest(clk, 38400) > lower_limit = clk_round_rate(clk, 38400) -> 38000 > upper_limit = clk_round_rate(clk, 38800) -> 38600 > > return 38600 > > but 38401 would have been the better/correct answer. I think you cannot > implement clk_round_rate_nearest without iteration if you don't want to > add specific logic to the clock providers. You're right. Apparently we haven't had such a case. Anyway, this needs to be implemented differently. Thanks, Sören -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index dff0373f53c1..b715f5a9826c 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1011,8 +1011,9 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned long rate) * @rate: the rate which is to be rounded * * Takes in a rate as input and rounds it to a rate that the clk can actually - * use which is then returned. If clk doesn't support round_rate operation - * then the parent rate is returned. + * use and does not exceed the requested frequency, which is then returned. + * If clk doesn't support round_rate operation then the parent rate + * is returned. */ long clk_round_rate(struct clk *clk, unsigned long rate) { @@ -1027,6 +1028,27 @@ long clk_round_rate(struct clk *clk, unsigned long rate) EXPORT_SYMBOL_GPL(clk_round_rate); /** + * clk_round_rate_nearest - round the given rate for a clk + * @clk: the clk for which we are rounding a rate + * @rate: the rate which is to be rounded + * + * Takes in a rate as input and rounds it to the closest rate that the clk + * can actually use which is then returned. If clk doesn't support + * round_rate operation then the parent rate is returned. + */ +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; +} +EXPORT_SYMBOL_GPL(clk_round_rate_nearest); + +/** * __clk_notify - call clk notifier chain * @clk: struct clk * that is changing rate * @msg: clk notifier type (see include/linux/clk.h) diff --git a/include/linux/clk.h b/include/linux/clk.h index fb5e097d8f72..2f83bf030ac6 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -255,15 +255,25 @@ void devm_clk_put(struct device *dev, struct clk *clk); /** - * clk_round_rate - adjust a rate to the exact rate a clock can provide + * clk_round_rate - round a rate to the exact rate a clock can provide not + * exceeding @rate * @clk: clock source * @rate: desired clock rate in Hz * - * Returns rounded clock rate in Hz, or negative errno. + * Returns rounded clock rate in Hz, or parent rate */ long clk_round_rate(struct clk *clk, unsigned long rate); /** + * clk_round_rate_nearest - round a rate to the exact rate a clock can provide + * @clk: the clk for which we are rounding a rate + * @rate: the rate which is to be rounded + * + * Returns rounded clock rate in Hz, or parent rate + */ +long clk_round_rate_nearest(struct clk *clk, unsigned long rate); + +/** * clk_set_rate - set the clock rate for a clock source * @clk: clock source * @rate: desired clock rate in Hz
Introduce a new API function to round a rate to the closest possible rate the HW clock can generate. In contrast to 'clk_round_rate()' which works similar, but always returns a frequency <= its input rate. The code comes from Uwe and was copied from this LKML thread: https://lkml.org/lkml/2013/3/21/115 Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com> --- drivers/clk/clk.c | 26 ++++++++++++++++++++++++-- include/linux/clk.h | 14 ++++++++++++-- 2 files changed, 36 insertions(+), 4 deletions(-)