Re: [Qemu-devel] [PATCH v6 2/3] xlnx-zynqmp-rtc: Add basic time support

[Peter Maydell]

On 23 February 2018 at 17:21, Alistair Francis
<address@hidden> wrote:
> Allow the guest to determine the time set from the QEMU command line.
>
> This includes adding a trace event to debug the new time.
>
> Signed-off-by: Alistair Francis <address@hidden>

Thanks for reworking this -- I think it looks simpler this way.

> diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h 
> b/include/hw/timer/xlnx-zynqmp-rtc.h
> index 87649836cc..2867563bdd 100644
> --- a/include/hw/timer/xlnx-zynqmp-rtc.h
> +++ b/include/hw/timer/xlnx-zynqmp-rtc.h
> @@ -79,6 +79,9 @@ typedef struct XlnxZynqMPRTC {
>      qemu_irq irq_rtc_int;
>      qemu_irq irq_addr_error_int;
>
> +    struct tm current_tm;
> +    uint32_t tick_offset;
> +

Do we need current_tm to be a struct field now?

> @@ -143,6 +164,10 @@ static void rtc_reset(DeviceState *dev)
>          register_reset(&s->regs_info[i]);
>      }
>
> +    trace_xlnx_zynqmp_rtc_gettime(s->current_tm.tm_year, 
> s->current_tm.tm_mon,
> +                                  s->current_tm.tm_mday, 
> s->current_tm.tm_hour,
> +                                  s->current_tm.tm_min, 
> s->current_tm.tm_sec);
> +

...this trace is in reset, and is the only place we read
current_tm, except for...

>      rtc_int_update_irq(s);
>      addr_error_int_update_irq(s);
>  }
> @@ -178,14 +203,46 @@ static void rtc_init(Object *obj)
>      sysbus_init_mmio(sbd, &s->iomem);
>      sysbus_init_irq(sbd, &s->irq_rtc_int);
>      sysbus_init_irq(sbd, &s->irq_addr_error_int);
> +
> +    qemu_get_timedate(&s->current_tm, 0);
> +    s->tick_offset = mktimegm(&s->current_tm) -
> +        qemu_clock_get_ns(rtc_clock) / NANOSECONDS_PER_SECOND;

...this code in init, which could just use a local variable
instead, since the trace in reset isn't really reporting
interesting information any more (the value never changes, so
you could trace it in init instead).

> +}
>

thanks
-- PMM