clk: Update comment for clk_round_rate()
Subsystems (2)
PATCH 1/1 clk: Update comment for clk_round_rate()
From: Viresh Kumar <[email protected]>
clk_round_rate() must not provide rate greater than requested rate, as hardware may not work at it. This wasn't clear from documentation of clk_round_rate().
This patch updates documentation for this.
---
Mike,
I don't know if this holds true for common clock framework or not? But it looks more logical to me and this is how it was implemented for SPEAr.
drivers/clk/clk.c | 6 +++--- include/linux/clk.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-)
drivers/clk/clk.c +0 -0
@@ -603,9 +603,9 @@ unsigned long __clk_round_rate(struct clk *clk,
unsigned long rate)
* @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 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.
+ * Takes in a rate as input and rounds it to a rate <= input 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(struct clk *clk, unsigned long rate)
{
include/linux/clk.h +2 -1
@@ -213,7 +213,8 @@ void clk_put(struct clk *clk);/** -* clk_round_rate - adjust a rate to the exact rate a clock can provide +* clk_round_rate - adjust a rate to the exact rate a clock can provide. Output +* clock rate should be <= requested rate * @clk: clock source * @rate: desired clock rate in Hz *
Signature · 1 line
1.7.9
PATCH 1/1 clk: Update comment for clk_round_rate()
Quoted 7 lines · viresh kumar
From: Viresh Kumar <[email protected]>
clk_round_rate()must not provide rate greater than requested rate, as hardware may not work at it. This wasn't clear from documentation ofclk_round_rate().This patch updates documentation for this.
Quoted 7 lines
Signed-off-by Viresh Kumar <[email protected]>---
Mike,
I don't know if this holds true for common clock framework or not? But it looks more logical to me and this is how it was implemented for SPEAr.
The common clk framework doesn't enforce any policy like this, nor do I think it should. The clk framework is far from complete and I wouldn't be surprised if we see folks who want their rounded rate to represent a minimum value (instead of a maximum as your patch states).
This might be achieved later on by a platform-specific or
framework-wide flag, similar to how CPUfreq does it today (CPUFREQ_RELATION_L and CPUFREQ_RELATION_H).
Thank you for submitting the patch, but for now it's a NACK.
Regards,
Mike
Quoted 44 lines
?drivers/clk/clk.c ? | ? ?6 +++---
?include/linux/clk.h | ? ?3 ++-
?2 files changed, 5 insertions(+), 4 deletions(-)drivers/clk/clk.c +0
-0@@ -603,9 +603,9 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned long rate)?* @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 a rate that the clk can actually
- * use which is then returned. ?If clk doesn't supportround_rateoperation
- * then the parent rate is returned.
+ * Takes in a rate as input and rounds it to a rate <= input rate that the clk
+ * can actually use which is then returned. ?If clk doesn't supportround_rate
+ * operation then the parent rate is returned.
?*/
?longclk_round_rate(struct clk *clk, unsigned long rate)
?{include/linux/clk.h +0
-0@@ -213,7 +213,8 @@ void clk_put(struct clk *clk);?/**
- *clk_round_rate- adjust a rate to the exact rate a clock can provide
+ *clk_round_rate- adjust a rate to the exact rate a clock can provide. Output
+ * ? ? ? ? ? ? ? ? clock rate should be <= requested rate
?* @clk: clock source
?* @rate: desired clock rate in Hz
?*
--
1.7.9_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 clk: Update comment for clk_round_rate()
Quoted 4 lines · Mike
The common clk framework doesn't enforce any policy like this, nor do I think it should. The clk framework is far from complete and I wouldn't be surprised if we see folks who want their rounded rate to represent a minimum value (instead of a maximum as your patch states).
Ok. Just for example, suppose foo_clk can have following rates:
100, 110, 120, 130, 140.
And we call round_rate with a value of 126.
Now i can't visualize why would anybody want it to return 130 (above the limits requested)? I agree both 100 and 120 can be returned, based on your below logic.
Quoted 6 lines
This might be achieved later on by a platform-specific or
framework-wide flag, similar to how CPUfreq does it today (CPUFREQ_RELATION_LandCPUFREQ_RELATION_H).Thank you for submitting the patch, but for now it's a NACK.
No Probs.
Signature · 1 line
viresh
PATCH 1/1 clk: Update comment for clk_round_rate()
Hello,
Quoted 6 lines · Viresh Kumar
Quoted 4 lines · Mike
The common clk framework doesn't enforce any policy like this, nor do I think it should. The clk framework is far from complete and I wouldn't be surprised if we see folks who want their rounded rate to represent a minimum value (instead of a maximum as your patch states).
Quoted 8 lines
Ok. Just for example, suppose
foo_clkcan have following rates:
100, 110, 120, 130, 140.And we call
round_ratewith a value of 126.Now i can't visualize why would anybody want it to return 130 (above the limits requested)? I agree both 100 and 120 can be returned, based on your below logic.
It depends on your scenario. E.g. for an UART clock it might be better to choose 130. Here choosing the frequency is not about a fixed maximum but to match the sample rate of the device connect to your UART.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/1 clk: Update comment for clk_round_rate()
Quoted 3 lines · Uwe Kleine-K?nig
It depends on your scenario. E.g. for an UART clock it might be better to choose 130. Here choosing the frequency is not about a fixed maximum but to match the sample rate of the device connect to your UART.
My point was: we can almost always guarantee that device will still be operational at any frequency below the one requested. But it may not work at any freq above the requested one. So, giving 130 here may break it.
Signature · 1 line
viresh
PATCH 1/1 clk: Update comment for clk_round_rate()
Quoted 9 lines · Viresh Kumar
Quoted 3 lines · Uwe Kleine-K?nig
It depends on your scenario. E.g. for an UART clock it might be better to choose 130. Here choosing the frequency is not about a fixed maximum but to match the sample rate of the device connect to your UART.
My point was: we can almost always guarantee that device will still be operational at any frequency below the one requested. But it may not work at any freq above the requested one. So, giving 130 here may break it.
It depends. If for satisfying a request for 115200 Bd you have the choice between 115202 Bd and 57601 Bd, I'd recommend the former.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |