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