diff mbox series

[PATCH-for-6.1] hw/avr/atmega: Convert to QDev Clock API

Message ID 20210409112555.2430933-1-f4bug@amsat.org (mailing list archive)
State New
Headers show
Series [PATCH-for-6.1] hw/avr/atmega: Convert to QDev Clock API | expand

Commit Message

Philippe Mathieu-Daudé April 9, 2021, 11:25 a.m. UTC
Create the oscillator object on the board, and propagate its
clock to the ATMega MCU. Wire this clock to the timers.

Properties are remplaced by clocks.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Based-on: <20210409062401.2350436-1-f4bug@amsat.org>

TODO: Add corresponding vmstates for migration,
      but these devices don't have any, so keep
      this for later.
---
 hw/avr/atmega.h                |  3 ++-
 include/hw/timer/avr_timer16.h |  4 +++-
 hw/avr/arduino.c               |  8 ++++++--
 hw/avr/atmega.c                | 20 +++++++++++---------
 hw/timer/avr_timer16.c         | 20 ++++++++++++--------
 5 files changed, 34 insertions(+), 21 deletions(-)

Comments

Philippe Mathieu-Daudé April 9, 2021, 11:33 a.m. UTC | #1
On 4/9/21 1:25 PM, Philippe Mathieu-Daudé wrote:
> Create the oscillator object on the board, and propagate its
> clock to the ATMega MCU. Wire this clock to the timers.
> 
> Properties are remplaced by clocks.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Based-on: <20210409062401.2350436-1-f4bug@amsat.org>
> 
> TODO: Add corresponding vmstates for migration,
>       but these devices don't have any, so keep
>       this for later.
> ---
>  hw/avr/atmega.h                |  3 ++-
>  include/hw/timer/avr_timer16.h |  4 +++-
>  hw/avr/arduino.c               |  8 ++++++--
>  hw/avr/atmega.c                | 20 +++++++++++---------
>  hw/timer/avr_timer16.c         | 20 ++++++++++++--------
>  5 files changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e1..12e856386b6 100644
> --- a/hw/avr/atmega.h
> +++ b/hw/avr/atmega.h
> @@ -11,6 +11,7 @@
>  #ifndef HW_AVR_ATMEGA_H
>  #define HW_AVR_ATMEGA_H
>  
> +#include "hw/clock.h"
>  #include "hw/char/avr_usart.h"
>  #include "hw/timer/avr_timer16.h"
>  #include "hw/misc/avr_power.h"
> @@ -41,11 +42,11 @@ struct AtmegaMcuState {
>      MemoryRegion flash;
>      MemoryRegion eeprom;
>      MemoryRegion sram;
> +    Clock *xtal_clkin;
>      DeviceState *io;
>      AVRMaskState pwr[POWER_MAX];
>      AVRUsartState usart[USART_MAX];
>      AVRTimer16State timer[TIMER_MAX];
> -    uint64_t xtal_freq_hz;
>  };
>  
>  #endif /* HW_AVR_ATMEGA_H */
> diff --git a/include/hw/timer/avr_timer16.h b/include/hw/timer/avr_timer16.h
> index 05362543378..86dc9e98d95 100644
> --- a/include/hw/timer/avr_timer16.h
> +++ b/include/hw/timer/avr_timer16.h
> @@ -28,6 +28,7 @@
>  #ifndef HW_TIMER_AVR_TIMER16_H
>  #define HW_TIMER_AVR_TIMER16_H
>  
> +#include "hw/clock.h"
>  #include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "hw/hw.h"
> @@ -84,7 +85,8 @@ struct AVRTimer16State {
>      uint8_t ifr;
>  
>      uint8_t id;
> -    uint64_t cpu_freq_hz;
> +    Clock *io_clkin;
> +
>      uint64_t freq_hz;
>      uint64_t period_ns;
>      uint64_t reset_time_ns;
> diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
> index 3ff31492fa6..71e69823c07 100644
> --- a/hw/avr/arduino.c
> +++ b/hw/avr/arduino.c
> @@ -12,7 +12,9 @@
>  
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "hw/clock.h"
>  #include "hw/boards.h"
> +#include "hw/qdev-clock.h"
>  #include "atmega.h"
>  #include "boot.h"
>  #include "qom/object.h"
> @@ -21,6 +23,7 @@ struct ArduinoMachineState {
>      /*< private >*/
>      MachineState parent_obj;
>      /*< public >*/
> +    Clock *xtal;
>      AtmegaMcuState mcu;
>  };
>  typedef struct ArduinoMachineState ArduinoMachineState;
> @@ -44,9 +47,10 @@ static void arduino_machine_init(MachineState *machine)
>      ArduinoMachineClass *amc = ARDUINO_MACHINE_GET_CLASS(machine);
>      ArduinoMachineState *ams = ARDUINO_MACHINE(machine);
>  
> +    ams->xtal = machine_create_constant_clock(machine, "osc", amc->xtal_hz);
> +
>      object_initialize_child(OBJECT(machine), "mcu", &ams->mcu, amc->mcu_type);
> -    object_property_set_uint(OBJECT(&ams->mcu), "xtal-frequency-hz",
> -                             amc->xtal_hz, &error_abort);
> +    qdev_connect_clock_in(DEVICE(&ams->mcu), "osc-in", ams->xtal);
>      sysbus_realize(SYS_BUS_DEVICE(&ams->mcu), &error_abort);
>  
>      if (machine->firmware) {
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb6..b8a11965435 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -15,6 +15,7 @@
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/qdev-clock.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/sysbus.h"
>  #include "qom/object.h"
> @@ -216,6 +217,14 @@ static void connect_power_reduction_gpio(AtmegaMcuState *s,
>                         qdev_get_gpio_in(cpu, 0));
>  }
>  
> +static void atmega_init(Object *obj)
> +{
> +    AtmegaMcuState *s = ATMEGA_MCU(obj);
> +
> +    s->xtal_clkin = qdev_init_clock_in(DEVICE(obj), "osc-in",
> +                                       NULL, NULL, ClockUpdate);
> +}
> +
>  static void atmega_realize(DeviceState *dev, Error **errp)
>  {
>      AtmegaMcuState *s = ATMEGA_MCU(dev);
> @@ -227,11 +236,6 @@ static void atmega_realize(DeviceState *dev, Error **errp)
>  
>      assert(mc->io_size <= 0x200);
>  
> -    if (!s->xtal_freq_hz) {
> -        error_setg(errp, "\"xtal-frequency-hz\" property must be provided.");
> -        return;
> -    }
> -
>      /* CPU */
>      object_initialize_child(OBJECT(dev), "cpu", &s->cpu, mc->cpu_type);
>      object_property_set_bool(OBJECT(&s->cpu), "realized", true, &error_abort);
> @@ -328,8 +332,7 @@ static void atmega_realize(DeviceState *dev, Error **errp)
>          devname = g_strdup_printf("timer%zu", i);
>          object_initialize_child(OBJECT(dev), devname, &s->timer[i],
>                                  TYPE_AVR_TIMER16);

I forgot to commit this hunk:

  +        /*
  +         * Since we do not model the Clock Distribution system,
  +         * directly connect the external clock to I/O clock.
  +         */

> -        object_property_set_uint(OBJECT(&s->timer[i]), "cpu-frequency-hz",
> -                                 s->xtal_freq_hz, &error_abort);
> +        qdev_connect_clock_in(DEVICE(&s->timer[i]), "io-clk", s->xtal_clkin);
>          sbd = SYS_BUS_DEVICE(&s->timer[i]);
>          sysbus_realize(sbd, &error_abort);
>          sysbus_mmio_map(sbd, 0, OFFSET_DATA + mc->dev[idx].addr);
> @@ -353,8 +356,6 @@ static void atmega_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property atmega_props[] = {
> -    DEFINE_PROP_UINT64("xtal-frequency-hz", AtmegaMcuState,
> -                       xtal_freq_hz, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> @@ -449,6 +450,7 @@ static const TypeInfo atmega_mcu_types[] = {
>          .name           = TYPE_ATMEGA_MCU,
>          .parent         = TYPE_SYS_BUS_DEVICE,
>          .instance_size  = sizeof(AtmegaMcuState),
> +        .instance_init  = atmega_init,
>          .class_size     = sizeof(AtmegaMcuClass),
>          .class_init     = atmega_class_init,
>          .abstract       = true,
> diff --git a/hw/timer/avr_timer16.c b/hw/timer/avr_timer16.c
> index c48555da525..7092023d616 100644
> --- a/hw/timer/avr_timer16.c
> +++ b/hw/timer/avr_timer16.c
> @@ -35,6 +35,7 @@
>  #include "qapi/error.h"
>  #include "qemu/log.h"
>  #include "hw/irq.h"
> +#include "hw/qdev-clock.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/timer/avr_timer16.h"
>  #include "trace.h"
> @@ -167,7 +168,7 @@ static void avr_timer16_clksrc_update(AVRTimer16State *t16)
>          break;
>      }
>      if (divider) {
> -        t16->freq_hz = t16->cpu_freq_hz / divider;
> +        t16->freq_hz = clock_get_hz(t16->io_clkin) / divider;
>          t16->period_ns = NANOSECONDS_PER_SECOND / t16->freq_hz;
>          trace_avr_timer16_clksrc_update(t16->freq_hz, t16->period_ns,
>                                          (uint64_t)(1e6 / t16->freq_hz));
> @@ -544,8 +545,6 @@ static const MemoryRegionOps avr_timer16_ifr_ops = {
>  
>  static Property avr_timer16_properties[] = {
>      DEFINE_PROP_UINT8("id", struct AVRTimer16State, id, 0),
> -    DEFINE_PROP_UINT64("cpu-frequency-hz", struct AVRTimer16State,
> -                       cpu_freq_hz, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -560,10 +559,20 @@ static void avr_timer16_pr(void *opaque, int irq, int level)
>      }
>  }
>  
> +static void avr_timer16_clock_update(void *opaque, ClockEvent event)
> +{
> +    AVRTimer16State *s = opaque;
> +
> +    avr_timer16_clksrc_update(s);
> +}
> +
>  static void avr_timer16_init(Object *obj)
>  {
>      AVRTimer16State *s = AVR_TIMER16(obj);
>  
> +    s->io_clkin = qdev_init_clock_in(DEVICE(obj), "io-clk",
> +                                     avr_timer16_clock_update, s, ClockUpdate);
> +
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->capt_irq);
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compa_irq);
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compb_irq);
> @@ -587,11 +596,6 @@ static void avr_timer16_realize(DeviceState *dev, Error **errp)
>  {
>      AVRTimer16State *s = AVR_TIMER16(dev);
>  
> -    if (s->cpu_freq_hz == 0) {
> -        error_setg(errp, "AVR timer16: cpu-frequency-hz property must be set");
> -        return;
> -    }
> -
>      s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, avr_timer16_interrupt, s);
>      s->enabled = true;
>  }
>
diff mbox series

Patch

diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e1..12e856386b6 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -11,6 +11,7 @@ 
 #ifndef HW_AVR_ATMEGA_H
 #define HW_AVR_ATMEGA_H
 
+#include "hw/clock.h"
 #include "hw/char/avr_usart.h"
 #include "hw/timer/avr_timer16.h"
 #include "hw/misc/avr_power.h"
@@ -41,11 +42,11 @@  struct AtmegaMcuState {
     MemoryRegion flash;
     MemoryRegion eeprom;
     MemoryRegion sram;
+    Clock *xtal_clkin;
     DeviceState *io;
     AVRMaskState pwr[POWER_MAX];
     AVRUsartState usart[USART_MAX];
     AVRTimer16State timer[TIMER_MAX];
-    uint64_t xtal_freq_hz;
 };
 
 #endif /* HW_AVR_ATMEGA_H */
diff --git a/include/hw/timer/avr_timer16.h b/include/hw/timer/avr_timer16.h
index 05362543378..86dc9e98d95 100644
--- a/include/hw/timer/avr_timer16.h
+++ b/include/hw/timer/avr_timer16.h
@@ -28,6 +28,7 @@ 
 #ifndef HW_TIMER_AVR_TIMER16_H
 #define HW_TIMER_AVR_TIMER16_H
 
+#include "hw/clock.h"
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "hw/hw.h"
@@ -84,7 +85,8 @@  struct AVRTimer16State {
     uint8_t ifr;
 
     uint8_t id;
-    uint64_t cpu_freq_hz;
+    Clock *io_clkin;
+
     uint64_t freq_hz;
     uint64_t period_ns;
     uint64_t reset_time_ns;
diff --git a/hw/avr/arduino.c b/hw/avr/arduino.c
index 3ff31492fa6..71e69823c07 100644
--- a/hw/avr/arduino.c
+++ b/hw/avr/arduino.c
@@ -12,7 +12,9 @@ 
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "hw/clock.h"
 #include "hw/boards.h"
+#include "hw/qdev-clock.h"
 #include "atmega.h"
 #include "boot.h"
 #include "qom/object.h"
@@ -21,6 +23,7 @@  struct ArduinoMachineState {
     /*< private >*/
     MachineState parent_obj;
     /*< public >*/
+    Clock *xtal;
     AtmegaMcuState mcu;
 };
 typedef struct ArduinoMachineState ArduinoMachineState;
@@ -44,9 +47,10 @@  static void arduino_machine_init(MachineState *machine)
     ArduinoMachineClass *amc = ARDUINO_MACHINE_GET_CLASS(machine);
     ArduinoMachineState *ams = ARDUINO_MACHINE(machine);
 
+    ams->xtal = machine_create_constant_clock(machine, "osc", amc->xtal_hz);
+
     object_initialize_child(OBJECT(machine), "mcu", &ams->mcu, amc->mcu_type);
-    object_property_set_uint(OBJECT(&ams->mcu), "xtal-frequency-hz",
-                             amc->xtal_hz, &error_abort);
+    qdev_connect_clock_in(DEVICE(&ams->mcu), "osc-in", ams->xtal);
     sysbus_realize(SYS_BUS_DEVICE(&ams->mcu), &error_abort);
 
     if (machine->firmware) {
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb6..b8a11965435 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -15,6 +15,7 @@ 
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "qom/object.h"
@@ -216,6 +217,14 @@  static void connect_power_reduction_gpio(AtmegaMcuState *s,
                        qdev_get_gpio_in(cpu, 0));
 }
 
+static void atmega_init(Object *obj)
+{
+    AtmegaMcuState *s = ATMEGA_MCU(obj);
+
+    s->xtal_clkin = qdev_init_clock_in(DEVICE(obj), "osc-in",
+                                       NULL, NULL, ClockUpdate);
+}
+
 static void atmega_realize(DeviceState *dev, Error **errp)
 {
     AtmegaMcuState *s = ATMEGA_MCU(dev);
@@ -227,11 +236,6 @@  static void atmega_realize(DeviceState *dev, Error **errp)
 
     assert(mc->io_size <= 0x200);
 
-    if (!s->xtal_freq_hz) {
-        error_setg(errp, "\"xtal-frequency-hz\" property must be provided.");
-        return;
-    }
-
     /* CPU */
     object_initialize_child(OBJECT(dev), "cpu", &s->cpu, mc->cpu_type);
     object_property_set_bool(OBJECT(&s->cpu), "realized", true, &error_abort);
@@ -328,8 +332,7 @@  static void atmega_realize(DeviceState *dev, Error **errp)
         devname = g_strdup_printf("timer%zu", i);
         object_initialize_child(OBJECT(dev), devname, &s->timer[i],
                                 TYPE_AVR_TIMER16);
-        object_property_set_uint(OBJECT(&s->timer[i]), "cpu-frequency-hz",
-                                 s->xtal_freq_hz, &error_abort);
+        qdev_connect_clock_in(DEVICE(&s->timer[i]), "io-clk", s->xtal_clkin);
         sbd = SYS_BUS_DEVICE(&s->timer[i]);
         sysbus_realize(sbd, &error_abort);
         sysbus_mmio_map(sbd, 0, OFFSET_DATA + mc->dev[idx].addr);
@@ -353,8 +356,6 @@  static void atmega_realize(DeviceState *dev, Error **errp)
 }
 
 static Property atmega_props[] = {
-    DEFINE_PROP_UINT64("xtal-frequency-hz", AtmegaMcuState,
-                       xtal_freq_hz, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -449,6 +450,7 @@  static const TypeInfo atmega_mcu_types[] = {
         .name           = TYPE_ATMEGA_MCU,
         .parent         = TYPE_SYS_BUS_DEVICE,
         .instance_size  = sizeof(AtmegaMcuState),
+        .instance_init  = atmega_init,
         .class_size     = sizeof(AtmegaMcuClass),
         .class_init     = atmega_class_init,
         .abstract       = true,
diff --git a/hw/timer/avr_timer16.c b/hw/timer/avr_timer16.c
index c48555da525..7092023d616 100644
--- a/hw/timer/avr_timer16.c
+++ b/hw/timer/avr_timer16.c
@@ -35,6 +35,7 @@ 
 #include "qapi/error.h"
 #include "qemu/log.h"
 #include "hw/irq.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "hw/timer/avr_timer16.h"
 #include "trace.h"
@@ -167,7 +168,7 @@  static void avr_timer16_clksrc_update(AVRTimer16State *t16)
         break;
     }
     if (divider) {
-        t16->freq_hz = t16->cpu_freq_hz / divider;
+        t16->freq_hz = clock_get_hz(t16->io_clkin) / divider;
         t16->period_ns = NANOSECONDS_PER_SECOND / t16->freq_hz;
         trace_avr_timer16_clksrc_update(t16->freq_hz, t16->period_ns,
                                         (uint64_t)(1e6 / t16->freq_hz));
@@ -544,8 +545,6 @@  static const MemoryRegionOps avr_timer16_ifr_ops = {
 
 static Property avr_timer16_properties[] = {
     DEFINE_PROP_UINT8("id", struct AVRTimer16State, id, 0),
-    DEFINE_PROP_UINT64("cpu-frequency-hz", struct AVRTimer16State,
-                       cpu_freq_hz, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -560,10 +559,20 @@  static void avr_timer16_pr(void *opaque, int irq, int level)
     }
 }
 
+static void avr_timer16_clock_update(void *opaque, ClockEvent event)
+{
+    AVRTimer16State *s = opaque;
+
+    avr_timer16_clksrc_update(s);
+}
+
 static void avr_timer16_init(Object *obj)
 {
     AVRTimer16State *s = AVR_TIMER16(obj);
 
+    s->io_clkin = qdev_init_clock_in(DEVICE(obj), "io-clk",
+                                     avr_timer16_clock_update, s, ClockUpdate);
+
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->capt_irq);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compa_irq);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->compb_irq);
@@ -587,11 +596,6 @@  static void avr_timer16_realize(DeviceState *dev, Error **errp)
 {
     AVRTimer16State *s = AVR_TIMER16(dev);
 
-    if (s->cpu_freq_hz == 0) {
-        error_setg(errp, "AVR timer16: cpu-frequency-hz property must be set");
-        return;
-    }
-
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, avr_timer16_interrupt, s);
     s->enabled = true;
 }