Hi Peter, On 2/9/21 2:20 PM, Peter Maydell wrote: > The Clock framework allows users to specify a callback which is > called after the clock's period has been updated. Some users need to > also have a callback which is called before the clock period is > updated. > > As the first step in adding support for notifying Clock users on > pre-update events, add an argument to the ClockCallback to specify > what event is being notified, and add an argument to the various > functions for registering a callback to specify which events are > of interest to that callback. > > Note that the documentation update renders correct the previously > incorrect claim in 'Adding a new clock' that callbacks "will be > explained in a following section". > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > v1->v2: (suggested by Luc) instead of making callback functions check > whether 'event' is one they are interested in, specify mask of > interesting events at callback registration time. > --- > docs/devel/clocks.rst | 52 +++++++++++++++++++++++++++----- > include/hw/clock.h | 21 +++++++++++-- > include/hw/qdev-clock.h | 17 ++++++++--- > hw/adc/npcm7xx_adc.c | 2 +- > hw/arm/armsse.c | 9 +++--- > hw/char/cadence_uart.c | 4 +-- > hw/char/ibex_uart.c | 4 +-- > hw/char/pl011.c | 5 +-- > hw/core/clock.c | 20 +++++++++--- > hw/core/qdev-clock.c | 8 +++-- > hw/mips/cps.c | 2 +- > hw/misc/bcm2835_cprman.c | 23 ++++++++------ > hw/misc/npcm7xx_clk.c | 26 +++++++++++++--- > hw/misc/npcm7xx_pwm.c | 2 +- > hw/misc/zynq_slcr.c | 5 +-- > hw/timer/cmsdk-apb-dualtimer.c | 5 +-- > hw/timer/cmsdk-apb-timer.c | 4 +-- > hw/timer/npcm7xx_timer.c | 2 +- > hw/watchdog/cmsdk-apb-watchdog.c | 5 +-- > target/mips/cpu.c | 2 +- > 20 files changed, 160 insertions(+), 58 deletions(-)
> diff --git a/include/hw/clock.h b/include/hw/clock.h > index e5f45e2626d..5c73b4e7ae9 100644 > --- a/include/hw/clock.h > +++ b/include/hw/clock.h > @@ -22,7 +22,17 @@ > #define TYPE_CLOCK "clock" > OBJECT_DECLARE_SIMPLE_TYPE(Clock, CLOCK) > > -typedef void ClockCallback(void *opaque); > +/* > + * Argument to ClockCallback functions indicating why the callback > + * has been called. A mask of these values logically ORed together > + * is used to specify which events are interesting when the callback > + * is registered, so these values must all be different bit values. > + */ > +typedef enum ClockEvent { > + ClockUpdate = 1, /* Clock period has just updated */ Just wondering loudly (QEMU doesn't have enum naming conventions), maybe rename ClockUpdate -> ClockUpdateEvent to clarify. > +} ClockEvent; > + > +typedef void ClockCallback(void *opaque, ClockEvent event); > > /* > * clock store a value representing the clock's period in 2^-32ns unit. > @@ -50,6 +60,7 @@ typedef void ClockCallback(void *opaque); > * @canonical_path: clock path string cache (used for trace purpose) > * @callback: called when clock changes > * @callback_opaque: argument for @callback > + * @callback_events: mask of events when callback should be called > * @source: source (or parent in clock tree) of the clock > * @children: list of clocks connected to this one (it is their source) > * @sibling: structure used to form a clock list > @@ -67,6 +78,7 @@ struct Clock { > char *canonical_path; > ClockCallback *callback; > void *callback_opaque; > + int callback_events; Isn't "unsigned" recommended for bit mask? Otherwise (minor Hao Wu's NULL remark): Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>