Re: [PATCH-for-9.0? v2 2/8] hw/clock: Pass optional &bool argument to clock_set()

From: Peter Maydell <peter.maydell@linaro.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org,
	"Inès Varhol" <ines.varhol@telecom-paris.fr>,
	"Arnaud Minier" <arnaud.minier@telecom-paris.fr>,
	"Damien Hedde" <damien.hedde@dahe.fr>,
	qemu-arm@nongnu.org, "Alistair Francis" <alistair@alistair23.me>,
	"Luc Michel" <luc@lmichel.fr>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Subject: Re: [PATCH-for-9.0? v2 2/8] hw/clock: Pass optional &bool argument to clock_set()
Date: Mon, 25 Mar 2024 13:47:22 +0000	[thread overview]
Message-ID: <CAFEAcA9aM8J+0RjYnvr8Xr8Q2j3w_TgxHO-gPDn8MaAcAUDynw@mail.gmail.com> (raw)
In-Reply-To: <20240325133259.57235-3-philmd@linaro.org>

On Mon, 25 Mar 2024 at 13:33, Philippe Mathieu-Daudé <philmd@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é <philmd@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).

thanks
-- PMM