Re: [Qemu-devel] [PATCH 12/13] cuda.c: fix T2 timer and enable its interrupt

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4473 bytes --]

On Fri, Oct 23, 2015 at 02:56:37PM +0100, Mark Cave-Ayland wrote:
> Fix the counter loading logic and enable the T2 interrupt when the timer
> expires.

A mention of what uses T2, and therefore why this is useful would be
good.

> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/misc/macio/cuda.c |   30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 687cb54..d864b24 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -136,7 +136,7 @@ static void cuda_timer_update(CUDAState *s, CUDATimer *ti,
>  
>  static void cuda_update_irq(CUDAState *s)
>  {
> -    if (s->ifr & s->ier & (SR_INT | T1_INT)) {
> +    if (s->ifr & s->ier & (SR_INT | T1_INT | T2_INT)) {
>          qemu_irq_raise(s->irq);
>      } else {
>          qemu_irq_lower(s->irq);
> @@ -175,7 +175,7 @@ static unsigned int get_counter(CUDATimer *ti)
>  
>  static void set_counter(CUDAState *s, CUDATimer *ti, unsigned int val)
>  {
> -    CUDA_DPRINTF("T%d.counter=%d\n", 1 + (ti->timer == NULL), val);
> +    CUDA_DPRINTF("T%d.counter=%d\n", 1 + ti->index, val);
>      ti->load_time = get_tb(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
>                             s->frequency);
>      ti->counter_value = val;
> @@ -220,7 +220,7 @@ static void cuda_timer_update(CUDAState *s, CUDATimer *ti,
>  {
>      if (!ti->timer)
>          return;
> -    if ((s->acr & T1MODE) != T1MODE_CONT) {
> +    if (ti->index == 0 && (s->acr & T1MODE) != T1MODE_CONT) {
>          timer_del(ti->timer);
>      } else {
>          ti->next_irq_time = get_next_irq_time(ti, current_time);
> @@ -238,6 +238,16 @@ static void cuda_timer1(void *opaque)
>      cuda_update_irq(s);
>  }
>  
> +static void cuda_timer2(void *opaque)
> +{
> +    CUDAState *s = opaque;
> +    CUDATimer *ti = &s->timers[1];
> +
> +    cuda_timer_update(s, ti, ti->next_irq_time);
> +    s->ifr |= T2_INT;
> +    cuda_update_irq(s);
> +}
> +
>  static uint32_t cuda_readb(void *opaque, hwaddr addr)
>  {
>      CUDAState *s = opaque;
> @@ -276,6 +286,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr)
>      case CUDA_REG_T2CL:
>          val = get_counter(&s->timers[1]) & 0xff;
>          s->ifr &= ~T2_INT;
> +        cuda_update_irq(s);
>          break;
>      case CUDA_REG_T2CH:
>          val = get_counter(&s->timers[1]) >> 8;
> @@ -352,11 +363,12 @@ static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val)
>          cuda_timer_update(s, &s->timers[0], qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>          break;
>      case CUDA_REG_T2CL:
> -        s->timers[1].latch = val;
> -        set_counter(s, &s->timers[1], val);
> +        s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
>          break;
>      case CUDA_REG_T2CH:
> -        set_counter(s, &s->timers[1], (val << 8) | s->timers[1].latch);
> +        s->timers[1].latch = (s->timers[1].latch & 0xff) | (val << 8);
> +        s->ifr &= ~T2_INT;
> +        set_counter(s, &s->timers[1], s->timers[1].latch);

So the new code appears to be like that for T1CL / T1CH, which makes
sense.  However, T1CL has a cuda_timer_update() call.  Do you also
need that for T2CL?

>          break;
>      case CUDA_REG_SR:
>          s->sr = val;
> @@ -719,8 +731,7 @@ static void cuda_reset(DeviceState *dev)
>      s->timers[0].latch = 0xffff;
>      set_counter(s, &s->timers[0], 0xffff);
>  
> -    s->timers[1].latch = 0;
> -    set_counter(s, &s->timers[1], 0xffff);
> +    s->timers[1].latch = 0xffff;
>  }
>  
>  static void cuda_realizefn(DeviceState *dev, Error **errp)
> @@ -730,7 +741,8 @@ static void cuda_realizefn(DeviceState *dev, Error **errp)
>  
>      s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer1, s);
>      s->timers[0].frequency = s->frequency;
> -    s->timers[1].frequency = s->frequency;
> +    s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cuda_timer2, s);
> +    s->timers[1].frequency = (SCALE_US * 6000) / 4700;

Where does this T2 frequency come from?

>      qemu_get_timedate(&tm, 0);
>      s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]