On 12/21/20 10:27 AM, Uwe Kleine-König wrote:
> the documentation about clk_get_rate in include/linux/clk.h reads:
> 
> 	[...] obtain the current clock rate (in Hz) for a clock source.
> 	This is only valid once the clock source has been enabled.
> 
> The second part isn't enforced and (I think) there are many consumers
> who don't ensure the clock being enabled. (I just stumbled over
> rockchip_pwm_get_state().)
> 
> I wonder if it would be sensible to add a development check to
> clk_get_rate, something like:
> 
> 	if (WARN(!clk->usecount, "Trying to get rate of a disabled clk"))
> 		return 0;
> 
> (or something less consequent like not returning 0 but the value it also
> returns today). Or is the statement in the comment wrong today and it
> can be assumed that clk_get_rate() also works for a disabled clock (and
> yields the rate it would have were it enabled)?

What about the problem, that if a different clock (which has a common parent
with the clock in question) is enabled and sets the rate, so that our clock's
rate would changes? Does it make a difference if our clock is enabled or not?

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |