Re: [PATCH v5 02/11] hw/misc: Add NPCM7xx Clock Controller device model

[Philippe Mathieu-Daudé]

On 7/9/20 2:35 AM, Havard Skinnemoen wrote:
> Enough functionality to boot the Linux kernel has been implemented. This
> includes:
> 
>   - Correct power-on reset values so the various clock rates can be
>     accurately calculated.
>   - Clock enables stick around when written.
> 
> In addition, a best effort attempt to implement SECCNT and CNTR25M was
> made even though I don't think the kernel needs them.
> 
> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
> ---
>  include/hw/misc/npcm7xx_clk.h |  66 ++++++++++
>  hw/misc/npcm7xx_clk.c         | 230 ++++++++++++++++++++++++++++++++++
>  hw/misc/Makefile.objs         |   1 +
>  hw/misc/trace-events          |   4 +
>  4 files changed, 301 insertions(+)
>  create mode 100644 include/hw/misc/npcm7xx_clk.h
>  create mode 100644 hw/misc/npcm7xx_clk.c
> 
...

> +static uint64_t npcm7xx_clk_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    uint32_t reg = offset / sizeof(uint32_t);
> +    NPCM7xxCLKState *s = opaque;
> +    int64_t now_ns;
> +    uint32_t value = 0;
> +
> +    if (reg >= NPCM7XX_CLK_NR_REGS) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: offset 0x%04x out of range\n",
> +                      __func__, (unsigned int)offset);
> +        return 0;
> +    }
> +
> +    switch (reg) {
> +    case NPCM7XX_CLK_SWRSTR:
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: register @ 0x%04x is 
> write-only\n",
> +                      __func__, (unsigned int)offset);
> +        break;
> +
> +    case NPCM7XX_CLK_SECCNT:
> +        now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        value = (now_ns - s->ref_ns) / NANOSECONDS_PER_SECOND;
> +        break;
> +
> +    case NPCM7XX_CLK_CNTR25M:
> +        now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        /*
> +         * This register counts 25 MHz cycles, updating every 640 ns. It 
> rolls
> +         * over to zero every second.
> +         *
> +         * The 4 LSBs are always zero: (1e9 / 640) << 4 = 25000000.
> +         */
> +        value = (((now_ns - s->ref_ns) / 640) << 4) % 25000000;

Can we declare NPCM7XX_TIMER_REF_HZ in hw/misc/npcm7xx_clk.h and
have the timer device include hw/misc/npcm7xx_clk.h?

> +        break;
> +
> +    default:
> +        value = s->regs[reg];
> +        break;
> +    };
> +
> +    trace_npcm7xx_clk_read(offset, value);
> +
> +    return value;
> +}
...