| Message ID | 20210121190622.22000-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert CMSDK timer, watchdog, dualtimer to Clock framework | expand |
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.maydell@linaro.org> Reviewed-by: Luc Michel <luc@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 > --
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) {
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.maydell@linaro.org> --- 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(+) -- 2.20.1