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

[Dario Binacchi]

> Il 29/08/2020 23:50 Sean Anderson <seanga2 at gmail.com> ha scritto:
> 
>  
> 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.

I agree.

> > 
> > --Sean
> >