| Message ID | cover.1519406423.git.alistair.francis@xilinx.com |
|---|---|
| Headers | show |
| Series | Add and connect the ZynqMP RTC | expand |
On 23 February 2018 at 17:21, Alistair Francis <alistair.francis@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.francis@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). > +} > thanks -- PMM
On Tue, Feb 27, 2018 at 8:36 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 February 2018 at 17:21, Alistair Francis > <alistair.francis@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.francis@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