On 19:05 Thu 21 Jan , Peter Maydell wrote:
> The ptimer API currently provides two methods for setting the period:
> ptimer_set_period(), which takes a period in nanoseconds, and
> ptimer_set_freq(), which takes a frequency in Hz. Neither of these
> lines up nicely with the Clock API, because although both the Clock
> and the ptimer track the frequency using a representation of whole
> and fractional nanoseconds, conversion via either period-in-ns or
> frequency-in-Hz will introduce a rounding error.
>
> Add a new function ptimer_set_period_from_clock() which takes the
> Clock object directly to avoid the rounding issues. This includes a
> facility for the user to specify that there is a frequency divider
> between the Clock proper and the timer, as some timer devices like
> the CMSDK APB dualtimer need this.
>
> To avoid having to drag in clock.h from ptimer.h we add the Clock
> type to typedefs.h.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Luc Michel <l...@lmichel.fr>
> ---
> Side note, I forget why we didn't go for 64.32 fixedpoint for the
> Clock too. I kinda feel we might run into the "clocks can't handle
> periods greater than 4 seconds" limit some day. Hopefully we can
> backwards-compatibly expand it if that day ever comes...
>
> The 'divisor' functionality seemed like the simplest way to get
> to what I needed for the dualtimer; perhaps we should think about
> whether we can have generic lightweight support for clock
> frequency divider/multipliers...
> ---
> include/hw/ptimer.h | 22 ++++++++++++++++++++++
> include/qemu/typedefs.h | 1 +
> hw/core/ptimer.c | 34 ++++++++++++++++++++++++++++++++++
> 3 files changed, 57 insertions(+)
>
> diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
> index 412763fffb2..c443218475b 100644
> --- a/include/hw/ptimer.h
> +++ b/include/hw/ptimer.h
> @@ -165,6 +165,28 @@ void ptimer_transaction_commit(ptimer_state *s);
> */
> void ptimer_set_period(ptimer_state *s, int64_t period);
>
> +/**
> + * ptimer_set_period_from_clock - Set counter increment from a Clock
> + * @s: ptimer to configure
> + * @clk: pointer to Clock object to take period from
> + * @divisor: value to scale the clock frequency down by
> + *
> + * If the ptimer is being driven from a Clock, this is the preferred
> + * way to tell the ptimer about the period, because it avoids any
> + * possible rounding errors that might happen if the internal
> + * representation of the Clock period was converted to either a period
> + * in ns or a frequency in Hz.
> + *
> + * If the ptimer should run at the same frequency as the clock,
> + * pass 1 as the @divisor; if the ptimer should run at half the
> + * frequency, pass 2, and so on.
> + *
> + * This function will assert if it is called outside a
> + * ptimer_transaction_begin/commit block.
> + */
> +void ptimer_set_period_from_clock(ptimer_state *s, const Clock *clock,
> + unsigned int divisor);
> +
> /**
> * ptimer_set_freq - Set counter frequency in Hz
> * @s: ptimer to configure
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 976b529dfb5..68deb74ef6f 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -34,6 +34,7 @@ typedef struct BlockDriverState BlockDriverState;
> typedef struct BusClass BusClass;
> typedef struct BusState BusState;
> typedef struct Chardev Chardev;
> +typedef struct Clock Clock;
> typedef struct CompatProperty CompatProperty;
> typedef struct CoMutex CoMutex;
> typedef struct CPUAddressSpace CPUAddressSpace;
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index 2aa97cb665c..6ba19fd9658 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -15,6 +15,7 @@
> #include "sysemu/qtest.h"
> #include "block/aio.h"
> #include "sysemu/cpus.h"
> +#include "hw/clock.h"
>
> #define DELTA_ADJUST 1
> #define DELTA_NO_ADJUST -1
> @@ -348,6 +349,39 @@ void ptimer_set_period(ptimer_state *s, int64_t period)
> }
> }
>
> +/* Set counter increment interval from a Clock */
> +void ptimer_set_period_from_clock(ptimer_state *s, const Clock *clk,
> + unsigned int divisor)
> +{
> + /*
> + * The raw clock period is a 64-bit value in units of 2^-32 ns;
> + * put another way it's a 32.32 fixed-point ns value. Our internal
> + * representation of the period is 64.32 fixed point ns, so
> + * the conversion is simple.
> + */
> + uint64_t raw_period = clock_get(clk);
> + uint64_t period_frac;
> +
> + assert(s->in_transaction);
> + s->delta = ptimer_get_count(s);
> + s->period = extract64(raw_period, 32, 32);
> + period_frac = extract64(raw_period, 0, 32);
> + /*
> + * divisor specifies a possible frequency divisor between the
> + * clock and the timer, so it is a multiplier on the period.
> + * We do the multiply after splitting the raw period out into
> + * period and frac to avoid having to do a 32*64->96 multiply.
> + */
> + s->period *= divisor;
> + period_frac *= divisor;
> + s->period += extract64(period_frac, 32, 32);
> + s->period_frac = (uint32_t)period_frac;
> +
> + if (s->enabled) {
> + s->need_reload = true;
> + }
> +}
> +
> /* Set counter frequency in Hz. */
> void ptimer_set_freq(ptimer_state *s, uint32_t freq)
> {
> --
> 2.20.1
>
--
