Re: [PATCH 08/20] hw/timer: Renesas TMU/CMT module.

[Philippe Mathieu-Daudé]

On 8/27/20 2:38 PM, Yoshinori Sato wrote:
> TMU - SH4 Timer module.
> CMT - Compare and match timer used by some Renesas MCUs.
>
> The two modules have similar interfaces and have been merged.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>   include/hw/timer/renesas_timer.h | 103 +++++
>   hw/timer/renesas_timer.c         | 639 +++++++++++++++++++++++++++++++
>   hw/timer/Kconfig                 |   4 +-
>   hw/timer/meson.build             |   2 +-
>   4 files changed, 745 insertions(+), 3 deletions(-)
>   create mode 100644 include/hw/timer/renesas_timer.h
>   create mode 100644 hw/timer/renesas_timer.c
...

> +static void update_clk(RenesasTimerBaseState *tmr, int ch)
> +{
> +    RenesasTimerBaseClass *tc = RenesasTimer_GET_CLASS(tmr);
> +    int t;
> +    t = tc->divrate(tmr, ch);
> +    if (t > 0) {
> +        t = tmr->input_freq / t;

If the clock is connected between INIT and REALIZE,
then t=0 and ...

> +        tmr->ch[ch].clk = NANOSECONDS_PER_SECOND / t;

... you get a DIV#0 here.

You can avoid it using clock_get_hz(tmr->pck) instead of
tmr->input_freq, which can be removed IMO.

> +    } else {
> +        tmr->ch[ch].clk = 0;
> +    }
> +}
...

> +static void tmr_pck_update(void *opaque)
> +{
> +    RenesasTimerBaseState *tmr = RenesasTimerBase(opaque);
> +    int64_t now;
> +    int i;
> +    struct rtimer_ch *ch;
> +    for (i = 0; i < TIMER_CH_CMT; i++) {
> +        if (tmr->ch[i].start) {
> +            tmr->ch[i].cnt = read_tcnt(tmr, i);
> +        }
> +    }
> +    if (clock_is_enabled(tmr->pck)) {
> +        tmr->input_freq = clock_get_hz(tmr->pck);
> +        now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        for (i = 0; i < TIMER_CH_CMT; i++) {
> +            update_clk(tmr, i);
> +            ch = &tmr->ch[i];
> +            if (ch->start) {
> +                ch->next = ch->base = now;
> +                if (tmr->direction == countup) {
> +                    ch->next += (ch->cor - ch->cnt) * ch->clk;
> +                } else {
> +                    ch->next += ch->cnt * ch->clk;
> +                }
> +                timer_mod(ch->timer, ch->next);
> +            }
> +        }
> +    } else {
> +        for (i = 0; i < TIMER_CH_CMT; i++) {
> +            if (tmr->ch[i].timer) {
> +                timer_del(tmr->ch[i].timer);
> +            }
> +        }
> +    }
> +}
...

> +static void cmt_init(Object *obj)
> +{
> +    SysBusDevice *d = SYS_BUS_DEVICE(obj);
> +    RenesasCMTState *cmt = RenesasCMT(obj);
> +    RenesasTimerBaseState *tmr = RenesasTimerBase(cmt);
> +    int i;
> +
> +    tmr->direction = countup;
> +    memory_region_init_io(&tmr->memory, obj, &cmt_ops,
> +                          tmr, "renesas-cmt", 0x10);
> +    sysbus_init_mmio(d, &tmr->memory);
> +
> +    for (i = 0; i < TIMER_CH_CMT; i++) {
> +        sysbus_init_irq(d, &tmr->ch[i].irq);
> +    }
> +    tmr->pck = qdev_init_clock_in(DEVICE(obj), "pck",
> +                                  tmr_pck_update, tmr);
> +}
...