Hi Philippe,

On Mon, Jan 13, 2020 at 11:57 PM Philippe Mathieu-Daudé <phi...@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 <nieklinnenb...@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 <nieklinnenb...@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 <nieklinnenb...@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?
>
> 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

Reply via email to