claims that clk_get_rate() must only be called for enabled clocks. So emit a warning if a consumer calls this function without ensuring the clock being on. Signed-off-by: Uwe Kleine-König --- Hello, I didn't hear back, so went on to create a proper patch now. On Mon, Dec 21, 2020 at 10:27:13AM +0100, 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). This conservative approach is what I implemented now, and I only emit 1 warning to not overflow systems that trigger that problem several times. I'm unsure if I really must take the enable_lock, but it is not completely wrong. drivers/clk/clk.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 8c1d04db990d..7558753883dc 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1614,6 +1614,16 @@ static void __clk_recalc_rates(struct clk_core *core, unsigned long msg) static unsigned long clk_core_get_rate_recalc(struct clk_core *core) { + unsigned long flags; + unsigned int enable_count; + + flags = clk_enable_lock(); + enable_count = core->enable_count; + clk_enable_unlock(flags); + + WARN_ONCE(enable_count == 0, + "A clock must be enabled to determine its rate\n"); + if (core && (core->flags & CLK_GET_RATE_NOCACHE)) __clk_recalc_rates(core, 0); -- 2.29.2 > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ | -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |