Re: [PATCH for-6.2 05/25] hw/timer/armv7m_systick: Add input clocks

[Alistair Francis]

On Thu, Aug 12, 2021 at 7:36 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The v7M systick timer can be programmed to run from either of
> two clocks:
>  * an "external reference clock" (when SYST_CSR.CLKSOURCE == 0)
>  * the main CPU clock (when SYST_CSR.CLKSOURCE == 1)
>
> Our implementation currently hardwires the external reference clock
> to be 1MHz, and allows boards to set the main CPU clock frequency via
> the global 'system_clock_scale'.  (Most boards set that to a constant
> value; the Stellaris boards allow the guest to reprogram it via the
> board-specific RCC registers).
>
> As the first step in converting this to use the Clock infrastructure,
> add input clocks to the systick device for the reference clock and
> the CPU clock.  The device implementation ignores them; once we have
> made all the users of the device correctly wire up the new Clocks we
> will switch the implementation to use them and ignore the old
> system_clock_scale.
>
> This is a migration compat break for all M-profile boards, because of
> the addition of the new clock objects to the vmstate struct.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/timer/armv7m_systick.h |  7 +++++++
>  hw/timer/armv7m_systick.c         | 10 ++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/timer/armv7m_systick.h 
> b/include/hw/timer/armv7m_systick.h
> index 685fc5bc0d7..38adf8d274e 100644
> --- a/include/hw/timer/armv7m_systick.h
> +++ b/include/hw/timer/armv7m_systick.h
> @@ -15,6 +15,7 @@
>  #include "hw/sysbus.h"
>  #include "qom/object.h"
>  #include "hw/ptimer.h"
> +#include "hw/clock.h"
>
>  #define TYPE_SYSTICK "armv7m_systick"
>
> @@ -25,6 +26,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(SysTickState, SYSTICK)
>   *  + sysbus MMIO region 0 is the register interface (covering
>   *    the registers which are mapped at address 0xE000E010)
>   *  + sysbus IRQ 0 is the interrupt line to the NVIC
> + *  + Clock input "refclk" is the external reference clock
> + *    (used when SYST_CSR.CLKSOURCE == 0)
> + *  + Clock input "cpuclk" is the main CPU clock
> + *    (used when SYST_CSR.CLKSOURCE == 1)
>   */
>
>  struct SysTickState {
> @@ -38,6 +43,8 @@ struct SysTickState {
>      ptimer_state *ptimer;
>      MemoryRegion iomem;
>      qemu_irq irq;
> +    Clock *refclk;
> +    Clock *cpuclk;
>  };
>
>  /*
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> index 2f192011eb0..e43f74114e8 100644
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -14,6 +14,7 @@
>  #include "migration/vmstate.h"
>  #include "hw/irq.h"
>  #include "hw/sysbus.h"
> +#include "hw/qdev-clock.h"
>  #include "qemu/timer.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> @@ -201,6 +202,9 @@ static void systick_instance_init(Object *obj)
>      memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick", 0xe0);
>      sysbus_init_mmio(sbd, &s->iomem);
>      sysbus_init_irq(sbd, &s->irq);
> +
> +    s->refclk = qdev_init_clock_in(DEVICE(obj), "refclk", NULL, NULL, 0);
> +    s->cpuclk = qdev_init_clock_in(DEVICE(obj), "cpuclk", NULL, NULL, 0);
>  }
>
>  static void systick_realize(DeviceState *dev, Error **errp)
> @@ -215,9 +219,11 @@ static void systick_realize(DeviceState *dev, Error 
> **errp)
>
>  static const VMStateDescription vmstate_systick = {
>      .name = "armv7m_systick",
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> +    .version_id = 3,
> +    .minimum_version_id = 3,
>      .fields = (VMStateField[]) {
> +        VMSTATE_CLOCK(refclk, SysTickState),
> +        VMSTATE_CLOCK(cpuclk, SysTickState),
>          VMSTATE_UINT32(control, SysTickState),
>          VMSTATE_INT64(tick, SysTickState),
>          VMSTATE_PTIMER(ptimer, SysTickState),
> --
> 2.20.1
>
>