[PATCH 04/31] clk: add clk_round_rate()

[Sean Anderson]

On 8/29/20 5:20 PM, Simon Glass wrote:
> Hi Dario,
> 
> +Stephen Warren
> On Tue, 25 Aug 2020 at 03:24, Dario Binacchi <dariobin at libero.it> wrote:
>>
>> It returns the rate which will be set if you ask clk_set_rate() to set
>> that rate. It provides a way to query exactly what rate you'll get if
>> you call clk_set_rate() with that same argument.
>> So essentially, clk_round_rate() and clk_set_rate() are equivalent
>> except the former does not modify the clock hardware in any way.
>>
>> Signed-off-by: Dario Binacchi <dariobin at libero.it>
>> ---
>>
>>  arch/sandbox/include/asm/clk.h |  9 +++++++++
>>  drivers/clk/clk-uclass.c       | 15 +++++++++++++++
>>  drivers/clk/clk_sandbox.c      | 17 +++++++++++++++++
>>  drivers/clk/clk_sandbox_test.c | 10 ++++++++++
>>  include/clk-uclass.h           |  8 ++++++++
>>  include/clk.h                  | 29 +++++++++++++++++++++++++++++
>>  test/dm/clk.c                  | 22 ++++++++++++++++++++++
>>  7 files changed, 110 insertions(+)
>>
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> But I wonder if we should change the set_rate() uclass interface to
> have a flag value, one of the flags being 'dry run' which doesn't
> actually set the value?
> 
> You would still have the same call to the uclass functions
> clk_set_rate() and clk_round_rate() but the driver API would implement
> both with calls to set_rate()?

Linux uses separate clk_ops functions for this purpose

>  * @recalc_rate	Recalculate the rate of this clock, by querying hardware. The
>  *		parent rate is an input parameter.  It is up to the caller to
>  *		ensure that the prepare_mutex is held across this call.
>  *		Returns the calculated rate.  Optional, but recommended - if
>  *		this op is not set then clock rate will be initialized to 0.
> (...snip...)
> 	long		(*round_rate)(struct clk_hw *hw, unsigned long rate,
> 					unsigned long *parent_rate);

Besides matching their interface, I think there is good reason for
keeping these functions separate. Existing clock drivers would need to
be rewritten so they don't set the clock rate when you just want to do a
dry run. This way, it's very clear when a driver supports recalc_rate.

--Sean