On Sat, 23 Jan 2021 at 20:30, Luc Michel <l...@lmichel.fr> wrote:
>
> On 19:06 Thu 21 Jan , Peter Maydell wrote:
> > Use the MAINCLK Clock input to set the system_clock_scale variable
> > rather than using the mainclk_frq property.
> >
> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> > ---
> > At some point we should make the SysTick take a Clock itself so
> > that we can get rid of the system_clock_scale global entirely.
> > (In fact we want two Clocks: one that is the CPU clock and one
> > for the 'external reference clock' whose period is currently
> > hardcoded at 1000ns in systick_scale()...)
> > ---
> > hw/arm/armsse.c | 21 +++++++++++++++++----
> > 1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> > index 4349ce9bfdb..1da0c1be4c7 100644
> > --- a/hw/arm/armsse.c
> > +++ b/hw/arm/armsse.c
> > @@ -232,6 +232,16 @@ static void armsse_forward_sec_resp_cfg(ARMSSE *s)
> > qdev_connect_gpio_out(dev_splitter, 2, s->sec_resp_cfg_in);
> > }
> >
> > +static void armsse_mainclk_update(void *opaque)
> > +{
> > + ARMSSE *s = ARM_SSE(opaque);
> > + /*
> > + * Set system_clock_scale from our Clock input; this is what
> > + * controls the tick rate of the CPU SysTick timer.
> > + */
> > + system_clock_scale = clock_ticks_to_ns(s->mainclk, 1);
> > +}
> > +
> I think you forgot to connect this callback to the clock itself (the
> `qdev_init_clock_in` call in `armsse_init`).
Whoops, yes :-) As it happens all ARMSSE users are fixed-frequency
so this doesn't make a guest-visible difference, but it was
definitely my intention that this was a clock update callback.
> Moreover on a clock change event, shouldn't the SysTick timer be
> recomputed?
Systick correctly handles the system_clock_scale global being changed
at runtime, so once this callback is actually wired up to its Clock
that should just work.
thanks
-- PMM
