On Mon, 25 Mar 2024 at 14:39, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > On 25/3/24 14:47, Peter Maydell wrote: > > On Mon, 25 Mar 2024 at 13:33, Philippe Mathieu-Daudé <phi...@linaro.org> > > wrote: > >> > >> Currently clock_set() returns whether the clock has > >> been changed or not. In order to combine this information > >> with other clock calls, pass an optional boolean and do > >> not return anything. The single caller ignores the return > >> value, have it use NULL. > >> > >> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> > >> --- > >> include/hw/clock.h | 22 ++++++++++++++++------ > >> hw/core/clock.c | 8 +++++--- > >> hw/misc/bcm2835_cprman.c | 2 +- > >> hw/misc/zynq_slcr.c | 4 ++-- > >> 4 files changed, 24 insertions(+), 12 deletions(-) > >> > >> diff --git a/include/hw/clock.h b/include/hw/clock.h > >> index bb12117f67..474bbc07fe 100644 > >> --- a/include/hw/clock.h > >> +++ b/include/hw/clock.h > >> @@ -180,21 +180,28 @@ static inline bool clock_has_source(const Clock *clk) > >> * clock_set: > >> * @clk: the clock to initialize. > >> * @value: the clock's value, 0 means unclocked > >> + * @changed: set to true if the clock is changed, ignored if set to NULL. > >> * > >> * Set the local cached period value of @clk to @value. > >> - * > >> - * @return: true if the clock is changed. > >> */ > >> -bool clock_set(Clock *clk, uint64_t value); > >> +void clock_set(Clock *clk, uint64_t period, bool *changed); > > > > What's wrong with using the return value? Generally > > returning a value via passing in a pointer is much > > clunkier in C than using the return value, so we only > > do it if we have to (e.g. the return value is already > > being used for something else, or we need to return > > more than one thing at once). > > Then I'd rather remove (by inlining) the clock_update*() methods, > to have explicit calls to clock_propagate(), after multiple > clock_set*() calls.
You mean, so that we handle "set the clock period" and "set the mul/div" the same way, by just setting them and making it always the caller's responsibility to call clock_propagate() ? Would you keep the bool return for clock_set and clock_set_mul_div to tell the caller whether a clock_propagate() call is needed ? thanks -- PMM
