diff mbox

[v4,5/7] allwinner-a10-pit: implement prescaler and source selection

Message ID 1395350719-3778-6-git-send-email-b.galvani@gmail.com
State New
Headers show

Commit Message

Beniamino Galvani March 20, 2014, 9:25 p.m. UTC
This implements the prescaler and source fields of the timer control
register. The source for each timer can be selected among 4 clock
inputs whose frequencies are set through model properties.

Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
 hw/arm/cubieboard.c                  |   13 ++++++++++
 hw/timer/allwinner-a10-pit.c         |   43 +++++++++++++++++++++++++++++++++-
 include/hw/timer/allwinner-a10-pit.h |    4 ++++
 3 files changed, 59 insertions(+), 1 deletion(-)

Comments

Peter Crosthwaite March 24, 2014, 11:49 p.m. UTC | #1
On Fri, Mar 21, 2014 at 7:25 AM, Beniamino Galvani <b.galvani@gmail.com> wrote:
> This implements the prescaler and source fields of the timer control
> register. The source for each timer can be selected among 4 clock
> inputs whose frequencies are set through model properties.
>
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> ---
>  hw/arm/cubieboard.c                  |   13 ++++++++++
>  hw/timer/allwinner-a10-pit.c         |   43 +++++++++++++++++++++++++++++++++-
>  include/hw/timer/allwinner-a10-pit.h |    4 ++++
>  3 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> index d95a7f3..9d158c7 100644
> --- a/hw/arm/cubieboard.c
> +++ b/hw/arm/cubieboard.c
> @@ -43,6 +43,19 @@ static void cubieboard_init(QEMUMachineInitArgs *args)
>          exit(1);
>      }
>
> +    object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", &err);
> +    if (err != NULL) {
> +        error_report("Couldn't set clk0 frequency: %s", error_get_pretty(err));
> +        exit(1);
> +    }
> +
> +    object_property_set_int(OBJECT(&s->a10->timer), 24000000, "clk1-freq",
> +                            &err);
> +    if (err != NULL) {
> +        error_report("Couldn't set clk1 frequency: %s", error_get_pretty(err));
> +        exit(1);
> +    }
> +
>      object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
>      if (err != NULL) {
>          error_report("Couldn't realize Allwinner A10: %s",
> diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
> index 5aa78a9..5c1ffba 100644
> --- a/hw/timer/allwinner-a10-pit.c
> +++ b/hw/timer/allwinner-a10-pit.c
> @@ -74,6 +74,37 @@ static uint64_t a10_pit_read(void *opaque, hwaddr offset, unsigned size)
>      return 0;
>  }
>
> +static void a10_pit_set_freq(AwA10PITState *s, int index)
> +{
> +    uint32_t prescaler, source;
> +    uint32_t source_freq;
> +
> +    prescaler = 1 << extract32(s->control[index], 4, 3);
> +    source = extract32(s->control[index], 2, 2);
> +
> +    switch (source) {
> +    case 0:
> +        source_freq = s->clk0_freq;
> +        break;
> +    case 1:
> +        source_freq = s->clk1_freq;
> +        break;
> +    case 2:
> +        source_freq = s->clk2_freq;
> +        break;
> +    case 3:
> +        source_freq = s->clk3_freq;
> +        break;
> +    }

You can get rid of this [1] ...

> +
> +    if (source_freq) {
> +        ptimer_set_freq(s->timer[index], source_freq / prescaler);
> +    } else {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid clock source %u\n",
> +                      __func__, source);
> +    }
> +}
> +
>  static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
>                              unsigned size)
>  {
> @@ -96,6 +127,7 @@ static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
>          switch (offset & 0x0f) {
>          case AW_A10_PIT_TIMER_CONTROL:
>              s->control[index] = value;
> +            a10_pit_set_freq(s, index);
>              if (s->control[index] & AW_A10_PIT_TIMER_RELOAD) {
>                  ptimer_set_count(s->timer[index], s->interval[index]);
>              }
> @@ -161,6 +193,14 @@ static const MemoryRegionOps a10_pit_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> +static Property a10_pit_properties[] = {
> +    DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk0_freq, 0),

... [1] by changing this to:

DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk_freq[0], 0),

> +    DEFINE_PROP_UINT32("clk1-freq", AwA10PITState, clk1_freq, 0),
> +    DEFINE_PROP_UINT32("clk2-freq", AwA10PITState, clk2_freq, 0),
> +    DEFINE_PROP_UINT32("clk3-freq", AwA10PITState, clk3_freq, 0),


Note that there is an array property system (DEFINE_PROP_ARRAY) that
should kinda work here. Although it requires you to set the length of
the array on the setter level, which should be fixed to 4 in this
case. Check the "db-voltage" property of arm_sysctrl and its usage in
vexpress. But I can't see a clean way of fixing this imminently
without patching that property system. Curious it works nicely for
dynamic arrays but not static (unless I am missing something).

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static const VMStateDescription vmstate_a10_pit = {
>      .name = "a10.pit",
>      .version_id = 1,
> @@ -196,6 +236,7 @@ static void a10_pit_reset(DeviceState *dev)
>          s->interval[i] = 0;
>          s->count[i] = 0;
>          ptimer_stop(s->timer[i]);
> +        a10_pit_set_freq(s, i);
>      }
>      s->watch_dog_mode = 0;
>      s->watch_dog_control = 0;
> @@ -241,7 +282,6 @@ static void a10_pit_init(Object *obj)
>          tc->index = i;
>          bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
>          s->timer[i] = ptimer_init(bh[i]);
> -        ptimer_set_freq(s->timer[i], 240000);
>      }
>  }
>
> @@ -250,6 +290,7 @@ static void a10_pit_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->reset = a10_pit_reset;
> +    dc->props = a10_pit_properties;
>      dc->desc = "allwinner a10 timer";
>      dc->vmsd = &vmstate_a10_pit;
>  }
> diff --git a/include/hw/timer/allwinner-a10-pit.h b/include/hw/timer/allwinner-a10-pit.h
> index a48d3c7..5388b2b 100644
> --- a/include/hw/timer/allwinner-a10-pit.h
> +++ b/include/hw/timer/allwinner-a10-pit.h
> @@ -50,6 +50,10 @@ typedef struct AwA10PITState {
>      ptimer_state * timer[AW_A10_PIT_TIMER_NR];
>      AwA10TimerContext timer_context[AW_A10_PIT_TIMER_NR];
>      MemoryRegion iomem;
> +    uint32_t clk0_freq;
> +    uint32_t clk1_freq;
> +    uint32_t clk2_freq;
> +    uint32_t clk3_freq;
>

And this becomes an array.

With just the arrayification change:

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Regards,
Peter

>      uint32_t irq_enable;
>      uint32_t irq_status;
> --
> 1.7.10.4
>
>
Beniamino Galvani March 25, 2014, 6:08 p.m. UTC | #2
On Tue, Mar 25, 2014 at 09:49:21AM +1000, Peter Crosthwaite wrote:
> On Fri, Mar 21, 2014 at 7:25 AM, Beniamino Galvani <b.galvani@gmail.com> wrote:
> > This implements the prescaler and source fields of the timer control
> > register. The source for each timer can be selected among 4 clock
> > inputs whose frequencies are set through model properties.
> >
> > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> > ---
> >  hw/arm/cubieboard.c                  |   13 ++++++++++
> >  hw/timer/allwinner-a10-pit.c         |   43 +++++++++++++++++++++++++++++++++-
> >  include/hw/timer/allwinner-a10-pit.h |    4 ++++
> >  3 files changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> > index d95a7f3..9d158c7 100644
> > --- a/hw/arm/cubieboard.c
> > +++ b/hw/arm/cubieboard.c
> > @@ -43,6 +43,19 @@ static void cubieboard_init(QEMUMachineInitArgs *args)
> >          exit(1);
> >      }
> >
> > +    object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", &err);
> > +    if (err != NULL) {
> > +        error_report("Couldn't set clk0 frequency: %s", error_get_pretty(err));
> > +        exit(1);
> > +    }
> > +
> > +    object_property_set_int(OBJECT(&s->a10->timer), 24000000, "clk1-freq",
> > +                            &err);
> > +    if (err != NULL) {
> > +        error_report("Couldn't set clk1 frequency: %s", error_get_pretty(err));
> > +        exit(1);
> > +    }
> > +
> >      object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
> >      if (err != NULL) {
> >          error_report("Couldn't realize Allwinner A10: %s",
> > diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
> > index 5aa78a9..5c1ffba 100644
> > --- a/hw/timer/allwinner-a10-pit.c
> > +++ b/hw/timer/allwinner-a10-pit.c
> > @@ -74,6 +74,37 @@ static uint64_t a10_pit_read(void *opaque, hwaddr offset, unsigned size)
> >      return 0;
> >  }
> >
> > +static void a10_pit_set_freq(AwA10PITState *s, int index)
> > +{
> > +    uint32_t prescaler, source;
> > +    uint32_t source_freq;
> > +
> > +    prescaler = 1 << extract32(s->control[index], 4, 3);
> > +    source = extract32(s->control[index], 2, 2);
> > +
> > +    switch (source) {
> > +    case 0:
> > +        source_freq = s->clk0_freq;
> > +        break;
> > +    case 1:
> > +        source_freq = s->clk1_freq;
> > +        break;
> > +    case 2:
> > +        source_freq = s->clk2_freq;
> > +        break;
> > +    case 3:
> > +        source_freq = s->clk3_freq;
> > +        break;
> > +    }
> 
> You can get rid of this [1] ...
> 
> > +
> > +    if (source_freq) {
> > +        ptimer_set_freq(s->timer[index], source_freq / prescaler);
> > +    } else {
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid clock source %u\n",
> > +                      __func__, source);
> > +    }
> > +}
> > +
> >  static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
> >                              unsigned size)
> >  {
> > @@ -96,6 +127,7 @@ static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
> >          switch (offset & 0x0f) {
> >          case AW_A10_PIT_TIMER_CONTROL:
> >              s->control[index] = value;
> > +            a10_pit_set_freq(s, index);
> >              if (s->control[index] & AW_A10_PIT_TIMER_RELOAD) {
> >                  ptimer_set_count(s->timer[index], s->interval[index]);
> >              }
> > @@ -161,6 +193,14 @@ static const MemoryRegionOps a10_pit_ops = {
> >      .endianness = DEVICE_NATIVE_ENDIAN,
> >  };
> >
> > +static Property a10_pit_properties[] = {
> > +    DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk0_freq, 0),
> 
> ... [1] by changing this to:
> 
> DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk_freq[0], 0),
> 
> > +    DEFINE_PROP_UINT32("clk1-freq", AwA10PITState, clk1_freq, 0),
> > +    DEFINE_PROP_UINT32("clk2-freq", AwA10PITState, clk2_freq, 0),
> > +    DEFINE_PROP_UINT32("clk3-freq", AwA10PITState, clk3_freq, 0),
> 
> 
> Note that there is an array property system (DEFINE_PROP_ARRAY) that
> should kinda work here. Although it requires you to set the length of
> the array on the setter level, which should be fixed to 4 in this
> case. Check the "db-voltage" property of arm_sysctrl and its usage in
> vexpress. But I can't see a clean way of fixing this imminently
> without patching that property system. Curious it works nicely for
> dynamic arrays but not static (unless I am missing something).
> 
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> >  static const VMStateDescription vmstate_a10_pit = {
> >      .name = "a10.pit",
> >      .version_id = 1,
> > @@ -196,6 +236,7 @@ static void a10_pit_reset(DeviceState *dev)
> >          s->interval[i] = 0;
> >          s->count[i] = 0;
> >          ptimer_stop(s->timer[i]);
> > +        a10_pit_set_freq(s, i);
> >      }
> >      s->watch_dog_mode = 0;
> >      s->watch_dog_control = 0;
> > @@ -241,7 +282,6 @@ static void a10_pit_init(Object *obj)
> >          tc->index = i;
> >          bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
> >          s->timer[i] = ptimer_init(bh[i]);
> > -        ptimer_set_freq(s->timer[i], 240000);
> >      }
> >  }
> >
> > @@ -250,6 +290,7 @@ static void a10_pit_class_init(ObjectClass *klass, void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >
> >      dc->reset = a10_pit_reset;
> > +    dc->props = a10_pit_properties;
> >      dc->desc = "allwinner a10 timer";
> >      dc->vmsd = &vmstate_a10_pit;
> >  }
> > diff --git a/include/hw/timer/allwinner-a10-pit.h b/include/hw/timer/allwinner-a10-pit.h
> > index a48d3c7..5388b2b 100644
> > --- a/include/hw/timer/allwinner-a10-pit.h
> > +++ b/include/hw/timer/allwinner-a10-pit.h
> > @@ -50,6 +50,10 @@ typedef struct AwA10PITState {
> >      ptimer_state * timer[AW_A10_PIT_TIMER_NR];
> >      AwA10TimerContext timer_context[AW_A10_PIT_TIMER_NR];
> >      MemoryRegion iomem;
> > +    uint32_t clk0_freq;
> > +    uint32_t clk1_freq;
> > +    uint32_t clk2_freq;
> > +    uint32_t clk3_freq;
> >
> 
> And this becomes an array.

I will change this in the next version, thank you for the review.

Beniamino
diff mbox

Patch

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index d95a7f3..9d158c7 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -43,6 +43,19 @@  static void cubieboard_init(QEMUMachineInitArgs *args)
         exit(1);
     }
 
+    object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", &err);
+    if (err != NULL) {
+        error_report("Couldn't set clk0 frequency: %s", error_get_pretty(err));
+        exit(1);
+    }
+
+    object_property_set_int(OBJECT(&s->a10->timer), 24000000, "clk1-freq",
+                            &err);
+    if (err != NULL) {
+        error_report("Couldn't set clk1 frequency: %s", error_get_pretty(err));
+        exit(1);
+    }
+
     object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
     if (err != NULL) {
         error_report("Couldn't realize Allwinner A10: %s",
diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
index 5aa78a9..5c1ffba 100644
--- a/hw/timer/allwinner-a10-pit.c
+++ b/hw/timer/allwinner-a10-pit.c
@@ -74,6 +74,37 @@  static uint64_t a10_pit_read(void *opaque, hwaddr offset, unsigned size)
     return 0;
 }
 
+static void a10_pit_set_freq(AwA10PITState *s, int index)
+{
+    uint32_t prescaler, source;
+    uint32_t source_freq;
+
+    prescaler = 1 << extract32(s->control[index], 4, 3);
+    source = extract32(s->control[index], 2, 2);
+
+    switch (source) {
+    case 0:
+        source_freq = s->clk0_freq;
+        break;
+    case 1:
+        source_freq = s->clk1_freq;
+        break;
+    case 2:
+        source_freq = s->clk2_freq;
+        break;
+    case 3:
+        source_freq = s->clk3_freq;
+        break;
+    }
+
+    if (source_freq) {
+        ptimer_set_freq(s->timer[index], source_freq / prescaler);
+    } else {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid clock source %u\n",
+                      __func__, source);
+    }
+}
+
 static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
                             unsigned size)
 {
@@ -96,6 +127,7 @@  static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
         switch (offset & 0x0f) {
         case AW_A10_PIT_TIMER_CONTROL:
             s->control[index] = value;
+            a10_pit_set_freq(s, index);
             if (s->control[index] & AW_A10_PIT_TIMER_RELOAD) {
                 ptimer_set_count(s->timer[index], s->interval[index]);
             }
@@ -161,6 +193,14 @@  static const MemoryRegionOps a10_pit_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static Property a10_pit_properties[] = {
+    DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk0_freq, 0),
+    DEFINE_PROP_UINT32("clk1-freq", AwA10PITState, clk1_freq, 0),
+    DEFINE_PROP_UINT32("clk2-freq", AwA10PITState, clk2_freq, 0),
+    DEFINE_PROP_UINT32("clk3-freq", AwA10PITState, clk3_freq, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static const VMStateDescription vmstate_a10_pit = {
     .name = "a10.pit",
     .version_id = 1,
@@ -196,6 +236,7 @@  static void a10_pit_reset(DeviceState *dev)
         s->interval[i] = 0;
         s->count[i] = 0;
         ptimer_stop(s->timer[i]);
+        a10_pit_set_freq(s, i);
     }
     s->watch_dog_mode = 0;
     s->watch_dog_control = 0;
@@ -241,7 +282,6 @@  static void a10_pit_init(Object *obj)
         tc->index = i;
         bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
         s->timer[i] = ptimer_init(bh[i]);
-        ptimer_set_freq(s->timer[i], 240000);
     }
 }
 
@@ -250,6 +290,7 @@  static void a10_pit_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->reset = a10_pit_reset;
+    dc->props = a10_pit_properties;
     dc->desc = "allwinner a10 timer";
     dc->vmsd = &vmstate_a10_pit;
 }
diff --git a/include/hw/timer/allwinner-a10-pit.h b/include/hw/timer/allwinner-a10-pit.h
index a48d3c7..5388b2b 100644
--- a/include/hw/timer/allwinner-a10-pit.h
+++ b/include/hw/timer/allwinner-a10-pit.h
@@ -50,6 +50,10 @@  typedef struct AwA10PITState {
     ptimer_state * timer[AW_A10_PIT_TIMER_NR];
     AwA10TimerContext timer_context[AW_A10_PIT_TIMER_NR];
     MemoryRegion iomem;
+    uint32_t clk0_freq;
+    uint32_t clk1_freq;
+    uint32_t clk2_freq;
+    uint32_t clk3_freq;
 
     uint32_t irq_enable;
     uint32_t irq_status;