Does clk_core_round_rate_nolock() really do the right thing?

[Jeffrey Hugo <jhugo@codeaurora.org>]

"clk: rework calls to round and determine rate callbacks" [1] added 
clk_core_round_rate_nolock().

I question that it does the right thing in a specific scenario.

Lets assume we have a clock that is currently running at 5Mhz, but we 
want to have it run at 400Khz, and the clock supports both rates. 
Perhaps the clock is for a hardware block, where in to switch 
operational modes, the clock frequency is expected to run at the lower 
frequency.

The driver may call clk_set_rate() to do so, resulting in the following 
call flow (based on 4.20-rc2)-

clk_set_rate()
clk_core_set_rate_nolock()
clk_core_req_round_rate_nolock()
clk_core_round_rate_nolock()

In clk_core_round_rate_nolock(), lets assume the conditionals do not 
apply (clk core cannot round, and set rate parent flag isn't set).  In 
that case, the requested rate is modified by this line -

req->rate = core->rate;

This transforms the request from 400Khz to 5Mhz (the current clock rate).

What will then end up happening is we return from 
clk_core_round_rate_nolock(), through clk_core_req_round_rate_nolock() 
back to clk_core_set_rate_nolock() where we check to see if the rate 
returned from the "rounding" is the same as the current clock rate (5Mhz 
== 5Mhz).  Thus we conclude there is nothing to be done, and we return 
success back up to the caller having not actually touched the clock or 
set the requested rate.

On the face of it, this seems incorrect to me, although I am a neophyte 
to the clock framework.  Is this really the expected behavior, to leave 
the clock at the current rate even though the requested rate is 
different?  If so, why?


[1] - 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0f6cc2b8e94da5400528c0ba7fd910392ec598a2

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.