Re: [PATCH v3 12/17] hw/arm/allwinner: add RTC device support

[Niek Linnenbank <nieklinnenbank@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 30593 bytes --]

On Tue, Jan 14, 2020 at 11:52 PM Niek Linnenbank <nieklinnenbank@gmail.com>
wrote:

> Hi Philippe,
>
> On Mon, Jan 13, 2020 at 11:57 PM Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
>
>> On 1/8/20 9:00 PM, Niek Linnenbank wrote:
>> > Allwinner System-on-Chips usually contain a Real Time Clock (RTC)
>> > for non-volatile system date and time keeping. This commit adds a
>> generic
>> > Allwinner RTC device that supports the RTC devices found in Allwinner
>> SoC
>> > family sun4i (A10), sun7i (A20) and sun6i and newer (A31, H2+, H3, etc).
>> > The following RTC functionality and features are implemented:
>> >
>> >   * Year-Month-Day read/write
>> >   * Hour-Minute-Second read/write
>> >   * General Purpose storage
>> >
>> > The following boards are extended with the RTC device:
>> >
>> >   * Cubieboard (hw/arm/cubieboard.c)
>> >   * Orange Pi PC (hw/arm/orangepi.c)
>> >
>> > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>> > ---
>> >   include/hw/arm/allwinner-a10.h |   4 +-
>> >   include/hw/arm/allwinner-h3.h  |   3 +
>> >   include/hw/rtc/allwinner-rtc.h | 129 +++++++++++
>> >   hw/arm/allwinner-a10.c         |   7 +
>> >   hw/arm/allwinner-h3.c          |   9 +-
>> >   hw/rtc/allwinner-rtc.c         | 386 +++++++++++++++++++++++++++++++++
>> >   hw/rtc/Makefile.objs           |   1 +
>> >   hw/rtc/trace-events            |   4 +
>> >   8 files changed, 541 insertions(+), 2 deletions(-)
>> >   create mode 100644 include/hw/rtc/allwinner-rtc.h
>> >   create mode 100644 hw/rtc/allwinner-rtc.c
>> >
>> > diff --git a/include/hw/arm/allwinner-a10.h
>> b/include/hw/arm/allwinner-a10.h
>> > index 0e8250b244..81a16092e7 100644
>> > --- a/include/hw/arm/allwinner-a10.h
>> > +++ b/include/hw/arm/allwinner-a10.h
>> > @@ -9,6 +9,7 @@
>> >   #include "hw/net/allwinner_emac.h"
>> >   #include "hw/sd/allwinner-sdhost.h"
>> >   #include "hw/ide/ahci.h"
>> > +#include "hw/rtc/allwinner-rtc.h"
>> >
>> >   #include "target/arm/cpu.h"
>> >
>> > @@ -18,7 +19,7 @@
>> >   #define AW_A10_UART0_REG_BASE   0x01c28000
>> >   #define AW_A10_EMAC_BASE        0x01c0b000
>> >   #define AW_A10_SATA_BASE        0x01c18000
>> > -
>> > +#define AW_A10_RTC_BASE         0x01c20d00
>> >   #define AW_A10_SDRAM_BASE       0x40000000
>> >
>> >   #define TYPE_AW_A10 "allwinner-a10"
>> > @@ -36,6 +37,7 @@ typedef struct AwA10State {
>> >       AwEmacState emac;
>> >       AllwinnerAHCIState sata;
>> >       AwSdHostState mmc0;
>> > +    AwRtcState rtc;
>> >       MemoryRegion sram_a;
>> >   } AwA10State;
>> >
>> > diff --git a/include/hw/arm/allwinner-h3.h
>> b/include/hw/arm/allwinner-h3.h
>> > index d1b3d7ca67..1c275a34ed 100644
>> > --- a/include/hw/arm/allwinner-h3.h
>> > +++ b/include/hw/arm/allwinner-h3.h
>> > @@ -50,6 +50,7 @@
>> >   #include "hw/misc/allwinner-sid.h"
>> >   #include "hw/sd/allwinner-sdhost.h"
>> >   #include "hw/net/allwinner-sun8i-emac.h"
>> > +#include "hw/rtc/allwinner-rtc.h"
>> >   #include "target/arm/cpu.h"
>> >   #include "sysemu/block-backend.h"
>> >
>> > @@ -92,6 +93,7 @@ enum {
>> >       AW_H3_GIC_CPU,
>> >       AW_H3_GIC_HYP,
>> >       AW_H3_GIC_VCPU,
>> > +    AW_H3_RTC,
>> >       AW_H3_CPUCFG,
>> >       AW_H3_SDRAM
>> >   };
>> > @@ -130,6 +132,7 @@ typedef struct AwH3State {
>> >       AwSidState sid;
>> >       AwSdHostState mmc0;
>> >       AwSun8iEmacState emac;
>> > +    AwRtcState rtc;
>> >       GICState gic;
>> >       MemoryRegion sram_a1;
>> >       MemoryRegion sram_a2;
>> > diff --git a/include/hw/rtc/allwinner-rtc.h
>> b/include/hw/rtc/allwinner-rtc.h
>> > new file mode 100644
>> > index 0000000000..e29dfc775f
>> > --- /dev/null
>> > +++ b/include/hw/rtc/allwinner-rtc.h
>> > @@ -0,0 +1,129 @@
>> > +/*
>> > + * Allwinner Real Time Clock emulation
>> > + *
>> > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
>> > + *
>> > + * This program is free software: you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation, either version 2 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> > + * along with this program.  If not, see <http://www.gnu.org/licenses/
>> >.
>> > + */
>> > +
>> > +#ifndef HW_MISC_ALLWINNER_RTC_H
>> > +#define HW_MISC_ALLWINNER_RTC_H
>> > +
>> > +#include "qemu/osdep.h"
>> > +#include "qom/object.h"
>> > +#include "hw/sysbus.h"
>> > +
>> > +/**
>> > + * Constants
>> > + * @{
>> > + */
>> > +
>> > +/** Highest register address used by RTC device */
>> > +#define AW_RTC_REGS_MAXADDR     (0x1F4)
>>
>> I'd start using 0x200 here so in case new SoC use registers in this
>> block range (or undocumented reg) so we don't have to modify
>> allwinner_rtc_vmstate.
>>
>
> Good point, I'll change it to 0x200.
>
>
>> > +
>> > +/** Total number of known registers */
>> > +#define AW_RTC_REGS_NUM         (AW_RTC_REGS_MAXADDR /
>> sizeof(uint32_t))
>> > +
>> > +/** @} */
>> > +
>> > +/**
>> > + * Object model types
>> > + * @{
>> > + */
>> > +
>> > +/** Generic Allwinner RTC device (abstract) */
>> > +#define TYPE_AW_RTC          "allwinner-rtc"
>> > +
>> > +/** Allwinner RTC sun4i family (A10, A12) */
>> > +#define TYPE_AW_RTC_SUN4I    TYPE_AW_RTC "-sun4i"
>> > +
>> > +/** Allwinner RTC sun6i family and newer (A31, H2+, H3, etc) */
>> > +#define TYPE_AW_RTC_SUN6I    TYPE_AW_RTC "-sun6i"
>> > +
>> > +/** Allwinner RTC sun7i family (A20) */
>> > +#define TYPE_AW_RTC_SUN7I    TYPE_AW_RTC "-sun7i"
>> > +
>> > +/** @} */
>> > +
>> > +/**
>> > + * Object model macros
>> > + * @{
>> > + */
>> > +
>> > +#define AW_RTC(obj) \
>> > +    OBJECT_CHECK(AwRtcState, (obj), TYPE_AW_RTC)
>> > +#define AW_RTC_CLASS(klass) \
>> > +     OBJECT_CLASS_CHECK(AwRtcClass, (klass), TYPE_AW_RTC)
>> > +#define AW_RTC_GET_CLASS(obj) \
>> > +     OBJECT_GET_CLASS(AwRtcClass, (obj), TYPE_AW_RTC)
>> > +
>> > +/** @} */
>> > +
>> > +/**
>> > + * Allwinner RTC per-object instance state.
>> > + */
>> > +typedef struct AwRtcState {
>> > +    /*< private >*/
>> > +    SysBusDevice parent_obj;
>> > +    /*< public >*/
>> > +
>> > +    /** Maps I/O registers in physical memory */
>> > +    MemoryRegion iomem;
>> > +
>> > +    /** Array of hardware registers */
>> > +    uint32_t regs[AW_RTC_REGS_NUM];
>> > +
>> > +} AwRtcState;
>> > +
>> > +/**
>> > + * Allwinner RTC class-level struct.
>> > + *
>> > + * This struct is filled by each sunxi device specific code
>> > + * such that the generic code can use this struct to support
>> > + * all devices.
>> > + */
>> > +typedef struct AwRtcClass {
>> > +    /*< private >*/
>> > +    SysBusDeviceClass parent_class;
>> > +    /*< public >*/
>> > +
>> > +    /** Defines device specific register map */
>> > +    const uint8_t *regmap;
>> > +
>> > +    /** Number of entries in regmap */
>> > +    size_t regmap_size;
>>
>> I'd rather call this 'regmap_count'.
>>
> Sure, I'll name it regmap_count.
>
>
>> If you don't use this field, can we remove it?
>>
>
> Actually I think I made a mistake there, thanks for the catch!
> In fact it should be used to verify the guest does not read outside the
> AwRtcClass->regmap.
> I'll correct this in v4.
>
>
>> > +
>> > +    /** Device offset in years to 1900, for struct tm.tm_year */
>> > +    uint8_t year_offset;
>>
>> struct tm uses 'int' for this field.
>>
> OK.
>
> By the way, the behavior I noticed when reading the RTC from NetBSD versus
> Linux is due to a mismatch in
> the base year configured by the two operating systems. Linux starts
> counting from 1970 for the RTC in sun6i (drivers/rtc/rtc-sun6i.c,
> SUN6I_YEAR_MIN),
> while NetBSD uses 2000 as the base year
> (usr/src/sys/arch/arm/sunxi/sunxi_rtc.c, SUN6I_RTC_BASE_YEAR).
> So if the RTC year field is filled with the number 50, it results in 2020
> on Linux, but reads as 2050 on NetBSD.
>
> I'm not sure yet which one is correct. The datasheet does not explicitely
> mention what should be the base/start year,
> only that it is within the range 0..63.
>
>
>> > +
>> > +    /**
>> > +     * Read device specific register
>> > +     *
>> > +     * @offset: register offset to read
>> > +     * @return true if register read successful, false otherwise
>> > +     */
>> > +    bool (*read)(AwRtcState *s, uint32_t offset);
>> > +
>> > +    /**
>> > +     * Write device specific register
>> > +     *
>> > +     * @offset: register offset to write
>> > +     * @data: value to set in register
>> > +     * @return true if register write successful, false otherwise
>> > +     */
>> > +    bool (*write)(AwRtcState *s, uint32_t offset, uint32_t data);
>> > +
>> > +} AwRtcClass;
>> > +
>> > +#endif /* HW_MISC_ALLWINNER_RTC_H */
>> > diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>> > index 61cf3550a6..3f8f9d0d19 100644
>> > --- a/hw/arm/allwinner-a10.c
>> > +++ b/hw/arm/allwinner-a10.c
>> > @@ -46,6 +46,9 @@ static void aw_a10_init(Object *obj)
>> >
>> >       sysbus_init_child_obj(obj, "mmc0", &s->mmc0, sizeof(s->mmc0),
>> >                             TYPE_AW_SDHOST_SUN4I);
>> > +
>> > +    sysbus_init_child_obj(obj, "rtc", &s->rtc, sizeof(s->rtc),
>> > +                          TYPE_AW_RTC_SUN4I);
>> >   }
>> >
>> >   static void aw_a10_realize(DeviceState *dev, Error **errp)
>> > @@ -128,6 +131,10 @@ static void aw_a10_realize(DeviceState *dev, Error
>> **errp)
>> >       sysbus_connect_irq(SYS_BUS_DEVICE(&s->mmc0), 0, s->irq[32]);
>> >       object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(&s->mmc0),
>> >                                 "sd-bus", &error_abort);
>> > +
>> > +    /* RTC */
>> > +    qdev_init_nofail(DEVICE(&s->rtc));
>> > +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->rtc), 0,
>> AW_A10_RTC_BASE, 10);
>> >   }
>> >
>> >   static void aw_a10_class_init(ObjectClass *oc, void *data)
>> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
>> > index 77b823e7d8..caa4d8b196 100644
>> > --- a/hw/arm/allwinner-h3.c
>> > +++ b/hw/arm/allwinner-h3.c
>> > @@ -61,6 +61,7 @@ const hwaddr allwinner_h3_memmap[] = {
>> >       [AW_H3_GIC_CPU]    = 0x01c82000,
>> >       [AW_H3_GIC_HYP]    = 0x01c84000,
>> >       [AW_H3_GIC_VCPU]   = 0x01c86000,
>> > +    [AW_H3_RTC]        = 0x01f00000,
>> >       [AW_H3_CPUCFG]     = 0x01f01c00,
>> >       [AW_H3_SDRAM]      = 0x40000000
>> >   };
>> > @@ -116,7 +117,6 @@ struct AwH3Unimplemented {
>> >       { "csi",       0x01cb0000, 320 * KiB },
>> >       { "tve",       0x01e00000, 64 * KiB },
>> >       { "hdmi",      0x01ee0000, 128 * KiB },
>> > -    { "rtc",       0x01f00000, 1 * KiB },
>> >       { "r_timer",   0x01f00800, 1 * KiB },
>> >       { "r_intc",    0x01f00c00, 1 * KiB },
>> >       { "r_wdog",    0x01f01000, 1 * KiB },
>> > @@ -244,6 +244,9 @@ static void allwinner_h3_init(Object *obj)
>> >                                "ram-addr", &error_abort);
>> >       object_property_add_alias(obj, "ram-size", OBJECT(&s->dramc),
>> >                                 "ram-size", &error_abort);
>> > +
>> > +    sysbus_init_child_obj(obj, "rtc", &s->rtc, sizeof(s->rtc),
>> > +                          TYPE_AW_RTC_SUN6I);
>> >   }
>> >
>> >   static void allwinner_h3_realize(DeviceState *dev, Error **errp)
>> > @@ -437,6 +440,10 @@ static void allwinner_h3_realize(DeviceState *dev,
>> Error **errp)
>> >       sysbus_mmio_map(SYS_BUS_DEVICE(&s->dramc), 1,
>> s->memmap[AW_H3_DRAMCTL]);
>> >       sysbus_mmio_map(SYS_BUS_DEVICE(&s->dramc), 2,
>> s->memmap[AW_H3_DRAMPHY]);
>> >
>> > +    /* RTC */
>> > +    qdev_init_nofail(DEVICE(&s->rtc));
>> > +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, s->memmap[AW_H3_RTC]);
>> > +
>> >       /* Unimplemented devices */
>> >       for (int i = 0; i < ARRAY_SIZE(unimplemented); i++) {
>> >           create_unimplemented_device(unimplemented[i].device_name,
>> > diff --git a/hw/rtc/allwinner-rtc.c b/hw/rtc/allwinner-rtc.c
>> > new file mode 100644
>> > index 0000000000..812fe7f10b
>> > --- /dev/null
>> > +++ b/hw/rtc/allwinner-rtc.c
>> > @@ -0,0 +1,386 @@
>> > +/*
>> > + * Allwinner Real Time Clock emulation
>> > + *
>> > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com>
>> > + *
>> > + * This program is free software: you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation, either version 2 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU General Public License
>> > + * along with this program.  If not, see <http://www.gnu.org/licenses/
>> >.
>> > + */
>> > +
>> > +#include "qemu/osdep.h"
>> > +#include "qemu/units.h"
>> > +#include "hw/sysbus.h"
>> > +#include "migration/vmstate.h"
>> > +#include "qemu/log.h"
>> > +#include "qemu/module.h"
>> > +#include "qemu-common.h"
>> > +#include "hw/rtc/allwinner-rtc.h"
>> > +#include "trace.h"
>> > +
>> > +/* RTC registers */
>> > +enum {
>> > +    REG_LOSC = 1,        /* Low Oscillator Control */
>> > +    REG_YYMMDD,          /* RTC Year-Month-Day */
>> > +    REG_HHMMSS,          /* RTC Hour-Minute-Second */
>> > +    REG_ALARM1_WKHHMMSS, /* Alarm1 Week Hour-Minute-Second */
>> > +    REG_ALARM1_EN,       /* Alarm1 Enable */
>> > +    REG_ALARM1_IRQ_EN,   /* Alarm1 IRQ Enable */
>> > +    REG_ALARM1_IRQ_STA,  /* Alarm1 IRQ Status */
>> > +    REG_GP0,             /* General Purpose Register 0 */
>> > +    REG_GP1,             /* General Purpose Register 1 */
>> > +    REG_GP2,             /* General Purpose Register 2 */
>> > +    REG_GP3,             /* General Purpose Register 3 */
>> > +
>> > +    /* sun4i registers */
>> > +    REG_ALARM1_DDHHMMSS, /* Alarm1 Day Hour-Minute-Second */
>> > +    REG_CPUCFG,          /* CPU Configuration Register */
>> > +
>> > +    /* sun6i registers */
>> > +    REG_LOSC_AUTOSTA,    /* LOSC Auto Switch Status */
>> > +    REG_INT_OSC_PRE,     /* Internal OSC Clock Prescaler */
>> > +    REG_ALARM0_COUNTER,  /* Alarm0 Counter */
>> > +    REG_ALARM0_CUR_VLU,  /* Alarm0 Counter Current Value */
>> > +    REG_ALARM0_ENABLE,   /* Alarm0 Enable */
>> > +    REG_ALARM0_IRQ_EN,   /* Alarm0 IRQ Enable */
>> > +    REG_ALARM0_IRQ_STA,  /* Alarm0 IRQ Status */
>> > +    REG_ALARM_CONFIG,    /* Alarm Config */
>> > +    REG_LOSC_OUT_GATING, /* LOSC Output Gating Register */
>> > +    REG_GP4,             /* General Purpose Register 4 */
>> > +    REG_GP5,             /* General Purpose Register 5 */
>> > +    REG_GP6,             /* General Purpose Register 6 */
>> > +    REG_GP7,             /* General Purpose Register 7 */
>> > +    REG_RTC_DBG,         /* RTC Debug Register */
>> > +    REG_GPL_HOLD_OUT,    /* GPL Hold Output Register */
>> > +    REG_VDD_RTC,         /* VDD RTC Regulate Register */
>> > +    REG_IC_CHARA,        /* IC Characteristics Register */
>> > +};
>> > +
>> > +/* RTC register flags */
>> > +enum {
>> > +    REG_LOSC_YMD   = (1 << 7),
>> > +    REG_LOSC_HMS   = (1 << 8),
>> > +};
>> > +
>> > +/* RTC sun4i register map (offset to name) */
>> > +const uint8_t allwinner_rtc_sun4i_regmap[] = {
>> > +    [0x0000] = REG_LOSC,
>> > +    [0x0004] = REG_YYMMDD,
>> > +    [0x0008] = REG_HHMMSS,
>> > +    [0x000C] = REG_ALARM1_DDHHMMSS,
>> > +    [0x0010] = REG_ALARM1_WKHHMMSS,
>> > +    [0x0014] = REG_ALARM1_EN,
>> > +    [0x0018] = REG_ALARM1_IRQ_EN,
>> > +    [0x001C] = REG_ALARM1_IRQ_STA,
>> > +    [0x0020] = REG_GP0,
>> > +    [0x0024] = REG_GP1,
>> > +    [0x0028] = REG_GP2,
>> > +    [0x002C] = REG_GP3,
>> > +    [0x003C] = REG_CPUCFG,
>> > +};
>> > +
>> > +/* RTC sun6i register map (offset to name) */
>> > +const uint8_t allwinner_rtc_sun6i_regmap[] = {
>> > +    [0x0000] = REG_LOSC,
>> > +    [0x0004] = REG_LOSC_AUTOSTA,
>> > +    [0x0008] = REG_INT_OSC_PRE,
>> > +    [0x0010] = REG_YYMMDD,
>> > +    [0x0014] = REG_HHMMSS,
>> > +    [0x0020] = REG_ALARM0_COUNTER,
>> > +    [0x0024] = REG_ALARM0_CUR_VLU,
>> > +    [0x0028] = REG_ALARM0_ENABLE,
>> > +    [0x002C] = REG_ALARM0_IRQ_EN,
>> > +    [0x0030] = REG_ALARM0_IRQ_STA,
>> > +    [0x0040] = REG_ALARM1_WKHHMMSS,
>> > +    [0x0044] = REG_ALARM1_EN,
>> > +    [0x0048] = REG_ALARM1_IRQ_EN,
>> > +    [0x004C] = REG_ALARM1_IRQ_STA,
>> > +    [0x0050] = REG_ALARM_CONFIG,
>> > +    [0x0060] = REG_LOSC_OUT_GATING,
>> > +    [0x0100] = REG_GP0,
>> > +    [0x0104] = REG_GP1,
>> > +    [0x0108] = REG_GP2,
>> > +    [0x010C] = REG_GP3,
>> > +    [0x0110] = REG_GP4,
>> > +    [0x0114] = REG_GP5,
>> > +    [0x0118] = REG_GP6,
>> > +    [0x011C] = REG_GP7,
>> > +    [0x0170] = REG_RTC_DBG,
>> > +    [0x0180] = REG_GPL_HOLD_OUT,
>> > +    [0x0190] = REG_VDD_RTC,
>> > +    [0x01F0] = REG_IC_CHARA,
>> > +};
>> > +
>> > +static bool allwinner_rtc_sun4i_read(AwRtcState *s, uint32_t offset)
>> > +{
>> > +    /* no sun4i specific registers currently implemented */
>> > +    return false;
>> > +}
>> > +
>> > +static bool allwinner_rtc_sun4i_write(AwRtcState *s, uint32_t offset,
>> > +                                      uint32_t data)
>> > +{
>> > +    /* no sun4i specific registers currently implemented */
>> > +    return false;
>> > +}
>> > +
>> > +static bool allwinner_rtc_sun6i_read(AwRtcState *s, uint32_t offset)
>> > +{
>> > +    const AwRtcClass *c = AW_RTC_GET_CLASS(s);
>> > +
>> > +    switch (c->regmap[offset]) {
>> > +    case REG_GP4:             /* General Purpose Register 4 */
>> > +    case REG_GP5:             /* General Purpose Register 5 */
>> > +    case REG_GP6:             /* General Purpose Register 6 */
>> > +    case REG_GP7:             /* General Purpose Register 7 */
>> > +        return true;
>> > +    default:
>> > +        break;
>> > +    }
>> > +    return false;
>> > +}
>> > +
>> > +static bool allwinner_rtc_sun6i_write(AwRtcState *s, uint32_t offset,
>> > +                                      uint32_t data)
>> > +{
>> > +    const AwRtcClass *c = AW_RTC_GET_CLASS(s);
>> > +
>> > +    switch (c->regmap[offset]) {
>> > +    case REG_GP4:             /* General Purpose Register 4 */
>> > +    case REG_GP5:             /* General Purpose Register 5 */
>> > +    case REG_GP6:             /* General Purpose Register 6 */
>> > +    case REG_GP7:             /* General Purpose Register 7 */
>> > +        return true;
>> > +    default:
>> > +        break;
>> > +    }
>> > +    return false;
>> > +}
>> > +
>> > +static uint64_t allwinner_rtc_read(void *opaque, hwaddr offset,
>> > +                                   unsigned size)
>> > +{
>> > +    AwRtcState *s = AW_RTC(opaque);
>> > +    const AwRtcClass *c = AW_RTC_GET_CLASS(s);
>> > +    uint64_t val = 0;
>> > +
>> > +    if (offset >= AW_RTC_REGS_MAXADDR) {
>> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset
>> 0x%04x\n",
>> > +                      __func__, (uint32_t)offset);
>> > +        return 0;
>> > +    }
>> > +
>> > +    if (!c->regmap[offset]) {
>> > +            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid register
>> 0x%04x\n",
>> > +                          __func__, (uint32_t)offset);
>> > +        return 0;
>> > +    }
>> > +
>> > +    switch (c->regmap[offset]) {
>> > +    case REG_LOSC:       /* Low Oscillator Control */
>> > +        val = s->regs[REG_LOSC];
>> > +        s->regs[REG_LOSC] &= ~(REG_LOSC_YMD | REG_LOSC_HMS);
>> > +        break;
>> > +    case REG_YYMMDD:     /* RTC Year-Month-Day */
>> > +    case REG_HHMMSS:     /* RTC Hour-Minute-Second */
>> > +    case REG_GP0:        /* General Purpose Register 0 */
>> > +    case REG_GP1:        /* General Purpose Register 1 */
>> > +    case REG_GP2:        /* General Purpose Register 2 */
>> > +    case REG_GP3:        /* General Purpose Register 3 */
>> > +        val = s->regs[c->regmap[offset]];
>> > +        break;
>> > +    default:
>> > +        if (!c->read(s, offset)) {
>> > +            qemu_log_mask(LOG_UNIMP, "%s: unimplemented register
>> 0x%04x\n",
>> > +                          __func__, (uint32_t)offset);
>> > +        }
>> > +        val = s->regs[c->regmap[offset]];
>> > +        break;
>> > +    }
>> > +
>> > +    trace_allwinner_rtc_read(offset, val);
>> > +    return val;
>> > +}
>> > +
>> > +static void allwinner_rtc_write(void *opaque, hwaddr offset,
>> > +                                uint64_t val, unsigned size)
>> > +{
>> > +    AwRtcState *s = AW_RTC(opaque);
>> > +    const AwRtcClass *c = AW_RTC_GET_CLASS(s);
>> > +
>> > +    if (offset >= AW_RTC_REGS_MAXADDR) {
>> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds offset
>> 0x%04x\n",
>> > +                      __func__, (uint32_t)offset);
>> > +        return;
>> > +    }
>> > +
>> > +    if (!c->regmap[offset]) {
>> > +            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid register
>> 0x%04x\n",
>> > +                          __func__, (uint32_t)offset);
>> > +        return;
>> > +    }
>> > +
>> > +    trace_allwinner_rtc_write(offset, val);
>> > +
>> > +    switch (c->regmap[offset]) {
>> > +    case REG_YYMMDD:     /* RTC Year-Month-Day */
>> > +        s->regs[REG_YYMMDD] = val;
>> > +        s->regs[REG_LOSC]  |= REG_LOSC_YMD;
>> > +        break;
>> > +    case REG_HHMMSS:     /* RTC Hour-Minute-Second */
>> > +        s->regs[REG_HHMMSS] = val;
>> > +        s->regs[REG_LOSC]  |= REG_LOSC_HMS;
>> > +        break;
>> > +    case REG_GP0:        /* General Purpose Register 0 */
>> > +    case REG_GP1:        /* General Purpose Register 1 */
>> > +    case REG_GP2:        /* General Purpose Register 2 */
>> > +    case REG_GP3:        /* General Purpose Register 3 */
>> > +        s->regs[c->regmap[offset]] = val;
>> > +        break;
>> > +    default:
>> > +        if (!c->write(s, offset, val)) {
>> > +            qemu_log_mask(LOG_UNIMP, "%s: unimplemented register
>> 0x%04x\n",
>> > +                          __func__, (uint32_t)offset);
>> > +        }
>> > +        break;
>> > +    }
>> > +}
>> > +
>> > +static const MemoryRegionOps allwinner_rtc_ops = {
>> > +    .read = allwinner_rtc_read,
>> > +    .write = allwinner_rtc_write,
>> > +    .endianness = DEVICE_NATIVE_ENDIAN,
>> > +    .valid = {
>> > +        .min_access_size = 4,
>> > +        .max_access_size = 4,
>> > +    },
>> > +    .impl.min_access_size = 4,
>> > +};
>> > +
>> > +static void allwinner_rtc_reset(DeviceState *dev)
>> > +{
>> > +    AwRtcState *s = AW_RTC(dev);
>> > +    const AwRtcClass *c = AW_RTC_GET_CLASS(dev);
>> > +    struct tm now;
>> > +
>> > +    /* Clear registers */
>> > +    memset(s->regs, 0, sizeof(s->regs));
>> > +
>> > +    /* Get current datetime */
>> > +    qemu_get_timedate(&now, 0);
>> > +
>> > +    /* Set RTC with current datetime */
>> > +    s->regs[REG_YYMMDD] =  ((now.tm_year - c->year_offset) << 16) |
>> > +                           ((now.tm_mon + 1) << 8) |
>> > +                             now.tm_mday;
>> > +    s->regs[REG_HHMMSS] = (((now.tm_wday + 6) % 7) << 29) |
>> > +                              (now.tm_hour << 16) |
>> > +                              (now.tm_min << 8) |
>> > +                               now.tm_sec;
>>
>> This doesn't look correct.
>>
>>  From H3 Datasheet (Rev1.2):
>>    4.8.3.4. RTC YY-MM-DD Register (Default Value: 0x00000000)
>>    4.8.3.5. RTC HH-MM-SS Register (Default Value: 0x00000000)
>>
>
> I don't yet fully understand what you mean. Could you please explain a bit
> more what should be changed?
>
> For filling the YYMMDD and HHMMSS register fields, I simply looked at the
> 4.8.3.4 and 4.8.3.5 sections
> and filled it with the time retrieved from qemu_get_timedate. The shifts
> are done so the values are set in the proper bits.
> If I read it with the hwclock -r command under Linux, this reads out OK.
> On NetBSD, this works as well, except for the base year mismatch which I
> explained above.
>
>
>>
>> I'm not sure what is the proper to model this, maybe set this value in
>> init()? If we suspend a machine, migrate it, and resume it, what RTC are
>> we expecting?
>>
> I forgot to reply on this one.

I have not used migration very often, but I did manage to test it a couple
of times
using the 'migrate' command on the Qemu monitor console before I submitted
each
new version of the patch series. My expectation would be that the RTC time
is suspended
along with all the other state of the machine such as memory, I/O devices
etc. So that would mean
the time is 'frozen' until resumed. I think that is what we currently have
here.

Do you think that is correct or should it work differently?



>
>> Rest of the patch looks good.
>>
> Thanks for your feedback Philippe!
> I'm processing it for v4.
>
> Regards,
> Niek
>
>>
>> > +}
>> > +
>> > +static void allwinner_rtc_init(Object *obj)
>> > +{
>> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> > +    AwRtcState *s = AW_RTC(obj);
>> > +
>> > +    /* Memory mapping */
>> > +    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_rtc_ops, s,
>> > +                          TYPE_AW_RTC, 1 * KiB);
>> > +    sysbus_init_mmio(sbd, &s->iomem);
>> > +}
>> > +
>> > +static const VMStateDescription allwinner_rtc_vmstate = {
>> > +    .name = "allwinner-rtc",
>> > +    .version_id = 1,
>> > +    .minimum_version_id = 1,
>> > +    .fields = (VMStateField[]) {
>> > +        VMSTATE_UINT32_ARRAY(regs, AwRtcState, AW_RTC_REGS_NUM),
>> > +        VMSTATE_END_OF_LIST()
>> > +    }
>> > +};
>> > +
>> > +static void allwinner_rtc_class_init(ObjectClass *klass, void *data)
>> > +{
>> > +    DeviceClass *dc = DEVICE_CLASS(klass);
>> > +
>> > +    dc->reset = allwinner_rtc_reset;
>> > +    dc->vmsd = &allwinner_rtc_vmstate;
>> > +}
>> > +
>> > +static void allwinner_rtc_sun4i_class_init(ObjectClass *klass, void
>> *data)
>> > +{
>> > +    AwRtcClass *arc = AW_RTC_CLASS(klass);
>> > +
>> > +    arc->regmap = allwinner_rtc_sun4i_regmap;
>> > +    arc->regmap_size = ARRAY_SIZE(allwinner_rtc_sun4i_regmap);
>> > +    arc->year_offset = 110;
>> > +    arc->read = allwinner_rtc_sun4i_read;
>> > +    arc->write = allwinner_rtc_sun4i_write;
>> > +}
>> > +
>> > +static void allwinner_rtc_sun6i_class_init(ObjectClass *klass, void
>> *data)
>> > +{
>> > +    AwRtcClass *arc = AW_RTC_CLASS(klass);
>> > +
>> > +    arc->regmap = allwinner_rtc_sun6i_regmap;
>> > +    arc->regmap_size = ARRAY_SIZE(allwinner_rtc_sun6i_regmap);
>> > +    arc->year_offset = 70;
>> > +    arc->read = allwinner_rtc_sun6i_read;
>> > +    arc->write = allwinner_rtc_sun6i_write;
>> > +}
>> > +
>> > +static void allwinner_rtc_sun7i_class_init(ObjectClass *klass, void
>> *data)
>> > +{
>> > +    AwRtcClass *arc = AW_RTC_CLASS(klass);
>> > +
>> > +    allwinner_rtc_sun4i_class_init(klass, arc);
>> > +    arc->year_offset = 70;
>> > +}
>> > +
>> > +static const TypeInfo allwinner_rtc_info = {
>> > +    .name          = TYPE_AW_RTC,
>> > +    .parent        = TYPE_SYS_BUS_DEVICE,
>> > +    .instance_init = allwinner_rtc_init,
>> > +    .instance_size = sizeof(AwRtcState),
>> > +    .class_init    = allwinner_rtc_class_init,
>> > +    .class_size    = sizeof(AwRtcClass),
>> > +    .abstract      = true,
>> > +};
>> > +
>> > +static const TypeInfo allwinner_rtc_sun4i_info = {
>> > +    .name          = TYPE_AW_RTC_SUN4I,
>> > +    .parent        = TYPE_AW_RTC,
>> > +    .class_init    = allwinner_rtc_sun4i_class_init,
>> > +};
>> > +
>> > +static const TypeInfo allwinner_rtc_sun6i_info = {
>> > +    .name          = TYPE_AW_RTC_SUN6I,
>> > +    .parent        = TYPE_AW_RTC,
>> > +    .class_init    = allwinner_rtc_sun6i_class_init,
>> > +};
>> > +
>> > +static const TypeInfo allwinner_rtc_sun7i_info = {
>> > +    .name          = TYPE_AW_RTC_SUN7I,
>> > +    .parent        = TYPE_AW_RTC,
>> > +    .class_init    = allwinner_rtc_sun7i_class_init,
>> > +};
>> > +
>> > +static void allwinner_rtc_register(void)
>> > +{
>> > +    type_register_static(&allwinner_rtc_info);
>> > +    type_register_static(&allwinner_rtc_sun4i_info);
>> > +    type_register_static(&allwinner_rtc_sun6i_info);
>> > +    type_register_static(&allwinner_rtc_sun7i_info);
>> > +}
>> > +
>> > +type_init(allwinner_rtc_register)
>> > diff --git a/hw/rtc/Makefile.objs b/hw/rtc/Makefile.objs
>> > index 8dc9fcd3a9..12d92feebf 100644
>> > --- a/hw/rtc/Makefile.objs
>> > +++ b/hw/rtc/Makefile.objs
>> > @@ -11,3 +11,4 @@ common-obj-$(CONFIG_EXYNOS4) += exynos4210_rtc.o
>> >   obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
>> >   common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
>> >   common-obj-$(CONFIG_ASPEED_SOC) += aspeed_rtc.o
>> > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-rtc.o
>> > diff --git a/hw/rtc/trace-events b/hw/rtc/trace-events
>> > index d6749f4616..eb57de3bd6 100644
>> > --- a/hw/rtc/trace-events
>> > +++ b/hw/rtc/trace-events
>> > @@ -1,5 +1,9 @@
>> >   # See docs/devel/tracing.txt for syntax documentation.
>> >
>> > +# allwinner-rtc.c
>> > +allwinner_rtc_read(uint64_t addr, uint64_t value) "addr 0x%" PRIx64 "
>> value 0x%" PRIx64
>> > +allwinner_rtc_write(uint64_t addr, uint64_t value) "addr 0x%" PRIx64 "
>> value 0x%" PRIx64
>> > +
>> >   # sun4v-rtc.c
>> >   sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64
>> " value 0x%" PRIx64
>> >   sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%"
>> PRIx64 " value 0x%" PRIx64
>> >
>>
>>
>
> --
> Niek Linnenbank
>
>

-- 
Niek Linnenbank

[-- Attachment #2: Type: text/html, Size: 38645 bytes --]