On 8/29/20 5:20 PM, Simon Glass wrote:
> Hi Dario,
> 
> +Stephen Warren
> On Tue, 25 Aug 2020 at 03:24, Dario Binacchi <dario...@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 <dario...@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 <s...@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

Reply via email to