> -----Original Message-----
> From: Peter Maydell <peter.mayd...@linaro.org>
> Sent: Tuesday, September 24, 2019 3:21 PM
> To: Anup Patel <anup.pa...@wdc.com>
> Cc: Palmer Dabbelt <pal...@sifive.com>; Alistair Francis
> <alistair.fran...@wdc.com>; Sagar Karandikar <sag...@eecs.berkeley.edu>;
> Bastian Koppelmann <kbast...@mail.uni-paderborn.de>; Atish Patra
> <atish.pa...@wdc.com>; qemu-ri...@nongnu.org; qemu-
> de...@nongnu.org; Anup Patel <a...@brainfault.org>
> Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
>
> On Tue, 24 Sep 2019 at 09:45, Anup Patel <anup.pa...@wdc.com> wrote:
> >
> > This patch adds model for Google Goldfish virtual platform RTC device.
> >
> > We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> > for providing real date-time to Guest Linux. The corresponding Linux
> > driver for Goldfish RTC device is already available in upstream Linux.
> >
> > For now, VM migration support is not available for Goldfish RTC device
> > but it will be added later when we implement VM migration for KVM RISC-
> V.
> >
> > Signed-off-by: Anup Patel <anup.pa...@wdc.com>
> > ---
> > +
> > +static Property goldfish_rtc_properties[] = {
> > + DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> > + DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> > + DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> alarm_running, 0),
> > + DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending,
> 0),
> > + DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled,
> 0),
> > + DEFINE_PROP_END_OF_LIST(),
> > +};
>
> What are all these properties trying to do ?
Argh, I forgot to remove these before sending.
I will drop these in next revision.
>
> > +
> > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + dc->props = goldfish_rtc_properties;
>
> Missing reset function.
Sure, I will add it. I got confused because I saw reset function in HPET
but not in PL031.
>
> If you don't want to implement migration support now you should at least
> put in something that block migration.
> (Personally I prefer to just write the vmstate, it's as easy as writing the
> code
> to block migrations.)
Okay, I will add vmstate.
Is there a way to unit test the vmstate part ?
OR
Are you fine if we test-n-fix it later ?
>
> thanks
> -- PMM
Regards,
Anup
