Thread (9 messages) 9 messages, 4 authors, 2021-11-12

Re: [PATCH] clk: Warn when clk_get_rate is called for a disabled clk

From: Stephen Boyd <sboyd@kernel.org>
Date: 2021-02-13 01:15:48

Quoting Uwe (2021-02-11 00:05:09)
Hello Stephen,

On Wed, Feb 10, 2021 at 07:14:09PM -0800, Stephen Boyd wrote:
quoted
What problem are you trying to address? Is there some issue you've
encountered in the kernel that would have been fixed by having this
warning?
The warning obviously doesn't fix anything. My eventual goal is to
answer the question in the initial mail in this thread. The motivating
situation is: Should I continue to tell patch authors who use
clk_get_rate() that they have to ensure that the given clk must be
enabled as the documentation suggests? And if yes: Can we please check
this automatically (e.g. with my patch or by returning 0 for a disabled
clk) and don't rely on human review to adhere to this rule.
I suggest to stop telling folks that they must enable the clk before
getting the rate. The documentation says

 "This is only valid once the clock source has been enabled"

which is really ambiguous. What is "this?" supposed to be? Is it trying
to say the clk isn't toggling at the frequency until it is enabled, so
the rate isn't valid until it is toggling? Or is it saying that the
return value isn't valid until the clk is enabled?  It's been there for
a long time, in fact since Russell introduced the header file on arm in
2004[1]. I'm inclined to read it as:

 "The clk won't be toggling at the rate returned by this function until
  the clk is enabled by clk_enable()"

Maybe send a documentation update instead?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/include/asm-arm/hardware/clock.h?id=0471b5fb91816b448f10e43f922b8a5e264fe039
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help