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

[Sean Anderson]

On 8/29/20 5:48 PM, Sean Anderson wrote:
> 
> 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.

err, I meant to quote

>  * @round_rate:	Given a target rate as input, returns the closest rate actually
>  *		supported by the clock. The parent rate is an input/output
>  *		parameter.

>> (...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
>