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

On Fri, Feb 12, 2021 at 05:14:36PM -0800, Stephen Boyd wrote:

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

quoted

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?

Please Cc me on CLK API matters, as per my entry in MAINTAINERS.

The subject is the clk_get_rate() function, and as it is in the
section describing that function, I don't see it is ambiguous.

From what I remember, the restriction came after some discussion,
and the problem that on some platforms, the clock tree would not
be known prior to the clock being enabled (at that time, which
was before the "prepare" stuff got added.) Consequently, because
the child/parent relationships were not known, and PLLs were not
initialised, the rate that a particular clock would be ticking
could be different before and after it being enabled.

For this reason, it was decided that the only sensible approach
was to declare that the return value of clk_get_rate() on a
disabled clock is undefined.

That said, finding the discussion is proving difficult, so I may
be misremembering.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!