[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 03/13] hw/timer: Add NPCM7xx Timer device model
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v6 03/13] hw/timer: Add NPCM7xx Timer device model |
Date: |
Fri, 17 Jul 2020 10:05:25 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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
...
> +static hwaddr npcm7xx_tcsr_index(hwaddr reg)
> +{
> + switch (reg) {
> + case NPCM7XX_TIMER_TCSR0:
> + return 0;
> + case NPCM7XX_TIMER_TCSR1:
> + return 1;
> + case NPCM7XX_TIMER_TCSR2:
> + return 2;
> + case NPCM7XX_TIMER_TCSR3:
> + return 3;
> + case NPCM7XX_TIMER_TCSR4:
> + return 4;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +static hwaddr npcm7xx_ticr_index(hwaddr reg)
> +{
> + switch (reg) {
> + case NPCM7XX_TIMER_TICR0:
> + return 0;
> + case NPCM7XX_TIMER_TICR1:
> + return 1;
> + case NPCM7XX_TIMER_TICR2:
> + return 2;
> + case NPCM7XX_TIMER_TICR3:
> + return 3;
> + case NPCM7XX_TIMER_TICR4:
> + return 4;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +static hwaddr npcm7xx_tdr_index(hwaddr reg)
> +{
> + switch (reg) {
> + case NPCM7XX_TIMER_TDR0:
> + return 0;
> + case NPCM7XX_TIMER_TDR1:
> + return 1;
> + case NPCM7XX_TIMER_TDR2:
> + return 2;
> + case NPCM7XX_TIMER_TDR3:
> + return 3;
> + case NPCM7XX_TIMER_TDR4:
> + return 4;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +static uint64_t npcm7xx_timer_read(void *opaque, hwaddr offset, unsigned
> size)
> +{
> + NPCM7xxTimerCtrlState *s = opaque;
> + uint64_t value = 0;
> + hwaddr reg;
> +
> + reg = offset / sizeof(uint32_t);
> + switch (reg) {
> + case NPCM7XX_TIMER_TCSR0:
> + case NPCM7XX_TIMER_TCSR1:
> + case NPCM7XX_TIMER_TCSR2:
> + case NPCM7XX_TIMER_TCSR3:
> + case NPCM7XX_TIMER_TCSR4:
> + value = s->timer[npcm7xx_tcsr_index(reg)].tcsr;
> + break;
> +
> + case NPCM7XX_TIMER_TICR0:
> + case NPCM7XX_TIMER_TICR1:
> + case NPCM7XX_TIMER_TICR2:
> + case NPCM7XX_TIMER_TICR3:
> + case NPCM7XX_TIMER_TICR4:
> + value = s->timer[npcm7xx_ticr_index(reg)].ticr;
> + break;
> +
> + case NPCM7XX_TIMER_TDR0:
> + case NPCM7XX_TIMER_TDR1:
> + case NPCM7XX_TIMER_TDR2:
> + case NPCM7XX_TIMER_TDR3:
> + case NPCM7XX_TIMER_TDR4:
> + value = npcm7xx_timer_read_tdr(&s->timer[npcm7xx_tdr_index(reg)]);
> + break;
Thanks for adding the register index getters, I'm not sure this is a
matter of taste, but I find it easier to review.
> +
> + case NPCM7XX_TIMER_TISR:
> + value = s->tisr;
> + break;
> +
> + case NPCM7XX_TIMER_WTCR:
> + value = s->wtcr;
> + break;
> +
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: invalid offset 0x%04" HWADDR_PRIx "\n",
> + __func__, offset);
> + break;
> + }
> +
> + trace_npcm7xx_timer_read(DEVICE(s)->canonical_path, offset, value);
> +
> + return value;
> +}
...