> Il 29/08/2020 23:50 Sean Anderson <[email protected]> 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 <[email protected]> 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 <[email protected]> > >>> --- > >>> > >>> 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 <[email protected]> > >> > >> 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 > >
