On Tue, Feb 27, 2018 at 8:36 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 23 February 2018 at 17:21, Alistair Francis
> <alistair.fran...@xilinx.com> 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 <alistair.fran...@xilinx.com>
>
> 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).
Yeah you are right. I have moved it all to the init() function.
Alistair
>
>> +}
>>
>
> thanks
> -- PMM
