Re: [PATCH 08/25] hw/timer/cmsdk-apb-dualtimer: Add Clock input

[Luc Michel]

On 19:06 Thu 21 Jan     , Peter Maydell wrote:
> As the first step in converting the CMSDK_APB_DUALTIMER device to the
> Clock framework, add a Clock input.  For the moment we do nothing
> with this clock; we will change the behaviour from using the pclk-frq
> property to using the Clock once all the users of this device have
> been converted to wire up the Clock.
> 
> We take the opportunity to correct the name of the clock input to
> match the hardware -- the dual timer names the clock which drives the
> timers TIMCLK.  (It does also have a 'pclk' input, which is used only
> for the register and APB bus logic; on the SSE-200 these clocks are
> both connected together.)
> 
> This is a migration compatibility break for machines mps2-an385,
> mps2-an386, mps2-an500, mps2-an511, mps2-an505, mps2-an521, musca-a,
> musca-b1.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
>  include/hw/timer/cmsdk-apb-dualtimer.h | 3 +++
>  hw/timer/cmsdk-apb-dualtimer.c         | 7 +++++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/timer/cmsdk-apb-dualtimer.h 
> b/include/hw/timer/cmsdk-apb-dualtimer.h
> index 08d9e6fa3d5..3adbb01dd34 100644
> --- a/include/hw/timer/cmsdk-apb-dualtimer.h
> +++ b/include/hw/timer/cmsdk-apb-dualtimer.h
> @@ -17,6 +17,7 @@
>   *
>   * QEMU interface:
>   *  + QOM property "pclk-frq": frequency at which the timer is clocked
> + *  + Clock input "TIMCLK": clock (for both timers)
>   *  + sysbus MMIO region 0: the register bank
>   *  + sysbus IRQ 0: combined timer interrupt TIMINTC
>   *  + sysbus IRO 1: timer block 1 interrupt TIMINT1
> @@ -28,6 +29,7 @@
>  
>  #include "hw/sysbus.h"
>  #include "hw/ptimer.h"
> +#include "hw/clock.h"
>  #include "qom/object.h"
>  
>  #define TYPE_CMSDK_APB_DUALTIMER "cmsdk-apb-dualtimer"
> @@ -62,6 +64,7 @@ struct CMSDKAPBDualTimer {
>      MemoryRegion iomem;
>      qemu_irq timerintc;
>      uint32_t pclk_frq;
> +    Clock *timclk;
>  
>      CMSDKAPBDualTimerModule timermod[CMSDK_APB_DUALTIMER_NUM_MODULES];
>      uint32_t timeritcr;
> diff --git a/hw/timer/cmsdk-apb-dualtimer.c b/hw/timer/cmsdk-apb-dualtimer.c
> index f6534241b94..781b496037b 100644
> --- a/hw/timer/cmsdk-apb-dualtimer.c
> +++ b/hw/timer/cmsdk-apb-dualtimer.c
> @@ -25,6 +25,7 @@
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/registerfields.h"
> +#include "hw/qdev-clock.h"
>  #include "hw/timer/cmsdk-apb-dualtimer.h"
>  #include "migration/vmstate.h"
>  
> @@ -445,6 +446,7 @@ static void cmsdk_apb_dualtimer_init(Object *obj)
>      for (i = 0; i < ARRAY_SIZE(s->timermod); i++) {
>          sysbus_init_irq(sbd, &s->timermod[i].timerint);
>      }
> +    s->timclk = qdev_init_clock_in(DEVICE(s), "TIMCLK", NULL, NULL);
>  }
>  
>  static void cmsdk_apb_dualtimer_realize(DeviceState *dev, Error **errp)
> @@ -485,9 +487,10 @@ static const VMStateDescription 
> cmsdk_dualtimermod_vmstate = {
>  
>  static const VMStateDescription cmsdk_apb_dualtimer_vmstate = {
>      .name = "cmsdk-apb-dualtimer",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
> +        VMSTATE_CLOCK(timclk, CMSDKAPBDualTimer),
>          VMSTATE_STRUCT_ARRAY(timermod, CMSDKAPBDualTimer,
>                               CMSDK_APB_DUALTIMER_NUM_MODULES,
>                               1, cmsdk_dualtimermod_vmstate,
> -- 
> 2.20.1
> 

--