Re: [PATCH v6 03/13] hw/timer: Add NPCM7xx Timer device model

[Philippe Mathieu-Daudé]

On 7/17/20 8:02 AM, Havard Skinnemoen wrote:
> The NPCM730 and NPCM750 SoCs have three timer modules each holding five
> timers and some shared registers (e.g. interrupt status).
> 
> Each timer runs at 25 MHz divided by a prescaler, and counts down from a
> configurable initial value to zero. When zero is reached, the interrupt
> flag for the timer is set, and the timer is disabled (one-shot mode) or
> reloaded from its initial value (periodic mode).
> 
> This implementation is sufficient to boot a Linux kernel configured for
> NPCM750. Note that the kernel does not seem to actually turn on the
> interrupts.
> 
> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
> ---
>  include/hw/timer/npcm7xx_timer.h |  96 ++++++
>  hw/timer/npcm7xx_timer.c         | 489 +++++++++++++++++++++++++++++++
>  hw/timer/Makefile.objs           |   1 +
>  hw/timer/trace-events            |   5 +
>  4 files changed, 591 insertions(+)
>  create mode 100644 include/hw/timer/npcm7xx_timer.h
>  create mode 100644 hw/timer/npcm7xx_timer.c
...

> +/*
> + * Raise the interrupt line if there's a pending interrupt and interrupts are
> + * enabled for this timer. If not, lower it.
> + */
> +static void npcm7xx_timer_check_interrupt(NPCM7xxTimer *t)
> +{
> +    NPCM7xxTimerCtrlState *tc = t->ctrl;
> +    int index = npcm7xx_timer_index(tc, t);
> +
> +    if ((t->tcsr & NPCM7XX_TCSR_IE) && (tc->tisr & BIT(index))) {
> +        qemu_irq_raise(t->irq);
> +        trace_npcm7xx_timer_irq(DEVICE(tc)->canonical_path, index, 1);
> +    } else {
> +        qemu_irq_lower(t->irq);
> +        trace_npcm7xx_timer_irq(DEVICE(tc)->canonical_path, index, 0);
> +    }

Maybe simpler:

  bool pending = (t->tcsr & NPCM7XX_TCSR_IE) && (tc->tisr & BIT(index));

  trace_npcm7xx_timer_irq(DEVICE(tc)->canonical_path, index, pending);
  qemu_set_irq(t->irq, pending);

Anyway,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>