All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Basic Allwinner WDT emulation
@ 2023-03-11 14:41 Strahinja Jankovic
  2023-03-11 14:41 ` [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset Strahinja Jankovic
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-11 14:41 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel,
	Strahinja Jankovic

This patch set introduces basic emulation of Allwinner WDT.
Since WDT in both A10 and H3 is part of Timer module, the WDT
functionality is added as an overlay in the memory map.

The focus was to enable reboot functionality, so WDT interrupt handling
is not covered in this patch set.

With these patches the `reboot` command can be used for both Cubieboard
and Orangepi-PC in order to restart the system.

Also, Cubieboard avocado tests have been improved to include reboot
steps as well.


Strahinja Jankovic (4):
  hw/watchdog: Allwinner WDT emulation for system reset
  hw/arm: Add WDT to Allwinner-A10 and Cubieboard
  hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC
  tests/avocado: Add reboot tests to Cubieboard

 docs/system/arm/cubieboard.rst      |   1 +
 docs/system/arm/orangepi.rst        |   1 +
 hw/arm/Kconfig                      |   2 +
 hw/arm/allwinner-a10.c              |   7 +
 hw/arm/allwinner-h3.c               |   8 +
 hw/watchdog/Kconfig                 |   4 +
 hw/watchdog/allwinner-wdt.c         | 428 ++++++++++++++++++++++++++++
 hw/watchdog/meson.build             |   1 +
 hw/watchdog/trace-events            |   7 +
 include/hw/arm/allwinner-a10.h      |   2 +
 include/hw/arm/allwinner-h3.h       |   5 +-
 include/hw/watchdog/allwinner-wdt.h | 123 ++++++++
 tests/avocado/boot_linux_console.py |  15 +-
 13 files changed, 600 insertions(+), 4 deletions(-)
 create mode 100644 hw/watchdog/allwinner-wdt.c
 create mode 100644 include/hw/watchdog/allwinner-wdt.h

-- 
2.30.2



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset
  2023-03-11 14:41 [PATCH 0/4] Basic Allwinner WDT emulation Strahinja Jankovic
@ 2023-03-11 14:41 ` Strahinja Jankovic
  2023-03-14 21:14   ` Niek Linnenbank
  2023-03-11 14:41 ` [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard Strahinja Jankovic
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-11 14:41 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel,
	Strahinja Jankovic

This patch adds basic support for Allwinner WDT.
Both sun4i and sun6i variants are supported.
However, interrupt generation is not supported, so WDT can be used only to trigger system reset.

Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
---
 hw/watchdog/Kconfig                 |   4 +
 hw/watchdog/allwinner-wdt.c         | 428 ++++++++++++++++++++++++++++
 hw/watchdog/meson.build             |   1 +
 hw/watchdog/trace-events            |   7 +
 include/hw/watchdog/allwinner-wdt.h | 123 ++++++++
 5 files changed, 563 insertions(+)
 create mode 100644 hw/watchdog/allwinner-wdt.c
 create mode 100644 include/hw/watchdog/allwinner-wdt.h

diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
index 66e1d029e3..861fd00334 100644
--- a/hw/watchdog/Kconfig
+++ b/hw/watchdog/Kconfig
@@ -20,3 +20,7 @@ config WDT_IMX2
 
 config WDT_SBSA
     bool
+
+config ALLWINNER_WDT
+    bool
+    select PTIMER
diff --git a/hw/watchdog/allwinner-wdt.c b/hw/watchdog/allwinner-wdt.c
new file mode 100644
index 0000000000..cf16ec7a56
--- /dev/null
+++ b/hw/watchdog/allwinner-wdt.c
@@ -0,0 +1,428 @@
+/*
+ * Allwinner Watchdog emulation
+ *
+ * Copyright (C) 2023 Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
+ *
+ *  This file is derived from Allwinner RTC,
+ *  by Niek Linnenbank.
+ *
+ * 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/log.h"
+#include "qemu/units.h"
+#include "qemu/module.h"
+#include "trace.h"
+#include "hw/sysbus.h"
+#include "hw/registerfields.h"
+#include "hw/watchdog/allwinner-wdt.h"
+#include "sysemu/watchdog.h"
+#include "migration/vmstate.h"
+
+/* WDT registers */
+enum {
+    REG_IRQ_EN = 0,     /* Watchdog interrupt enable */
+    REG_IRQ_STA,        /* Watchdog interrupt status */
+    REG_CTRL,           /* Watchdog control register */
+    REG_CFG,            /* Watchdog configuration register */
+    REG_MODE,           /* Watchdog mode register */
+};
+
+/* Universal WDT register flags */
+#define WDT_RESTART_MASK    (1 << 0)
+#define WDT_EN_MASK         (1 << 0)
+
+/* sun4i specific WDT register flags */
+#define RST_EN_SUN4I_MASK       (1 << 1)
+#define INTV_VALUE_SUN4I_SHIFT  (3)
+#define INTV_VALUE_SUN4I_MASK   (0xfu << INTV_VALUE_SUN4I_SHIFT)
+
+/* sun6i specific WDT register flags */
+#define RST_EN_SUN6I_MASK       (1 << 0)
+#define KEY_FIELD_SUN6I_SHIFT   (1)
+#define KEY_FIELD_SUN6I_MASK    (0xfffu << KEY_FIELD_SUN6I_SHIFT)
+#define KEY_FIELD_SUN6I         (0xA57u)
+#define INTV_VALUE_SUN6I_SHIFT  (4)
+#define INTV_VALUE_SUN6I_MASK   (0xfu << INTV_VALUE_SUN6I_SHIFT)
+
+/* Map of INTV_VALUE to 0.5s units. */
+static const uint8_t allwinner_wdt_count_map[] = {
+    1,
+    2,
+    4,
+    6,
+    8,
+    10,
+    12,
+    16,
+    20,
+    24,
+    28,
+    32
+};
+
+/* WDT sun4i register map (offset to name) */
+const uint8_t allwinner_wdt_sun4i_regmap[] = {
+    [0x0000] = REG_CTRL,
+    [0x0004] = REG_MODE,
+};
+
+/* WDT sun6i register map (offset to name) */
+const uint8_t allwinner_wdt_sun6i_regmap[] = {
+    [0x0000] = REG_IRQ_EN,
+    [0x0004] = REG_IRQ_STA,
+    [0x0010] = REG_CTRL,
+    [0x0014] = REG_CFG,
+    [0x0018] = REG_MODE,
+};
+
+static bool allwinner_wdt_sun4i_read(AwWdtState *s, uint32_t offset)
+{
+    /* no sun4i specific registers currently implemented */
+    return false;
+}
+
+static bool allwinner_wdt_sun4i_write(AwWdtState *s, uint32_t offset,
+                                      uint32_t data)
+{
+    /* no sun4i specific registers currently implemented */
+    return false;
+}
+
+static bool allwinner_wdt_sun4i_can_reset_system(AwWdtState *s)
+{
+    if (s->regs[REG_MODE] & RST_EN_SUN6I_MASK) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+static bool allwinner_wdt_sun4i_is_key_valid(AwWdtState *s, uint32_t val)
+{
+    /* sun4i has no key */
+    return true;
+}
+
+static uint8_t allwinner_wdt_sun4i_get_intv_value(AwWdtState *s)
+{
+    return ((s->regs[REG_MODE] & INTV_VALUE_SUN4I_MASK) >>
+            INTV_VALUE_SUN4I_SHIFT);
+}
+
+static bool allwinner_wdt_sun6i_read(AwWdtState *s, uint32_t offset)
+{
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+
+    switch (c->regmap[offset]) {
+    case REG_IRQ_EN:
+    case REG_IRQ_STA:
+    case REG_CFG:
+        return true;
+    default:
+        break;
+    }
+    return false;
+}
+
+static bool allwinner_wdt_sun6i_write(AwWdtState *s, uint32_t offset,
+                                      uint32_t data)
+{
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+
+    switch (c->regmap[offset]) {
+    case REG_IRQ_EN:
+    case REG_IRQ_STA:
+    case REG_CFG:
+        return true;
+    default:
+        break;
+    }
+    return false;
+}
+
+static bool allwinner_wdt_sun6i_can_reset_system(AwWdtState *s)
+{
+    if (s->regs[REG_CFG] & RST_EN_SUN6I_MASK) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+static bool allwinner_wdt_sun6i_is_key_valid(AwWdtState *s, uint32_t val)
+{
+    uint16_t key = (val & KEY_FIELD_SUN6I_MASK) >> KEY_FIELD_SUN6I_SHIFT;
+    return (key == KEY_FIELD_SUN6I);
+}
+
+static uint8_t allwinner_wdt_sun6i_get_intv_value(AwWdtState *s)
+{
+    return ((s->regs[REG_MODE] & INTV_VALUE_SUN6I_MASK) >>
+            INTV_VALUE_SUN6I_SHIFT);
+}
+
+static void allwinner_wdt_update_timer(AwWdtState *s)
+{
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+    uint8_t count = c->get_intv_value(s);
+
+    ptimer_transaction_begin(s->timer);
+    ptimer_stop(s->timer);
+
+    /* Use map to convert. */
+    if (count < sizeof(allwinner_wdt_count_map)) {
+        ptimer_set_count(s->timer, allwinner_wdt_count_map[count]);
+    } else {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect INTV_VALUE 0x%02x\n",
+                __func__, count);
+    }
+
+    ptimer_run(s->timer, 1);
+    ptimer_transaction_commit(s->timer);
+
+    trace_allwinner_wdt_update_timer(count);
+}
+
+static uint64_t allwinner_wdt_read(void *opaque, hwaddr offset,
+                                       unsigned size)
+{
+    AwWdtState *s = AW_WDT(opaque);
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+    uint64_t r;
+
+    if (offset >= c->regmap_size) {
+        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_CTRL:
+    case REG_MODE:
+        r = 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);
+            return 0;
+        }
+        r = s->regs[c->regmap[offset]];
+        break;
+    }
+
+    trace_allwinner_wdt_read(offset, r, size);
+
+    return r;
+}
+
+static void allwinner_wdt_write(void *opaque, hwaddr offset,
+                                   uint64_t val, unsigned size)
+{
+    AwWdtState *s = AW_WDT(opaque);
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+    uint32_t old_val;
+
+    if (offset >= c->regmap_size) {
+        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_wdt_write(offset, val, size);
+
+    switch (c->regmap[offset]) {
+    case REG_CTRL:
+        if (c->is_key_valid(s, val)) {
+            if (val & WDT_RESTART_MASK) {
+                /* Kick timer */
+                allwinner_wdt_update_timer(s);
+            }
+        }
+        break;
+    case REG_MODE:
+        old_val = s->regs[REG_MODE];
+        s->regs[REG_MODE] = (uint32_t)val;
+
+        /* Check for rising edge on WDOG_MODE_EN */
+        if ((s->regs[REG_MODE] & ~old_val) & WDT_EN_MASK) {
+            allwinner_wdt_update_timer(s);
+        }
+        break;
+    default:
+        if (!c->write(s, offset, val)) {
+            qemu_log_mask(LOG_UNIMP, "%s: unimplemented register 0x%04x\n",
+                          __func__, (uint32_t)offset);
+        }
+        s->regs[c->regmap[offset]] = (uint32_t)val;
+        break;
+    }
+}
+
+static const MemoryRegionOps allwinner_wdt_ops = {
+    .read = allwinner_wdt_read,
+    .write = allwinner_wdt_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .impl.min_access_size = 4,
+};
+
+static void allwinner_wdt_expired(void *opaque)
+{
+    AwWdtState *s = AW_WDT(opaque);
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+
+    bool enabled = s->regs[REG_MODE] & WDT_EN_MASK;
+    bool reset_enabled = c->can_reset_system(s);
+
+    trace_allwinner_wdt_expired(enabled, reset_enabled);
+
+    /* Perform watchdog action if watchdog is enabled and can trigger reset */
+    if (enabled && reset_enabled) {
+        watchdog_perform_action();
+    }
+}
+
+static void allwinner_wdt_reset_enter(Object *obj, ResetType type)
+{
+    AwWdtState *s = AW_WDT(obj);
+
+    trace_allwinner_wdt_reset_enter();
+
+    /* Clear registers */
+    memset(s->regs, 0, sizeof(s->regs));
+}
+
+static const VMStateDescription allwinner_wdt_vmstate = {
+    .name = "allwinner-wdt",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PTIMER(timer, AwWdtState),
+        VMSTATE_UINT32_ARRAY(regs, AwWdtState, AW_WDT_REGS_NUM),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void allwinner_wdt_init(Object *obj)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    AwWdtState *s = AW_WDT(obj);
+    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
+
+    /* Memory mapping */
+    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_wdt_ops, s,
+                          TYPE_AW_WDT, c->regmap_size * 4);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void allwinner_wdt_realize(DeviceState *dev, Error **errp)
+{
+    AwWdtState *s = AW_WDT(dev);
+
+    s->timer = ptimer_init(allwinner_wdt_expired, s,
+                           PTIMER_POLICY_NO_IMMEDIATE_TRIGGER |
+                           PTIMER_POLICY_NO_IMMEDIATE_RELOAD |
+                           PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
+
+    ptimer_transaction_begin(s->timer);
+    /* Set to 2Hz (0.5s period) */
+    ptimer_set_freq(s->timer, 2);
+    ptimer_set_limit(s->timer, 0xff, 1);
+    ptimer_transaction_commit(s->timer);
+}
+
+static void allwinner_wdt_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    rc->phases.enter = allwinner_wdt_reset_enter;
+    dc->realize = allwinner_wdt_realize;
+    dc->vmsd = &allwinner_wdt_vmstate;
+}
+
+static void allwinner_wdt_sun4i_class_init(ObjectClass *klass, void *data)
+{
+    AwWdtClass *awc = AW_WDT_CLASS(klass);
+
+    awc->regmap = allwinner_wdt_sun4i_regmap;
+    awc->regmap_size = sizeof(allwinner_wdt_sun4i_regmap);
+    awc->read = allwinner_wdt_sun4i_read;
+    awc->write = allwinner_wdt_sun4i_write;
+    awc->can_reset_system = allwinner_wdt_sun4i_can_reset_system;
+    awc->is_key_valid = allwinner_wdt_sun4i_is_key_valid;
+    awc->get_intv_value = allwinner_wdt_sun4i_get_intv_value;
+}
+
+static void allwinner_wdt_sun6i_class_init(ObjectClass *klass, void *data)
+{
+    AwWdtClass *awc = AW_WDT_CLASS(klass);
+
+    awc->regmap = allwinner_wdt_sun6i_regmap;
+    awc->regmap_size = sizeof(allwinner_wdt_sun6i_regmap);
+    awc->read = allwinner_wdt_sun6i_read;
+    awc->write = allwinner_wdt_sun6i_write;
+    awc->can_reset_system = allwinner_wdt_sun6i_can_reset_system;
+    awc->is_key_valid = allwinner_wdt_sun6i_is_key_valid;
+    awc->get_intv_value = allwinner_wdt_sun6i_get_intv_value;
+}
+
+static const TypeInfo allwinner_wdt_info = {
+    .name          = TYPE_AW_WDT,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_init = allwinner_wdt_init,
+    .instance_size = sizeof(AwWdtState),
+    .class_init    = allwinner_wdt_class_init,
+    .class_size    = sizeof(AwWdtClass),
+    .abstract      = true,
+};
+
+static const TypeInfo allwinner_wdt_sun4i_info = {
+    .name          = TYPE_AW_WDT_SUN4I,
+    .parent        = TYPE_AW_WDT,
+    .class_init    = allwinner_wdt_sun4i_class_init,
+};
+
+static const TypeInfo allwinner_wdt_sun6i_info = {
+    .name          = TYPE_AW_WDT_SUN6I,
+    .parent        = TYPE_AW_WDT,
+    .class_init    = allwinner_wdt_sun6i_class_init,
+};
+
+static void allwinner_wdt_register(void)
+{
+    type_register_static(&allwinner_wdt_info);
+    type_register_static(&allwinner_wdt_sun4i_info);
+    type_register_static(&allwinner_wdt_sun6i_info);
+}
+
+type_init(allwinner_wdt_register)
diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
index 8974b5cf4c..5dcd4fbe2f 100644
--- a/hw/watchdog/meson.build
+++ b/hw/watchdog/meson.build
@@ -1,4 +1,5 @@
 softmmu_ss.add(files('watchdog.c'))
+softmmu_ss.add(when: 'CONFIG_ALLWINNER_WDT', if_true: files('allwinner-wdt.c'))
 softmmu_ss.add(when: 'CONFIG_CMSDK_APB_WATCHDOG', if_true: files('cmsdk-apb-watchdog.c'))
 softmmu_ss.add(when: 'CONFIG_WDT_IB6300ESB', if_true: files('wdt_i6300esb.c'))
 softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true: files('wdt_ib700.c'))
diff --git a/hw/watchdog/trace-events b/hw/watchdog/trace-events
index 54371ae075..b1227860c4 100644
--- a/hw/watchdog/trace-events
+++ b/hw/watchdog/trace-events
@@ -1,5 +1,12 @@
 # See docs/devel/tracing.rst for syntax documentation.
 
+# allwinner-wdt.c
+allwinner_wdt_read(uint64_t offset, uint64_t data, unsigned size) "Allwinner watchdog read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
+allwinner_wdt_write(uint64_t offset, uint64_t data, unsigned size) "Allwinner watchdog write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
+allwinner_wdt_reset_enter(void) "Allwinner watchdog: reset"
+allwinner_wdt_update_timer(uint32_t count) "Allwinner watchdog: count %" PRIu32
+allwinner_wdt_expired(bool enabled, bool reset_enabled) "Allwinner watchdog: enabled %u reset_enabled %u"
+
 # cmsdk-apb-watchdog.c
 cmsdk_apb_watchdog_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB watchdog read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_watchdog_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB watchdog write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
diff --git a/include/hw/watchdog/allwinner-wdt.h b/include/hw/watchdog/allwinner-wdt.h
new file mode 100644
index 0000000000..37b49e77ee
--- /dev/null
+++ b/include/hw/watchdog/allwinner-wdt.h
@@ -0,0 +1,123 @@
+/*
+ * Allwinner Watchdog emulation
+ *
+ * Copyright (C) 2023 Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
+ *
+ *  This file is derived from Allwinner RTC,
+ *  by Niek Linnenbank.
+ *
+ * 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/>.
+ */
+/*
+ * This is a model of the Allwinner watchdog.
+ * Since watchdog registers belong to the timer module (and are shared with the
+ * RTC module), the interrupt line from watchdog is not handled right now.
+ * In QEMU, we just wire up the watchdog reset to watchdog_perform_action(),
+ * at least for the moment.
+ */
+
+#ifndef HW_WATCHDOG_ALLWINNER_WDT_H
+#define HW_WATCHDOG_ALLWINNER_WDT_H
+
+#include "qom/object.h"
+#include "hw/ptimer.h"
+#include "hw/sysbus.h"
+
+
+#define TYPE_AW_WDT    "allwinner-wdt"
+
+/** Allwinner WDT sun4i family (A10, A12), also sun7i (A20) */
+#define TYPE_AW_WDT_SUN4I    TYPE_AW_WDT "-sun4i"
+
+/** Allwinner WDT sun6i family and newer (A31, H2+, H3, etc) */
+#define TYPE_AW_WDT_SUN6I    TYPE_AW_WDT "-sun6i"
+
+/** Number of WDT registers */
+#define AW_WDT_REGS_NUM      (5)
+
+OBJECT_DECLARE_TYPE(AwWdtState, AwWdtClass, AW_WDT)
+
+/**
+ * Allwinner A10 WDT object instance state.
+ */
+struct AwWdtState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    MemoryRegion iomem;
+    struct ptimer_state *timer;
+
+    uint32_t regs[AW_WDT_REGS_NUM];
+};
+
+/**
+ * Allwinner WDT 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.
+ */
+struct AwWdtClass {
+    /*< private >*/
+    SysBusDeviceClass parent_class;
+    /*< public >*/
+
+    /** Defines device specific register map */
+    const uint8_t *regmap;
+
+    /** Size of the regmap in bytes */
+    size_t regmap_size;
+
+    /**
+     * Read device specific register
+     *
+     * @offset: register offset to read
+     * @return true if register read successful, false otherwise
+     */
+    bool (*read)(AwWdtState *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)(AwWdtState *s, uint32_t offset, uint32_t data);
+
+    /**
+     * Check if watchdog can generate system reset
+     *
+     * @return true if watchdog can generate system reset
+     */
+    bool (*can_reset_system)(AwWdtState *s);
+
+    /**
+     * Check if provided key is valid
+     *
+     * @value: value written to register
+     * @return true if key is valid, false otherwise
+     */
+    bool (*is_key_valid)(AwWdtState *s, uint32_t val);
+
+    /**
+     * Get current INTV_VALUE setting
+     *
+     * @return current INTV_VALUE (0-15)
+     */
+    uint8_t (*get_intv_value)(AwWdtState *s);
+};
+
+#endif /* HW_WATCHDOG_ALLWINNER_WDT_H */
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard
  2023-03-11 14:41 [PATCH 0/4] Basic Allwinner WDT emulation Strahinja Jankovic
  2023-03-11 14:41 ` [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset Strahinja Jankovic
@ 2023-03-11 14:41 ` Strahinja Jankovic
  2023-03-14 21:21   ` Niek Linnenbank
  2023-03-11 14:41 ` [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC Strahinja Jankovic
  2023-03-11 14:41 ` [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard Strahinja Jankovic
  3 siblings, 1 reply; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-11 14:41 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel,
	Strahinja Jankovic

This patch adds WDT to Allwinner-A10 and Cubieboard.
WDT is added as an overlay to the Timer module memory map.

Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
---
 docs/system/arm/cubieboard.rst | 1 +
 hw/arm/Kconfig                 | 1 +
 hw/arm/allwinner-a10.c         | 7 +++++++
 include/hw/arm/allwinner-a10.h | 2 ++
 4 files changed, 11 insertions(+)

diff --git a/docs/system/arm/cubieboard.rst b/docs/system/arm/cubieboard.rst
index 8d485f5435..58c4a2d3ea 100644
--- a/docs/system/arm/cubieboard.rst
+++ b/docs/system/arm/cubieboard.rst
@@ -15,3 +15,4 @@ Emulated devices:
 - USB controller
 - SATA controller
 - TWI (I2C) controller
+- Watchdog timer
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..ec15248536 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -325,6 +325,7 @@ config ALLWINNER_A10
     select ALLWINNER_A10_PIC
     select ALLWINNER_A10_CCM
     select ALLWINNER_A10_DRAMC
+    select ALLWINNER_WDT
     select ALLWINNER_EMAC
     select ALLWINNER_I2C
     select AXP209_PMU
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index b7ca795c71..b0ea3f7f66 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -38,6 +38,7 @@
 #define AW_A10_EHCI_BASE        0x01c14000
 #define AW_A10_OHCI_BASE        0x01c14400
 #define AW_A10_SATA_BASE        0x01c18000
+#define AW_A10_WDT_BASE         0x01c20c90
 #define AW_A10_RTC_BASE         0x01c20d00
 #define AW_A10_I2C0_BASE        0x01c2ac00
 
@@ -92,6 +93,8 @@ static void aw_a10_init(Object *obj)
     object_initialize_child(obj, "mmc0", &s->mmc0, TYPE_AW_SDHOST_SUN4I);
 
     object_initialize_child(obj, "rtc", &s->rtc, TYPE_AW_RTC_SUN4I);
+
+    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN4I);
 }
 
 static void aw_a10_realize(DeviceState *dev, Error **errp)
@@ -203,6 +206,10 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
     sysbus_realize(SYS_BUS_DEVICE(&s->i2c0), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c0), 0, AW_A10_I2C0_BASE);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c0), 0, qdev_get_gpio_in(dev, 7));
+
+    /* WDT */
+    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0, AW_A10_WDT_BASE, 1);
 }
 
 static void aw_a10_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
index 095afb225d..cd1465c613 100644
--- a/include/hw/arm/allwinner-a10.h
+++ b/include/hw/arm/allwinner-a10.h
@@ -13,6 +13,7 @@
 #include "hw/misc/allwinner-a10-ccm.h"
 #include "hw/misc/allwinner-a10-dramc.h"
 #include "hw/i2c/allwinner-i2c.h"
+#include "hw/watchdog/allwinner-wdt.h"
 #include "sysemu/block-backend.h"
 
 #include "target/arm/cpu.h"
@@ -41,6 +42,7 @@ struct AwA10State {
     AwSdHostState mmc0;
     AWI2CState i2c0;
     AwRtcState rtc;
+    AwWdtState wdt;
     MemoryRegion sram_a;
     EHCISysBusState ehci[AW_A10_NUM_USB];
     OHCISysBusState ohci[AW_A10_NUM_USB];
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC
  2023-03-11 14:41 [PATCH 0/4] Basic Allwinner WDT emulation Strahinja Jankovic
  2023-03-11 14:41 ` [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset Strahinja Jankovic
  2023-03-11 14:41 ` [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard Strahinja Jankovic
@ 2023-03-11 14:41 ` Strahinja Jankovic
  2023-03-13  7:53   ` Philippe Mathieu-Daudé
  2023-03-14 21:29   ` Niek Linnenbank
  2023-03-11 14:41 ` [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard Strahinja Jankovic
  3 siblings, 2 replies; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-11 14:41 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel,
	Strahinja Jankovic

This patch adds WDT to Allwinner-H3 and Orangepi-PC.
WDT is added as an overlay to the Timer module memory area.

Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
---
 docs/system/arm/orangepi.rst  | 1 +
 hw/arm/Kconfig                | 1 +
 hw/arm/allwinner-h3.c         | 8 ++++++++
 include/hw/arm/allwinner-h3.h | 5 ++++-
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/docs/system/arm/orangepi.rst b/docs/system/arm/orangepi.rst
index e5973600a1..9afa54213b 100644
--- a/docs/system/arm/orangepi.rst
+++ b/docs/system/arm/orangepi.rst
@@ -26,6 +26,7 @@ The Orange Pi PC machine supports the following devices:
  * System Control module
  * Security Identifier device
  * TWI (I2C)
+ * Watchdog timer
 
 Limitations
 """""""""""
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index ec15248536..7d916f5450 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -337,6 +337,7 @@ config ALLWINNER_H3
     select ALLWINNER_A10_PIT
     select ALLWINNER_SUN8I_EMAC
     select ALLWINNER_I2C
+    select ALLWINNER_WDT
     select SERIAL
     select ARM_TIMER
     select ARM_GIC
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index 69d0ad6f50..f05afddf7e 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -49,6 +49,7 @@ const hwaddr allwinner_h3_memmap[] = {
     [AW_H3_DEV_OHCI3]      = 0x01c1d400,
     [AW_H3_DEV_CCU]        = 0x01c20000,
     [AW_H3_DEV_PIT]        = 0x01c20c00,
+    [AW_H3_DEV_WDT]        = 0x01c20ca0,
     [AW_H3_DEV_UART0]      = 0x01c28000,
     [AW_H3_DEV_UART1]      = 0x01c28400,
     [AW_H3_DEV_UART2]      = 0x01c28800,
@@ -234,6 +235,8 @@ static void allwinner_h3_init(Object *obj)
     object_initialize_child(obj, "twi1",  &s->i2c1,  TYPE_AW_I2C_SUN6I);
     object_initialize_child(obj, "twi2",  &s->i2c2,  TYPE_AW_I2C_SUN6I);
     object_initialize_child(obj, "r_twi", &s->r_twi, TYPE_AW_I2C_SUN6I);
+
+    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN6I);
 }
 
 static void allwinner_h3_realize(DeviceState *dev, Error **errp)
@@ -453,6 +456,11 @@ static void allwinner_h3_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->r_twi), 0,
                        qdev_get_gpio_in(DEVICE(&s->gic), AW_H3_GIC_SPI_R_TWI));
 
+    /* WDT */
+    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0,
+                            s->memmap[AW_H3_DEV_WDT], 1);
+
     /* Unimplemented devices */
     for (i = 0; i < ARRAY_SIZE(unimplemented); i++) {
         create_unimplemented_device(unimplemented[i].device_name,
diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h
index 59e0f822d2..f15d6d7cc7 100644
--- a/include/hw/arm/allwinner-h3.h
+++ b/include/hw/arm/allwinner-h3.h
@@ -48,6 +48,7 @@
 #include "hw/net/allwinner-sun8i-emac.h"
 #include "hw/rtc/allwinner-rtc.h"
 #include "hw/i2c/allwinner-i2c.h"
+#include "hw/watchdog/allwinner-wdt.h"
 #include "target/arm/cpu.h"
 #include "sysemu/block-backend.h"
 
@@ -96,7 +97,8 @@ enum {
     AW_H3_DEV_RTC,
     AW_H3_DEV_CPUCFG,
     AW_H3_DEV_R_TWI,
-    AW_H3_DEV_SDRAM
+    AW_H3_DEV_SDRAM,
+    AW_H3_DEV_WDT
 };
 
 /** Total number of CPU cores in the H3 SoC */
@@ -141,6 +143,7 @@ struct AwH3State {
     AWI2CState r_twi;
     AwSun8iEmacState emac;
     AwRtcState rtc;
+    AwWdtState wdt;
     GICState gic;
     MemoryRegion sram_a1;
     MemoryRegion sram_a2;
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard
  2023-03-11 14:41 [PATCH 0/4] Basic Allwinner WDT emulation Strahinja Jankovic
                   ` (2 preceding siblings ...)
  2023-03-11 14:41 ` [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC Strahinja Jankovic
@ 2023-03-11 14:41 ` Strahinja Jankovic
  2023-03-14 20:02   ` Niek Linnenbank
  3 siblings, 1 reply; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-11 14:41 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel,
	Strahinja Jankovic

Cubieboard tests end with comment "reboot not functioning; omit test".
Fix this so reboot is done at the end of each test.

Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
---
 tests/avocado/boot_linux_console.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tests/avocado/boot_linux_console.py b/tests/avocado/boot_linux_console.py
index 574609bf43..c0675809e6 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -581,7 +581,10 @@ def test_arm_cubieboard_initrd(self):
                                                 'Allwinner sun4i/sun5i')
         exec_command_and_wait_for_pattern(self, 'cat /proc/iomem',
                                                 'system-control@1c00000')
-        # cubieboard's reboot is not functioning; omit reboot test.
+        exec_command_and_wait_for_pattern(self, 'reboot',
+                                                'reboot: Restarting system')
+        # Wait for VM to shut down gracefully
+        self.vm.wait()
 
     def test_arm_cubieboard_sata(self):
         """
@@ -625,7 +628,10 @@ def test_arm_cubieboard_sata(self):
                                                 'Allwinner sun4i/sun5i')
         exec_command_and_wait_for_pattern(self, 'cat /proc/partitions',
                                                 'sda')
-        # cubieboard's reboot is not functioning; omit reboot test.
+        exec_command_and_wait_for_pattern(self, 'reboot',
+                                                'reboot: Restarting system')
+        # Wait for VM to shut down gracefully
+        self.vm.wait()
 
     @skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
     def test_arm_cubieboard_openwrt_22_03_2(self):
@@ -672,7 +678,10 @@ def test_arm_cubieboard_openwrt_22_03_2(self):
 
         exec_command_and_wait_for_pattern(self, 'cat /proc/cpuinfo',
                                                 'Allwinner sun4i/sun5i')
-        # cubieboard's reboot is not functioning; omit reboot test.
+        exec_command_and_wait_for_pattern(self, 'reboot',
+                                                'reboot: Restarting system')
+        # Wait for VM to shut down gracefully
+        self.vm.wait()
 
     @skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
     def test_arm_quanta_gsj(self):
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC
  2023-03-11 14:41 ` [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC Strahinja Jankovic
@ 2023-03-13  7:53   ` Philippe Mathieu-Daudé
  2023-03-13 15:15     ` Strahinja Jankovic
  2023-03-14 21:29   ` Niek Linnenbank
  1 sibling, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-13  7:53 UTC (permalink / raw)
  To: Strahinja Jankovic, Peter Maydell
  Cc: Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel,
	Strahinja Jankovic

Hi,

On 11/3/23 15:41, Strahinja Jankovic wrote:
> This patch adds WDT to Allwinner-H3 and Orangepi-PC.
> WDT is added as an overlay to the Timer module memory area.
> 
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> ---
>   docs/system/arm/orangepi.rst  | 1 +
>   hw/arm/Kconfig                | 1 +
>   hw/arm/allwinner-h3.c         | 8 ++++++++
>   include/hw/arm/allwinner-h3.h | 5 ++++-
>   4 files changed, 14 insertions(+), 1 deletion(-)


> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> index 69d0ad6f50..f05afddf7e 100644
> --- a/hw/arm/allwinner-h3.c
> +++ b/hw/arm/allwinner-h3.c
> @@ -49,6 +49,7 @@ const hwaddr allwinner_h3_memmap[] = {
>       [AW_H3_DEV_OHCI3]      = 0x01c1d400,
>       [AW_H3_DEV_CCU]        = 0x01c20000,
>       [AW_H3_DEV_PIT]        = 0x01c20c00,
> +    [AW_H3_DEV_WDT]        = 0x01c20ca0,
>       [AW_H3_DEV_UART0]      = 0x01c28000,
>       [AW_H3_DEV_UART1]      = 0x01c28400,
>       [AW_H3_DEV_UART2]      = 0x01c28800,
> @@ -234,6 +235,8 @@ static void allwinner_h3_init(Object *obj)
>       object_initialize_child(obj, "twi1",  &s->i2c1,  TYPE_AW_I2C_SUN6I);
>       object_initialize_child(obj, "twi2",  &s->i2c2,  TYPE_AW_I2C_SUN6I);
>       object_initialize_child(obj, "r_twi", &s->r_twi, TYPE_AW_I2C_SUN6I);
> +
> +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN6I);
>   }
>   
>   static void allwinner_h3_realize(DeviceState *dev, Error **errp)
> @@ -453,6 +456,11 @@ static void allwinner_h3_realize(DeviceState *dev, Error **errp)
>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->r_twi), 0,
>                          qdev_get_gpio_in(DEVICE(&s->gic), AW_H3_GIC_SPI_R_TWI));
>   
> +    /* WDT */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0,
> +                            s->memmap[AW_H3_DEV_WDT], 1);

Why do you need to overlap?



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC
  2023-03-13  7:53   ` Philippe Mathieu-Daudé
@ 2023-03-13 15:15     ` Strahinja Jankovic
  0 siblings, 0 replies; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-13 15:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Beniamino Galvani, Niek Linnenbank, qemu-arm, qemu-devel

Hi,

On Mon, Mar 13, 2023 at 8:53 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi,
>
> On 11/3/23 15:41, Strahinja Jankovic wrote:
> > This patch adds WDT to Allwinner-H3 and Orangepi-PC.
> > WDT is added as an overlay to the Timer module memory area.
> >
> > Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> > ---
> >   docs/system/arm/orangepi.rst  | 1 +
> >   hw/arm/Kconfig                | 1 +
> >   hw/arm/allwinner-h3.c         | 8 ++++++++
> >   include/hw/arm/allwinner-h3.h | 5 ++++-
> >   4 files changed, 14 insertions(+), 1 deletion(-)
>
>
> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> > index 69d0ad6f50..f05afddf7e 100644
> > --- a/hw/arm/allwinner-h3.c
> > +++ b/hw/arm/allwinner-h3.c
> > @@ -49,6 +49,7 @@ const hwaddr allwinner_h3_memmap[] = {
> >       [AW_H3_DEV_OHCI3]      = 0x01c1d400,
> >       [AW_H3_DEV_CCU]        = 0x01c20000,
> >       [AW_H3_DEV_PIT]        = 0x01c20c00,
> > +    [AW_H3_DEV_WDT]        = 0x01c20ca0,
> >       [AW_H3_DEV_UART0]      = 0x01c28000,
> >       [AW_H3_DEV_UART1]      = 0x01c28400,
> >       [AW_H3_DEV_UART2]      = 0x01c28800,
> > @@ -234,6 +235,8 @@ static void allwinner_h3_init(Object *obj)
> >       object_initialize_child(obj, "twi1",  &s->i2c1,  TYPE_AW_I2C_SUN6I);
> >       object_initialize_child(obj, "twi2",  &s->i2c2,  TYPE_AW_I2C_SUN6I);
> >       object_initialize_child(obj, "r_twi", &s->r_twi, TYPE_AW_I2C_SUN6I);
> > +
> > +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN6I);
> >   }
> >
> >   static void allwinner_h3_realize(DeviceState *dev, Error **errp)
> > @@ -453,6 +456,11 @@ static void allwinner_h3_realize(DeviceState *dev, Error **errp)
> >       sysbus_connect_irq(SYS_BUS_DEVICE(&s->r_twi), 0,
> >                          qdev_get_gpio_in(DEVICE(&s->gic), AW_H3_GIC_SPI_R_TWI));
> >
> > +    /* WDT */
> > +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
> > +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0,
> > +                            s->memmap[AW_H3_DEV_WDT], 1);
>
> Why do you need to overlap?
>

The WDT is part of the Timer component and it lies in the part of
memory already initialized by the AW_H3_DEV_PIT.
I saw the overlay approach used for RTC in Allwinner A10 (same issue,
in A10 both RTC and WDT are part of Timer), so I just reused it.

If there is a better way to handle it, I can update the implementation.

Best regards,
Strahinja


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard
  2023-03-11 14:41 ` [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard Strahinja Jankovic
@ 2023-03-14 20:02   ` Niek Linnenbank
  0 siblings, 0 replies; 13+ messages in thread
From: Niek Linnenbank @ 2023-03-14 20:02 UTC (permalink / raw)
  To: Strahinja Jankovic
  Cc: Peter Maydell, Beniamino Galvani, qemu-arm, qemu-devel,
	Strahinja Jankovic

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

Hi Strahinja,

Looks good! I re-ran the tests on my machine, and they work fine:

ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes
./build/tests/venv/bin/avocado --show=app,console run -t
machine:orangepi-pc -t machine:cubieboard
tests/avocado/boot_linux_console.py
...
|console: Tue Mar 14 19:56:37 UTC 2023
\console: Starting root file system check:
PASS (22.45 s)
RESULTS    : PASS 8 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
CANCEL 0
JOB TIME   : 189.23 s

Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>

On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <
strahinjapjankovic@gmail.com> wrote:

> Cubieboard tests end with comment "reboot not functioning; omit test".
> Fix this so reboot is done at the end of each test.
>
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> ---
>  tests/avocado/boot_linux_console.py | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/tests/avocado/boot_linux_console.py
> b/tests/avocado/boot_linux_console.py
> index 574609bf43..c0675809e6 100644
> --- a/tests/avocado/boot_linux_console.py
> +++ b/tests/avocado/boot_linux_console.py
> @@ -581,7 +581,10 @@ def test_arm_cubieboard_initrd(self):
>                                                  'Allwinner sun4i/sun5i')
>          exec_command_and_wait_for_pattern(self, 'cat /proc/iomem',
>                                                  'system-control@1c00000')
> -        # cubieboard's reboot is not functioning; omit reboot test.
> +        exec_command_and_wait_for_pattern(self, 'reboot',
> +                                                'reboot: Restarting
> system')
> +        # Wait for VM to shut down gracefully
> +        self.vm.wait()
>
>      def test_arm_cubieboard_sata(self):
>          """
> @@ -625,7 +628,10 @@ def test_arm_cubieboard_sata(self):
>                                                  'Allwinner sun4i/sun5i')
>          exec_command_and_wait_for_pattern(self, 'cat /proc/partitions',
>                                                  'sda')
> -        # cubieboard's reboot is not functioning; omit reboot test.
> +        exec_command_and_wait_for_pattern(self, 'reboot',
> +                                                'reboot: Restarting
> system')
> +        # Wait for VM to shut down gracefully
> +        self.vm.wait()
>
>      @skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage
> limited')
>      def test_arm_cubieboard_openwrt_22_03_2(self):
> @@ -672,7 +678,10 @@ def test_arm_cubieboard_openwrt_22_03_2(self):
>
>          exec_command_and_wait_for_pattern(self, 'cat /proc/cpuinfo',
>                                                  'Allwinner sun4i/sun5i')
> -        # cubieboard's reboot is not functioning; omit reboot test.
> +        exec_command_and_wait_for_pattern(self, 'reboot',
> +                                                'reboot: Restarting
> system')
> +        # Wait for VM to shut down gracefully
> +        self.vm.wait()
>
>      @skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might
> timeout')
>      def test_arm_quanta_gsj(self):
> --
> 2.30.2
>
>

-- 
Niek Linnenbank

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset
  2023-03-11 14:41 ` [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset Strahinja Jankovic
@ 2023-03-14 21:14   ` Niek Linnenbank
  2023-03-15 21:59     ` Strahinja Jankovic
  0 siblings, 1 reply; 13+ messages in thread
From: Niek Linnenbank @ 2023-03-14 21:14 UTC (permalink / raw)
  To: Strahinja Jankovic
  Cc: Peter Maydell, Beniamino Galvani, qemu-arm, qemu-devel,
	Strahinja Jankovic

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

Hi Strahinja,


On Sat, Mar 11, 2023 at 3:41 PM Strahinja Jankovic <
strahinjapjankovic@gmail.com> wrote:

> This patch adds basic support for Allwinner WDT.
> Both sun4i and sun6i variants are supported.
> However, interrupt generation is not supported, so WDT can be used only to
> trigger system reset.
>
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> ---
>  hw/watchdog/Kconfig                 |   4 +
>  hw/watchdog/allwinner-wdt.c         | 428 ++++++++++++++++++++++++++++
>  hw/watchdog/meson.build             |   1 +
>  hw/watchdog/trace-events            |   7 +
>  include/hw/watchdog/allwinner-wdt.h | 123 ++++++++
>  5 files changed, 563 insertions(+)
>  create mode 100644 hw/watchdog/allwinner-wdt.c
>  create mode 100644 include/hw/watchdog/allwinner-wdt.h
>
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index 66e1d029e3..861fd00334 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -20,3 +20,7 @@ config WDT_IMX2
>
>  config WDT_SBSA
>      bool
> +
> +config ALLWINNER_WDT
> +    bool
> +    select PTIMER
> diff --git a/hw/watchdog/allwinner-wdt.c b/hw/watchdog/allwinner-wdt.c
> new file mode 100644
> index 0000000000..cf16ec7a56
> --- /dev/null
> +++ b/hw/watchdog/allwinner-wdt.c
> @@ -0,0 +1,428 @@
> +/*
> + * Allwinner Watchdog emulation
> + *
> + * Copyright (C) 2023 Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> + *
> + *  This file is derived from Allwinner RTC,
> + *  by Niek Linnenbank.
> + *
> + * 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/log.h"
> +#include "qemu/units.h"
> +#include "qemu/module.h"
> +#include "trace.h"
> +#include "hw/sysbus.h"
> +#include "hw/registerfields.h"
> +#include "hw/watchdog/allwinner-wdt.h"
> +#include "sysemu/watchdog.h"
> +#include "migration/vmstate.h"
> +
> +/* WDT registers */
> +enum {
> +    REG_IRQ_EN = 0,     /* Watchdog interrupt enable */
>

Since we are doing a check "if (!c->regmap[offset])" below, should the enum
values begin with 1 instead?


> +    REG_IRQ_STA,        /* Watchdog interrupt status */
> +    REG_CTRL,           /* Watchdog control register */
> +    REG_CFG,            /* Watchdog configuration register */
> +    REG_MODE,           /* Watchdog mode register */
> +};
> +
> +/* Universal WDT register flags */
> +#define WDT_RESTART_MASK    (1 << 0)
> +#define WDT_EN_MASK         (1 << 0)
> +
> +/* sun4i specific WDT register flags */
> +#define RST_EN_SUN4I_MASK       (1 << 1)
> +#define INTV_VALUE_SUN4I_SHIFT  (3)
> +#define INTV_VALUE_SUN4I_MASK   (0xfu << INTV_VALUE_SUN4I_SHIFT)
> +
> +/* sun6i specific WDT register flags */
> +#define RST_EN_SUN6I_MASK       (1 << 0)
> +#define KEY_FIELD_SUN6I_SHIFT   (1)
> +#define KEY_FIELD_SUN6I_MASK    (0xfffu << KEY_FIELD_SUN6I_SHIFT)
> +#define KEY_FIELD_SUN6I         (0xA57u)
> +#define INTV_VALUE_SUN6I_SHIFT  (4)
> +#define INTV_VALUE_SUN6I_MASK   (0xfu << INTV_VALUE_SUN6I_SHIFT)
> +
> +/* Map of INTV_VALUE to 0.5s units. */
> +static const uint8_t allwinner_wdt_count_map[] = {
> +    1,
> +    2,
> +    4,
> +    6,
> +    8,
> +    10,
> +    12,
> +    16,
> +    20,
> +    24,
> +    28,
> +    32
> +};
> +
> +/* WDT sun4i register map (offset to name) */
> +const uint8_t allwinner_wdt_sun4i_regmap[] = {
> +    [0x0000] = REG_CTRL,
> +    [0x0004] = REG_MODE,
> +};
> +
> +/* WDT sun6i register map (offset to name) */
> +const uint8_t allwinner_wdt_sun6i_regmap[] = {
> +    [0x0000] = REG_IRQ_EN,
> +    [0x0004] = REG_IRQ_STA,
> +    [0x0010] = REG_CTRL,
> +    [0x0014] = REG_CFG,
> +    [0x0018] = REG_MODE,
> +};
> +
> +static bool allwinner_wdt_sun4i_read(AwWdtState *s, uint32_t offset)
> +{
> +    /* no sun4i specific registers currently implemented */
> +    return false;
> +}
> +
> +static bool allwinner_wdt_sun4i_write(AwWdtState *s, uint32_t offset,
> +                                      uint32_t data)
> +{
> +    /* no sun4i specific registers currently implemented */
> +    return false;
> +}
> +
> +static bool allwinner_wdt_sun4i_can_reset_system(AwWdtState *s)
> +{
> +    if (s->regs[REG_MODE] & RST_EN_SUN6I_MASK) {
> +        return true;
> +    } else {
> +        return false;
> +    }
> +}
> +
> +static bool allwinner_wdt_sun4i_is_key_valid(AwWdtState *s, uint32_t val)
> +{
> +    /* sun4i has no key */
> +    return true;
> +}
> +
> +static uint8_t allwinner_wdt_sun4i_get_intv_value(AwWdtState *s)
> +{
> +    return ((s->regs[REG_MODE] & INTV_VALUE_SUN4I_MASK) >>
> +            INTV_VALUE_SUN4I_SHIFT);
> +}
> +
> +static bool allwinner_wdt_sun6i_read(AwWdtState *s, uint32_t offset)
> +{
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +
> +    switch (c->regmap[offset]) {
> +    case REG_IRQ_EN:
> +    case REG_IRQ_STA:
> +    case REG_CFG:
> +        return true;
> +    default:
> +        break;
> +    }
> +    return false;
> +}
> +
> +static bool allwinner_wdt_sun6i_write(AwWdtState *s, uint32_t offset,
> +                                      uint32_t data)
> +{
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +
> +    switch (c->regmap[offset]) {
> +    case REG_IRQ_EN:
> +    case REG_IRQ_STA:
> +    case REG_CFG:
> +        return true;
> +    default:
> +        break;
> +    }
> +    return false;
> +}
> +
> +static bool allwinner_wdt_sun6i_can_reset_system(AwWdtState *s)
> +{
> +    if (s->regs[REG_CFG] & RST_EN_SUN6I_MASK) {
> +        return true;
> +    } else {
> +        return false;
> +    }
> +}
> +
> +static bool allwinner_wdt_sun6i_is_key_valid(AwWdtState *s, uint32_t val)
> +{
> +    uint16_t key = (val & KEY_FIELD_SUN6I_MASK) >> KEY_FIELD_SUN6I_SHIFT;
> +    return (key == KEY_FIELD_SUN6I);
> +}
> +
> +static uint8_t allwinner_wdt_sun6i_get_intv_value(AwWdtState *s)
> +{
> +    return ((s->regs[REG_MODE] & INTV_VALUE_SUN6I_MASK) >>
> +            INTV_VALUE_SUN6I_SHIFT);
> +}
> +
> +static void allwinner_wdt_update_timer(AwWdtState *s)
> +{
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +    uint8_t count = c->get_intv_value(s);
> +
> +    ptimer_transaction_begin(s->timer);
> +    ptimer_stop(s->timer);
> +
> +    /* Use map to convert. */
> +    if (count < sizeof(allwinner_wdt_count_map)) {
> +        ptimer_set_count(s->timer, allwinner_wdt_count_map[count]);
> +    } else {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect INTV_VALUE
> 0x%02x\n",
> +                __func__, count);
> +    }
> +
> +    ptimer_run(s->timer, 1);
> +    ptimer_transaction_commit(s->timer);
> +
> +    trace_allwinner_wdt_update_timer(count);
> +}
> +
> +static uint64_t allwinner_wdt_read(void *opaque, hwaddr offset,
> +                                       unsigned size)
> +{
> +    AwWdtState *s = AW_WDT(opaque);
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +    uint64_t r;
> +
> +    if (offset >= c->regmap_size) {
> +        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_CTRL:
> +    case REG_MODE:
> +        r = 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);
> +            return 0;
> +        }
> +        r = s->regs[c->regmap[offset]];
> +        break;
> +    }
> +
> +    trace_allwinner_wdt_read(offset, r, size);
> +
> +    return r;
> +}
> +
> +static void allwinner_wdt_write(void *opaque, hwaddr offset,
> +                                   uint64_t val, unsigned size)
> +{
> +    AwWdtState *s = AW_WDT(opaque);
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +    uint32_t old_val;
> +
> +    if (offset >= c->regmap_size) {
> +        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_wdt_write(offset, val, size);
> +
> +    switch (c->regmap[offset]) {
> +    case REG_CTRL:
> +        if (c->is_key_valid(s, val)) {
> +            if (val & WDT_RESTART_MASK) {
> +                /* Kick timer */
> +                allwinner_wdt_update_timer(s);
> +            }
> +        }
> +        break;
> +    case REG_MODE:
> +        old_val = s->regs[REG_MODE];
> +        s->regs[REG_MODE] = (uint32_t)val;
> +
> +        /* Check for rising edge on WDOG_MODE_EN */
> +        if ((s->regs[REG_MODE] & ~old_val) & WDT_EN_MASK) {
> +            allwinner_wdt_update_timer(s);
> +        }
> +        break;
> +    default:
> +        if (!c->write(s, offset, val)) {
> +            qemu_log_mask(LOG_UNIMP, "%s: unimplemented register
> 0x%04x\n",
> +                          __func__, (uint32_t)offset);
> +        }
> +        s->regs[c->regmap[offset]] = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps allwinner_wdt_ops = {
> +    .read = allwinner_wdt_read,
> +    .write = allwinner_wdt_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .impl.min_access_size = 4,
> +};
> +
> +static void allwinner_wdt_expired(void *opaque)
> +{
> +    AwWdtState *s = AW_WDT(opaque);
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +
> +    bool enabled = s->regs[REG_MODE] & WDT_EN_MASK;
> +    bool reset_enabled = c->can_reset_system(s);
> +
> +    trace_allwinner_wdt_expired(enabled, reset_enabled);
> +
> +    /* Perform watchdog action if watchdog is enabled and can trigger
> reset */
> +    if (enabled && reset_enabled) {
> +        watchdog_perform_action();
> +    }
> +}
> +
> +static void allwinner_wdt_reset_enter(Object *obj, ResetType type)
> +{
> +    AwWdtState *s = AW_WDT(obj);
> +
> +    trace_allwinner_wdt_reset_enter();
> +
> +    /* Clear registers */
> +    memset(s->regs, 0, sizeof(s->regs));
> +}
> +
> +static const VMStateDescription allwinner_wdt_vmstate = {
> +    .name = "allwinner-wdt",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_PTIMER(timer, AwWdtState),
> +        VMSTATE_UINT32_ARRAY(regs, AwWdtState, AW_WDT_REGS_NUM),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void allwinner_wdt_init(Object *obj)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    AwWdtState *s = AW_WDT(obj);
> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
> +
> +    /* Memory mapping */
> +    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_wdt_ops, s,
> +                          TYPE_AW_WDT, c->regmap_size * 4);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static void allwinner_wdt_realize(DeviceState *dev, Error **errp)
> +{
> +    AwWdtState *s = AW_WDT(dev);
> +
> +    s->timer = ptimer_init(allwinner_wdt_expired, s,
> +                           PTIMER_POLICY_NO_IMMEDIATE_TRIGGER |
> +                           PTIMER_POLICY_NO_IMMEDIATE_RELOAD |
> +                           PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
> +
> +    ptimer_transaction_begin(s->timer);
> +    /* Set to 2Hz (0.5s period) */
> +    ptimer_set_freq(s->timer, 2);
>

Just wondering here where the 0.5s / 2Hz as the value come from?
I do see that wdt_imx2.c is also using the same value, but no code comment
is provided to clarify.


> +    ptimer_set_limit(s->timer, 0xff, 1);
> +    ptimer_transaction_commit(s->timer);
> +}
> +
> +static void allwinner_wdt_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> +
> +    rc->phases.enter = allwinner_wdt_reset_enter;
> +    dc->realize = allwinner_wdt_realize;
> +    dc->vmsd = &allwinner_wdt_vmstate;
> +}
> +
> +static void allwinner_wdt_sun4i_class_init(ObjectClass *klass, void *data)
> +{
> +    AwWdtClass *awc = AW_WDT_CLASS(klass);
> +
> +    awc->regmap = allwinner_wdt_sun4i_regmap;
> +    awc->regmap_size = sizeof(allwinner_wdt_sun4i_regmap);
> +    awc->read = allwinner_wdt_sun4i_read;
> +    awc->write = allwinner_wdt_sun4i_write;
> +    awc->can_reset_system = allwinner_wdt_sun4i_can_reset_system;
> +    awc->is_key_valid = allwinner_wdt_sun4i_is_key_valid;
> +    awc->get_intv_value = allwinner_wdt_sun4i_get_intv_value;
> +}
> +
> +static void allwinner_wdt_sun6i_class_init(ObjectClass *klass, void *data)
> +{
> +    AwWdtClass *awc = AW_WDT_CLASS(klass);
> +
> +    awc->regmap = allwinner_wdt_sun6i_regmap;
> +    awc->regmap_size = sizeof(allwinner_wdt_sun6i_regmap);
> +    awc->read = allwinner_wdt_sun6i_read;
> +    awc->write = allwinner_wdt_sun6i_write;
> +    awc->can_reset_system = allwinner_wdt_sun6i_can_reset_system;
> +    awc->is_key_valid = allwinner_wdt_sun6i_is_key_valid;
> +    awc->get_intv_value = allwinner_wdt_sun6i_get_intv_value;
> +}
> +
> +static const TypeInfo allwinner_wdt_info = {
> +    .name          = TYPE_AW_WDT,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_init = allwinner_wdt_init,
> +    .instance_size = sizeof(AwWdtState),
> +    .class_init    = allwinner_wdt_class_init,
> +    .class_size    = sizeof(AwWdtClass),
> +    .abstract      = true,
> +};
> +
> +static const TypeInfo allwinner_wdt_sun4i_info = {
> +    .name          = TYPE_AW_WDT_SUN4I,
> +    .parent        = TYPE_AW_WDT,
> +    .class_init    = allwinner_wdt_sun4i_class_init,
> +};
> +
> +static const TypeInfo allwinner_wdt_sun6i_info = {
> +    .name          = TYPE_AW_WDT_SUN6I,
> +    .parent        = TYPE_AW_WDT,
> +    .class_init    = allwinner_wdt_sun6i_class_init,
> +};
> +
> +static void allwinner_wdt_register(void)
> +{
> +    type_register_static(&allwinner_wdt_info);
> +    type_register_static(&allwinner_wdt_sun4i_info);
> +    type_register_static(&allwinner_wdt_sun6i_info);
> +}
> +
> +type_init(allwinner_wdt_register)
> diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
> index 8974b5cf4c..5dcd4fbe2f 100644
> --- a/hw/watchdog/meson.build
> +++ b/hw/watchdog/meson.build
> @@ -1,4 +1,5 @@
>  softmmu_ss.add(files('watchdog.c'))
> +softmmu_ss.add(when: 'CONFIG_ALLWINNER_WDT', if_true:
> files('allwinner-wdt.c'))
>  softmmu_ss.add(when: 'CONFIG_CMSDK_APB_WATCHDOG', if_true:
> files('cmsdk-apb-watchdog.c'))
>  softmmu_ss.add(when: 'CONFIG_WDT_IB6300ESB', if_true:
> files('wdt_i6300esb.c'))
>  softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true: files('wdt_ib700.c'))
> diff --git a/hw/watchdog/trace-events b/hw/watchdog/trace-events
> index 54371ae075..b1227860c4 100644
> --- a/hw/watchdog/trace-events
> +++ b/hw/watchdog/trace-events
> @@ -1,5 +1,12 @@
>  # See docs/devel/tracing.rst for syntax documentation.
>
> +# allwinner-wdt.c
> +allwinner_wdt_read(uint64_t offset, uint64_t data, unsigned size)
> "Allwinner watchdog read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
> +allwinner_wdt_write(uint64_t offset, uint64_t data, unsigned size)
> "Allwinner watchdog write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
> +allwinner_wdt_reset_enter(void) "Allwinner watchdog: reset"
> +allwinner_wdt_update_timer(uint32_t count) "Allwinner watchdog: count %"
> PRIu32
>

should the type for count be uint8_t and PRIu8 instead? In the function
allwinner_wdt_update_timer() above,
the variable 'uint8_t count' is passed, which is filled by the call to
c->get_intv_value().


> +allwinner_wdt_expired(bool enabled, bool reset_enabled) "Allwinner
> watchdog: enabled %u reset_enabled %u"
> +
>  # cmsdk-apb-watchdog.c
>  cmsdk_apb_watchdog_read(uint64_t offset, uint64_t data, unsigned size)
> "CMSDK APB watchdog read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_watchdog_write(uint64_t offset, uint64_t data, unsigned size)
> "CMSDK APB watchdog write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
> diff --git a/include/hw/watchdog/allwinner-wdt.h
> b/include/hw/watchdog/allwinner-wdt.h
> new file mode 100644
> index 0000000000..37b49e77ee
> --- /dev/null
> +++ b/include/hw/watchdog/allwinner-wdt.h
> @@ -0,0 +1,123 @@
> +/*
> + * Allwinner Watchdog emulation
> + *
> + * Copyright (C) 2023 Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> + *
> + *  This file is derived from Allwinner RTC,
> + *  by Niek Linnenbank.
> + *
> + * 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/>.
> + */
>

Although technically it doesn't matter, in my opinion the most logical
place for this
comment would be a bit lower in the file, right after the
ifdef/define/include.

+/*
> + * This is a model of the Allwinner watchdog.
> + * Since watchdog registers belong to the timer module (and are shared
> with the
> + * RTC module), the interrupt line from watchdog is not handled right now.
> + * In QEMU, we just wire up the watchdog reset to
> watchdog_perform_action(),
> + * at least for the moment.
> + */
> +
> +#ifndef HW_WATCHDOG_ALLWINNER_WDT_H
> +#define HW_WATCHDOG_ALLWINNER_WDT_H
> +
> +#include "qom/object.h"
> +#include "hw/ptimer.h"
> +#include "hw/sysbus.h"
> +
> +
> +#define TYPE_AW_WDT    "allwinner-wdt"
> +
> +/** Allwinner WDT sun4i family (A10, A12), also sun7i (A20) */
> +#define TYPE_AW_WDT_SUN4I    TYPE_AW_WDT "-sun4i"
> +
> +/** Allwinner WDT sun6i family and newer (A31, H2+, H3, etc) */
> +#define TYPE_AW_WDT_SUN6I    TYPE_AW_WDT "-sun6i"
> +
> +/** Number of WDT registers */
> +#define AW_WDT_REGS_NUM      (5)
> +
> +OBJECT_DECLARE_TYPE(AwWdtState, AwWdtClass, AW_WDT)
> +
> +/**
> + * Allwinner A10 WDT object instance state.
>

In principle, the "A10" here can be removed, since we've modelled it as a
generic allwinner driver.


> + */
> +struct AwWdtState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +
> +    /*< public >*/
> +    MemoryRegion iomem;
> +    struct ptimer_state *timer;
> +
> +    uint32_t regs[AW_WDT_REGS_NUM];
> +};
> +
> +/**
> + * Allwinner WDT 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.
> + */
> +struct AwWdtClass {
> +    /*< private >*/
> +    SysBusDeviceClass parent_class;
> +    /*< public >*/
> +
> +    /** Defines device specific register map */
> +    const uint8_t *regmap;
> +
> +    /** Size of the regmap in bytes */
> +    size_t regmap_size;
> +
> +    /**
> +     * Read device specific register
> +     *
> +     * @offset: register offset to read
> +     * @return true if register read successful, false otherwise
> +     */
> +    bool (*read)(AwWdtState *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)(AwWdtState *s, uint32_t offset, uint32_t data);
> +
> +    /**
> +     * Check if watchdog can generate system reset
> +     *
> +     * @return true if watchdog can generate system reset
> +     */
> +    bool (*can_reset_system)(AwWdtState *s);
> +
> +    /**
> +     * Check if provided key is valid
> +     *
> +     * @value: value written to register
> +     * @return true if key is valid, false otherwise
> +     */
> +    bool (*is_key_valid)(AwWdtState *s, uint32_t val);
> +
> +    /**
> +     * Get current INTV_VALUE setting
> +     *
> +     * @return current INTV_VALUE (0-15)
> +     */
> +    uint8_t (*get_intv_value)(AwWdtState *s);
> +};
> +
> +#endif /* HW_WATCHDOG_ALLWINNER_WDT_H */
> --
> 2.30.2
>
>
Regards,
Niek

-- 
Niek Linnenbank

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard
  2023-03-11 14:41 ` [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard Strahinja Jankovic
@ 2023-03-14 21:21   ` Niek Linnenbank
  2023-03-15 22:01     ` Strahinja Jankovic
  0 siblings, 1 reply; 13+ messages in thread
From: Niek Linnenbank @ 2023-03-14 21:21 UTC (permalink / raw)
  To: Strahinja Jankovic
  Cc: Peter Maydell, Beniamino Galvani, qemu-arm, qemu-devel,
	Strahinja Jankovic

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

Hi Strahinja,


On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <
strahinjapjankovic@gmail.com> wrote:

> This patch adds WDT to Allwinner-A10 and Cubieboard.
> WDT is added as an overlay to the Timer module memory map.
>
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> ---
>  docs/system/arm/cubieboard.rst | 1 +
>  hw/arm/Kconfig                 | 1 +
>  hw/arm/allwinner-a10.c         | 7 +++++++
>  include/hw/arm/allwinner-a10.h | 2 ++
>  4 files changed, 11 insertions(+)
>
> diff --git a/docs/system/arm/cubieboard.rst
> b/docs/system/arm/cubieboard.rst
> index 8d485f5435..58c4a2d3ea 100644
> --- a/docs/system/arm/cubieboard.rst
> +++ b/docs/system/arm/cubieboard.rst
> @@ -15,3 +15,4 @@ Emulated devices:
>  - USB controller
>  - SATA controller
>  - TWI (I2C) controller
> +- Watchdog timer
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index b5aed4aff5..ec15248536 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -325,6 +325,7 @@ config ALLWINNER_A10
>      select ALLWINNER_A10_PIC
>      select ALLWINNER_A10_CCM
>      select ALLWINNER_A10_DRAMC
> +    select ALLWINNER_WDT
>      select ALLWINNER_EMAC
>      select ALLWINNER_I2C
>      select AXP209_PMU
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index b7ca795c71..b0ea3f7f66 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -38,6 +38,7 @@
>  #define AW_A10_EHCI_BASE        0x01c14000
>  #define AW_A10_OHCI_BASE        0x01c14400
>  #define AW_A10_SATA_BASE        0x01c18000
> +#define AW_A10_WDT_BASE         0x01c20c90
>

Unfortunately I couldn't find any details about the watchdog in the
Allwinner A10 datasheet "A10_Datasheet.pdf", except for a very brief
summary in chapter 9.1 in the Timer Controller. But I did find that linux
is using this same base address and registers with the shared driver code
in drivers/watchdog/sunxi_wdt.c.

Looks good to me.

Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>


>  #define AW_A10_RTC_BASE         0x01c20d00
>  #define AW_A10_I2C0_BASE        0x01c2ac00
>
> @@ -92,6 +93,8 @@ static void aw_a10_init(Object *obj)
>      object_initialize_child(obj, "mmc0", &s->mmc0, TYPE_AW_SDHOST_SUN4I);
>
>      object_initialize_child(obj, "rtc", &s->rtc, TYPE_AW_RTC_SUN4I);
> +
> +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN4I);
>  }
>
>  static void aw_a10_realize(DeviceState *dev, Error **errp)
> @@ -203,6 +206,10 @@ static void aw_a10_realize(DeviceState *dev, Error
> **errp)
>      sysbus_realize(SYS_BUS_DEVICE(&s->i2c0), &error_fatal);
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c0), 0, AW_A10_I2C0_BASE);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c0), 0, qdev_get_gpio_in(dev,
> 7));
> +
> +    /* WDT */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0, AW_A10_WDT_BASE,
> 1);
>  }
>
>  static void aw_a10_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/arm/allwinner-a10.h
> b/include/hw/arm/allwinner-a10.h
> index 095afb225d..cd1465c613 100644
> --- a/include/hw/arm/allwinner-a10.h
> +++ b/include/hw/arm/allwinner-a10.h
> @@ -13,6 +13,7 @@
>  #include "hw/misc/allwinner-a10-ccm.h"
>  #include "hw/misc/allwinner-a10-dramc.h"
>  #include "hw/i2c/allwinner-i2c.h"
> +#include "hw/watchdog/allwinner-wdt.h"
>  #include "sysemu/block-backend.h"
>
>  #include "target/arm/cpu.h"
> @@ -41,6 +42,7 @@ struct AwA10State {
>      AwSdHostState mmc0;
>      AWI2CState i2c0;
>      AwRtcState rtc;
> +    AwWdtState wdt;
>      MemoryRegion sram_a;
>      EHCISysBusState ehci[AW_A10_NUM_USB];
>      OHCISysBusState ohci[AW_A10_NUM_USB];
> --
> 2.30.2
>
>

-- 
Niek Linnenbank

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC
  2023-03-11 14:41 ` [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC Strahinja Jankovic
  2023-03-13  7:53   ` Philippe Mathieu-Daudé
@ 2023-03-14 21:29   ` Niek Linnenbank
  1 sibling, 0 replies; 13+ messages in thread
From: Niek Linnenbank @ 2023-03-14 21:29 UTC (permalink / raw)
  To: Strahinja Jankovic
  Cc: Peter Maydell, Beniamino Galvani, qemu-arm, qemu-devel,
	Strahinja Jankovic

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

On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <
strahinjapjankovic@gmail.com> wrote:

> This patch adds WDT to Allwinner-H3 and Orangepi-PC.
> WDT is added as an overlay to the Timer module memory area.
>
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
>

Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>


> ---
>  docs/system/arm/orangepi.rst  | 1 +
>  hw/arm/Kconfig                | 1 +
>  hw/arm/allwinner-h3.c         | 8 ++++++++
>  include/hw/arm/allwinner-h3.h | 5 ++++-
>  4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/docs/system/arm/orangepi.rst b/docs/system/arm/orangepi.rst
> index e5973600a1..9afa54213b 100644
> --- a/docs/system/arm/orangepi.rst
> +++ b/docs/system/arm/orangepi.rst
> @@ -26,6 +26,7 @@ The Orange Pi PC machine supports the following devices:
>   * System Control module
>   * Security Identifier device
>   * TWI (I2C)
> + * Watchdog timer
>
>  Limitations
>  """""""""""
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index ec15248536..7d916f5450 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -337,6 +337,7 @@ config ALLWINNER_H3
>      select ALLWINNER_A10_PIT
>      select ALLWINNER_SUN8I_EMAC
>      select ALLWINNER_I2C
> +    select ALLWINNER_WDT
>      select SERIAL
>      select ARM_TIMER
>      select ARM_GIC
> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> index 69d0ad6f50..f05afddf7e 100644
> --- a/hw/arm/allwinner-h3.c
> +++ b/hw/arm/allwinner-h3.c
> @@ -49,6 +49,7 @@ const hwaddr allwinner_h3_memmap[] = {
>      [AW_H3_DEV_OHCI3]      = 0x01c1d400,
>      [AW_H3_DEV_CCU]        = 0x01c20000,
>      [AW_H3_DEV_PIT]        = 0x01c20c00,
> +    [AW_H3_DEV_WDT]        = 0x01c20ca0,
>      [AW_H3_DEV_UART0]      = 0x01c28000,
>      [AW_H3_DEV_UART1]      = 0x01c28400,
>      [AW_H3_DEV_UART2]      = 0x01c28800,
> @@ -234,6 +235,8 @@ static void allwinner_h3_init(Object *obj)
>      object_initialize_child(obj, "twi1",  &s->i2c1,  TYPE_AW_I2C_SUN6I);
>      object_initialize_child(obj, "twi2",  &s->i2c2,  TYPE_AW_I2C_SUN6I);
>      object_initialize_child(obj, "r_twi", &s->r_twi, TYPE_AW_I2C_SUN6I);
> +
> +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN6I);
>  }
>
>  static void allwinner_h3_realize(DeviceState *dev, Error **errp)
> @@ -453,6 +456,11 @@ static void allwinner_h3_realize(DeviceState *dev,
> Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->r_twi), 0,
>                         qdev_get_gpio_in(DEVICE(&s->gic),
> AW_H3_GIC_SPI_R_TWI));
>
> +    /* WDT */
> +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0,
> +                            s->memmap[AW_H3_DEV_WDT], 1);
> +
>      /* Unimplemented devices */
>      for (i = 0; i < ARRAY_SIZE(unimplemented); i++) {
>          create_unimplemented_device(unimplemented[i].device_name,
> diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h
> index 59e0f822d2..f15d6d7cc7 100644
> --- a/include/hw/arm/allwinner-h3.h
> +++ b/include/hw/arm/allwinner-h3.h
> @@ -48,6 +48,7 @@
>  #include "hw/net/allwinner-sun8i-emac.h"
>  #include "hw/rtc/allwinner-rtc.h"
>  #include "hw/i2c/allwinner-i2c.h"
> +#include "hw/watchdog/allwinner-wdt.h"
>  #include "target/arm/cpu.h"
>  #include "sysemu/block-backend.h"
>
> @@ -96,7 +97,8 @@ enum {
>      AW_H3_DEV_RTC,
>      AW_H3_DEV_CPUCFG,
>      AW_H3_DEV_R_TWI,
> -    AW_H3_DEV_SDRAM
> +    AW_H3_DEV_SDRAM,
> +    AW_H3_DEV_WDT
>  };
>
>  /** Total number of CPU cores in the H3 SoC */
> @@ -141,6 +143,7 @@ struct AwH3State {
>      AWI2CState r_twi;
>      AwSun8iEmacState emac;
>      AwRtcState rtc;
> +    AwWdtState wdt;
>      GICState gic;
>      MemoryRegion sram_a1;
>      MemoryRegion sram_a2;
> --
> 2.30.2
>
>

-- 
Niek Linnenbank

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset
  2023-03-14 21:14   ` Niek Linnenbank
@ 2023-03-15 21:59     ` Strahinja Jankovic
  0 siblings, 0 replies; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-15 21:59 UTC (permalink / raw)
  To: Niek Linnenbank; +Cc: Peter Maydell, Beniamino Galvani, qemu-arm, qemu-devel

)Hi Niek,

On Tue, Mar 14, 2023 at 10:14 PM Niek Linnenbank
<nieklinnenbank@gmail.com> wrote:
>
> Hi Strahinja,
>
>
> On Sat, Mar 11, 2023 at 3:41 PM Strahinja Jankovic <strahinjapjankovic@gmail.com> wrote:
>>
>> This patch adds basic support for Allwinner WDT.
>> Both sun4i and sun6i variants are supported.
>> However, interrupt generation is not supported, so WDT can be used only to trigger system reset.
>>
>> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
>> ---
>>  hw/watchdog/Kconfig                 |   4 +
>>  hw/watchdog/allwinner-wdt.c         | 428 ++++++++++++++++++++++++++++
>>  hw/watchdog/meson.build             |   1 +
>>  hw/watchdog/trace-events            |   7 +
>>  include/hw/watchdog/allwinner-wdt.h | 123 ++++++++
>>  5 files changed, 563 insertions(+)
>>  create mode 100644 hw/watchdog/allwinner-wdt.c
>>  create mode 100644 include/hw/watchdog/allwinner-wdt.h
>>
>> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
>> index 66e1d029e3..861fd00334 100644
>> --- a/hw/watchdog/Kconfig
>> +++ b/hw/watchdog/Kconfig
>> @@ -20,3 +20,7 @@ config WDT_IMX2
>>
>>  config WDT_SBSA
>>      bool
>> +
>> +config ALLWINNER_WDT
>> +    bool
>> +    select PTIMER
>> diff --git a/hw/watchdog/allwinner-wdt.c b/hw/watchdog/allwinner-wdt.c
>> new file mode 100644
>> index 0000000000..cf16ec7a56
>> --- /dev/null
>> +++ b/hw/watchdog/allwinner-wdt.c
>> @@ -0,0 +1,428 @@
>> +/*
>> + * Allwinner Watchdog emulation
>> + *
>> + * Copyright (C) 2023 Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
>> + *
>> + *  This file is derived from Allwinner RTC,
>> + *  by Niek Linnenbank.
>> + *
>> + * 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/log.h"
>> +#include "qemu/units.h"
>> +#include "qemu/module.h"
>> +#include "trace.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/registerfields.h"
>> +#include "hw/watchdog/allwinner-wdt.h"
>> +#include "sysemu/watchdog.h"
>> +#include "migration/vmstate.h"
>> +
>> +/* WDT registers */
>> +enum {
>> +    REG_IRQ_EN = 0,     /* Watchdog interrupt enable */
>
>
> Since we are doing a check "if (!c->regmap[offset])" below, should the enum values begin with 1 instead?

Yes, you are correct. I will fix this.

>
>>
>> +    REG_IRQ_STA,        /* Watchdog interrupt status */
>> +    REG_CTRL,           /* Watchdog control register */
>> +    REG_CFG,            /* Watchdog configuration register */
>> +    REG_MODE,           /* Watchdog mode register */
>> +};
>> +
>> +/* Universal WDT register flags */
>> +#define WDT_RESTART_MASK    (1 << 0)
>> +#define WDT_EN_MASK         (1 << 0)
>> +
>> +/* sun4i specific WDT register flags */
>> +#define RST_EN_SUN4I_MASK       (1 << 1)
>> +#define INTV_VALUE_SUN4I_SHIFT  (3)
>> +#define INTV_VALUE_SUN4I_MASK   (0xfu << INTV_VALUE_SUN4I_SHIFT)
>> +
>> +/* sun6i specific WDT register flags */
>> +#define RST_EN_SUN6I_MASK       (1 << 0)
>> +#define KEY_FIELD_SUN6I_SHIFT   (1)
>> +#define KEY_FIELD_SUN6I_MASK    (0xfffu << KEY_FIELD_SUN6I_SHIFT)
>> +#define KEY_FIELD_SUN6I         (0xA57u)
>> +#define INTV_VALUE_SUN6I_SHIFT  (4)
>> +#define INTV_VALUE_SUN6I_MASK   (0xfu << INTV_VALUE_SUN6I_SHIFT)
>> +
>> +/* Map of INTV_VALUE to 0.5s units. */
>> +static const uint8_t allwinner_wdt_count_map[] = {
>> +    1,
>> +    2,
>> +    4,
>> +    6,
>> +    8,
>> +    10,
>> +    12,
>> +    16,
>> +    20,
>> +    24,
>> +    28,
>> +    32
>> +};
>> +
>> +/* WDT sun4i register map (offset to name) */
>> +const uint8_t allwinner_wdt_sun4i_regmap[] = {
>> +    [0x0000] = REG_CTRL,
>> +    [0x0004] = REG_MODE,
>> +};
>> +
>> +/* WDT sun6i register map (offset to name) */
>> +const uint8_t allwinner_wdt_sun6i_regmap[] = {
>> +    [0x0000] = REG_IRQ_EN,
>> +    [0x0004] = REG_IRQ_STA,
>> +    [0x0010] = REG_CTRL,
>> +    [0x0014] = REG_CFG,
>> +    [0x0018] = REG_MODE,
>> +};
>> +
>> +static bool allwinner_wdt_sun4i_read(AwWdtState *s, uint32_t offset)
>> +{
>> +    /* no sun4i specific registers currently implemented */
>> +    return false;
>> +}
>> +
>> +static bool allwinner_wdt_sun4i_write(AwWdtState *s, uint32_t offset,
>> +                                      uint32_t data)
>> +{
>> +    /* no sun4i specific registers currently implemented */
>> +    return false;
>> +}
>> +
>> +static bool allwinner_wdt_sun4i_can_reset_system(AwWdtState *s)
>> +{
>> +    if (s->regs[REG_MODE] & RST_EN_SUN6I_MASK) {
>> +        return true;
>> +    } else {
>> +        return false;
>> +    }
>> +}
>> +
>> +static bool allwinner_wdt_sun4i_is_key_valid(AwWdtState *s, uint32_t val)
>> +{
>> +    /* sun4i has no key */
>> +    return true;
>> +}
>> +
>> +static uint8_t allwinner_wdt_sun4i_get_intv_value(AwWdtState *s)
>> +{
>> +    return ((s->regs[REG_MODE] & INTV_VALUE_SUN4I_MASK) >>
>> +            INTV_VALUE_SUN4I_SHIFT);
>> +}
>> +
>> +static bool allwinner_wdt_sun6i_read(AwWdtState *s, uint32_t offset)
>> +{
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +
>> +    switch (c->regmap[offset]) {
>> +    case REG_IRQ_EN:
>> +    case REG_IRQ_STA:
>> +    case REG_CFG:
>> +        return true;
>> +    default:
>> +        break;
>> +    }
>> +    return false;
>> +}
>> +
>> +static bool allwinner_wdt_sun6i_write(AwWdtState *s, uint32_t offset,
>> +                                      uint32_t data)
>> +{
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +
>> +    switch (c->regmap[offset]) {
>> +    case REG_IRQ_EN:
>> +    case REG_IRQ_STA:
>> +    case REG_CFG:
>> +        return true;
>> +    default:
>> +        break;
>> +    }
>> +    return false;
>> +}
>> +
>> +static bool allwinner_wdt_sun6i_can_reset_system(AwWdtState *s)
>> +{
>> +    if (s->regs[REG_CFG] & RST_EN_SUN6I_MASK) {
>> +        return true;
>> +    } else {
>> +        return false;
>> +    }
>> +}
>> +
>> +static bool allwinner_wdt_sun6i_is_key_valid(AwWdtState *s, uint32_t val)
>> +{
>> +    uint16_t key = (val & KEY_FIELD_SUN6I_MASK) >> KEY_FIELD_SUN6I_SHIFT;
>> +    return (key == KEY_FIELD_SUN6I);
>> +}
>> +
>> +static uint8_t allwinner_wdt_sun6i_get_intv_value(AwWdtState *s)
>> +{
>> +    return ((s->regs[REG_MODE] & INTV_VALUE_SUN6I_MASK) >>
>> +            INTV_VALUE_SUN6I_SHIFT);
>> +}
>> +
>> +static void allwinner_wdt_update_timer(AwWdtState *s)
>> +{
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +    uint8_t count = c->get_intv_value(s);
>> +
>> +    ptimer_transaction_begin(s->timer);
>> +    ptimer_stop(s->timer);
>> +
>> +    /* Use map to convert. */
>> +    if (count < sizeof(allwinner_wdt_count_map)) {
>> +        ptimer_set_count(s->timer, allwinner_wdt_count_map[count]);
>> +    } else {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect INTV_VALUE 0x%02x\n",
>> +                __func__, count);
>> +    }
>> +
>> +    ptimer_run(s->timer, 1);
>> +    ptimer_transaction_commit(s->timer);
>> +
>> +    trace_allwinner_wdt_update_timer(count);
>> +}
>> +
>> +static uint64_t allwinner_wdt_read(void *opaque, hwaddr offset,
>> +                                       unsigned size)
>> +{
>> +    AwWdtState *s = AW_WDT(opaque);
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +    uint64_t r;
>> +
>> +    if (offset >= c->regmap_size) {
>> +        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_CTRL:
>> +    case REG_MODE:
>> +        r = 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);
>> +            return 0;
>> +        }
>> +        r = s->regs[c->regmap[offset]];
>> +        break;
>> +    }
>> +
>> +    trace_allwinner_wdt_read(offset, r, size);
>> +
>> +    return r;
>> +}
>> +
>> +static void allwinner_wdt_write(void *opaque, hwaddr offset,
>> +                                   uint64_t val, unsigned size)
>> +{
>> +    AwWdtState *s = AW_WDT(opaque);
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +    uint32_t old_val;
>> +
>> +    if (offset >= c->regmap_size) {
>> +        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_wdt_write(offset, val, size);
>> +
>> +    switch (c->regmap[offset]) {
>> +    case REG_CTRL:
>> +        if (c->is_key_valid(s, val)) {
>> +            if (val & WDT_RESTART_MASK) {
>> +                /* Kick timer */
>> +                allwinner_wdt_update_timer(s);
>> +            }
>> +        }
>> +        break;
>> +    case REG_MODE:
>> +        old_val = s->regs[REG_MODE];
>> +        s->regs[REG_MODE] = (uint32_t)val;
>> +
>> +        /* Check for rising edge on WDOG_MODE_EN */
>> +        if ((s->regs[REG_MODE] & ~old_val) & WDT_EN_MASK) {
>> +            allwinner_wdt_update_timer(s);
>> +        }
>> +        break;
>> +    default:
>> +        if (!c->write(s, offset, val)) {
>> +            qemu_log_mask(LOG_UNIMP, "%s: unimplemented register 0x%04x\n",
>> +                          __func__, (uint32_t)offset);
>> +        }
>> +        s->regs[c->regmap[offset]] = (uint32_t)val;
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps allwinner_wdt_ops = {
>> +    .read = allwinner_wdt_read,
>> +    .write = allwinner_wdt_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4,
>> +    },
>> +    .impl.min_access_size = 4,
>> +};
>> +
>> +static void allwinner_wdt_expired(void *opaque)
>> +{
>> +    AwWdtState *s = AW_WDT(opaque);
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +
>> +    bool enabled = s->regs[REG_MODE] & WDT_EN_MASK;
>> +    bool reset_enabled = c->can_reset_system(s);
>> +
>> +    trace_allwinner_wdt_expired(enabled, reset_enabled);
>> +
>> +    /* Perform watchdog action if watchdog is enabled and can trigger reset */
>> +    if (enabled && reset_enabled) {
>> +        watchdog_perform_action();
>> +    }
>> +}
>> +
>> +static void allwinner_wdt_reset_enter(Object *obj, ResetType type)
>> +{
>> +    AwWdtState *s = AW_WDT(obj);
>> +
>> +    trace_allwinner_wdt_reset_enter();
>> +
>> +    /* Clear registers */
>> +    memset(s->regs, 0, sizeof(s->regs));
>> +}
>> +
>> +static const VMStateDescription allwinner_wdt_vmstate = {
>> +    .name = "allwinner-wdt",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_PTIMER(timer, AwWdtState),
>> +        VMSTATE_UINT32_ARRAY(regs, AwWdtState, AW_WDT_REGS_NUM),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void allwinner_wdt_init(Object *obj)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> +    AwWdtState *s = AW_WDT(obj);
>> +    const AwWdtClass *c = AW_WDT_GET_CLASS(s);
>> +
>> +    /* Memory mapping */
>> +    memory_region_init_io(&s->iomem, OBJECT(s), &allwinner_wdt_ops, s,
>> +                          TYPE_AW_WDT, c->regmap_size * 4);
>> +    sysbus_init_mmio(sbd, &s->iomem);
>> +}
>> +
>> +static void allwinner_wdt_realize(DeviceState *dev, Error **errp)
>> +{
>> +    AwWdtState *s = AW_WDT(dev);
>> +
>> +    s->timer = ptimer_init(allwinner_wdt_expired, s,
>> +                           PTIMER_POLICY_NO_IMMEDIATE_TRIGGER |
>> +                           PTIMER_POLICY_NO_IMMEDIATE_RELOAD |
>> +                           PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
>> +
>> +    ptimer_transaction_begin(s->timer);
>> +    /* Set to 2Hz (0.5s period) */
>> +    ptimer_set_freq(s->timer, 2);
>
>
> Just wondering here where the 0.5s / 2Hz as the value come from?
> I do see that wdt_imx2.c is also using the same value, but no code comment is provided to clarify.

The User manual for A10
(https://linux-sunxi.org/images/1/1e/Allwinner_A10_User_manual_V1.5.pdf)
and H3 say that interval values can be configured in the range from
0.5 to 16s (table is on page 100).
Since 0.5s is the shortest period (highest frequency), then 2Hz is set
as the timer frequency, and the array in allwinner_wdt_count_map is
used to map count values to register contents.

>
>>
>> +    ptimer_set_limit(s->timer, 0xff, 1);
>> +    ptimer_transaction_commit(s->timer);
>> +}
>> +
>> +static void allwinner_wdt_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>> +
>> +    rc->phases.enter = allwinner_wdt_reset_enter;
>> +    dc->realize = allwinner_wdt_realize;
>> +    dc->vmsd = &allwinner_wdt_vmstate;
>> +}
>> +
>> +static void allwinner_wdt_sun4i_class_init(ObjectClass *klass, void *data)
>> +{
>> +    AwWdtClass *awc = AW_WDT_CLASS(klass);
>> +
>> +    awc->regmap = allwinner_wdt_sun4i_regmap;
>> +    awc->regmap_size = sizeof(allwinner_wdt_sun4i_regmap);
>> +    awc->read = allwinner_wdt_sun4i_read;
>> +    awc->write = allwinner_wdt_sun4i_write;
>> +    awc->can_reset_system = allwinner_wdt_sun4i_can_reset_system;
>> +    awc->is_key_valid = allwinner_wdt_sun4i_is_key_valid;
>> +    awc->get_intv_value = allwinner_wdt_sun4i_get_intv_value;
>> +}
>> +
>> +static void allwinner_wdt_sun6i_class_init(ObjectClass *klass, void *data)
>> +{
>> +    AwWdtClass *awc = AW_WDT_CLASS(klass);
>> +
>> +    awc->regmap = allwinner_wdt_sun6i_regmap;
>> +    awc->regmap_size = sizeof(allwinner_wdt_sun6i_regmap);
>> +    awc->read = allwinner_wdt_sun6i_read;
>> +    awc->write = allwinner_wdt_sun6i_write;
>> +    awc->can_reset_system = allwinner_wdt_sun6i_can_reset_system;
>> +    awc->is_key_valid = allwinner_wdt_sun6i_is_key_valid;
>> +    awc->get_intv_value = allwinner_wdt_sun6i_get_intv_value;
>> +}
>> +
>> +static const TypeInfo allwinner_wdt_info = {
>> +    .name          = TYPE_AW_WDT,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_init = allwinner_wdt_init,
>> +    .instance_size = sizeof(AwWdtState),
>> +    .class_init    = allwinner_wdt_class_init,
>> +    .class_size    = sizeof(AwWdtClass),
>> +    .abstract      = true,
>> +};
>> +
>> +static const TypeInfo allwinner_wdt_sun4i_info = {
>> +    .name          = TYPE_AW_WDT_SUN4I,
>> +    .parent        = TYPE_AW_WDT,
>> +    .class_init    = allwinner_wdt_sun4i_class_init,
>> +};
>> +
>> +static const TypeInfo allwinner_wdt_sun6i_info = {
>> +    .name          = TYPE_AW_WDT_SUN6I,
>> +    .parent        = TYPE_AW_WDT,
>> +    .class_init    = allwinner_wdt_sun6i_class_init,
>> +};
>> +
>> +static void allwinner_wdt_register(void)
>> +{
>> +    type_register_static(&allwinner_wdt_info);
>> +    type_register_static(&allwinner_wdt_sun4i_info);
>> +    type_register_static(&allwinner_wdt_sun6i_info);
>> +}
>> +
>> +type_init(allwinner_wdt_register)
>> diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
>> index 8974b5cf4c..5dcd4fbe2f 100644
>> --- a/hw/watchdog/meson.build
>> +++ b/hw/watchdog/meson.build
>> @@ -1,4 +1,5 @@
>>  softmmu_ss.add(files('watchdog.c'))
>> +softmmu_ss.add(when: 'CONFIG_ALLWINNER_WDT', if_true: files('allwinner-wdt.c'))
>>  softmmu_ss.add(when: 'CONFIG_CMSDK_APB_WATCHDOG', if_true: files('cmsdk-apb-watchdog.c'))
>>  softmmu_ss.add(when: 'CONFIG_WDT_IB6300ESB', if_true: files('wdt_i6300esb.c'))
>>  softmmu_ss.add(when: 'CONFIG_WDT_IB700', if_true: files('wdt_ib700.c'))
>> diff --git a/hw/watchdog/trace-events b/hw/watchdog/trace-events
>> index 54371ae075..b1227860c4 100644
>> --- a/hw/watchdog/trace-events
>> +++ b/hw/watchdog/trace-events
>> @@ -1,5 +1,12 @@
>>  # See docs/devel/tracing.rst for syntax documentation.
>>
>> +# allwinner-wdt.c
>> +allwinner_wdt_read(uint64_t offset, uint64_t data, unsigned size) "Allwinner watchdog read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>> +allwinner_wdt_write(uint64_t offset, uint64_t data, unsigned size) "Allwinner watchdog write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>> +allwinner_wdt_reset_enter(void) "Allwinner watchdog: reset"
>> +allwinner_wdt_update_timer(uint32_t count) "Allwinner watchdog: count %" PRIu32
>
>
> should the type for count be uint8_t and PRIu8 instead? In the function allwinner_wdt_update_timer() above,
> the variable 'uint8_t count' is passed, which is filled by the call to c->get_intv_value().

Yes, I will correct this.

>
>>
>> +allwinner_wdt_expired(bool enabled, bool reset_enabled) "Allwinner watchdog: enabled %u reset_enabled %u"
>> +
>>  # cmsdk-apb-watchdog.c
>>  cmsdk_apb_watchdog_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB watchdog read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>>  cmsdk_apb_watchdog_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB watchdog write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>> diff --git a/include/hw/watchdog/allwinner-wdt.h b/include/hw/watchdog/allwinner-wdt.h
>> new file mode 100644
>> index 0000000000..37b49e77ee
>> --- /dev/null
>> +++ b/include/hw/watchdog/allwinner-wdt.h
>> @@ -0,0 +1,123 @@
>> +/*
>> + * Allwinner Watchdog emulation
>> + *
>> + * Copyright (C) 2023 Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
>> + *
>> + *  This file is derived from Allwinner RTC,
>> + *  by Niek Linnenbank.
>> + *
>> + * 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/>.
>> + */
>
>
> Although technically it doesn't matter, in my opinion the most logical place for this
> comment would be a bit lower in the file, right after the ifdef/define/include.

I saw how it was it was done in the CMSDK APB header file and did it
in the same way, but I can move the comment below as you suggested.

>
>> +/*
>> + * This is a model of the Allwinner watchdog.
>> + * Since watchdog registers belong to the timer module (and are shared with the
>> + * RTC module), the interrupt line from watchdog is not handled right now.
>> + * In QEMU, we just wire up the watchdog reset to watchdog_perform_action(),
>> + * at least for the moment.
>> + */
>> +
>> +#ifndef HW_WATCHDOG_ALLWINNER_WDT_H
>> +#define HW_WATCHDOG_ALLWINNER_WDT_H
>> +
>> +#include "qom/object.h"
>> +#include "hw/ptimer.h"
>> +#include "hw/sysbus.h"
>> +
>> +
>> +#define TYPE_AW_WDT    "allwinner-wdt"
>> +
>> +/** Allwinner WDT sun4i family (A10, A12), also sun7i (A20) */
>> +#define TYPE_AW_WDT_SUN4I    TYPE_AW_WDT "-sun4i"
>> +
>> +/** Allwinner WDT sun6i family and newer (A31, H2+, H3, etc) */
>> +#define TYPE_AW_WDT_SUN6I    TYPE_AW_WDT "-sun6i"
>> +
>> +/** Number of WDT registers */
>> +#define AW_WDT_REGS_NUM      (5)
>> +
>> +OBJECT_DECLARE_TYPE(AwWdtState, AwWdtClass, AW_WDT)
>> +
>> +/**
>> + * Allwinner A10 WDT object instance state.
>
>
> In principle, the "A10" here can be removed, since we've modelled it as a generic allwinner driver.

Thanks, this was left over from initial implementation, since I
started with A10... I will remove it.

Best regards,
Strahinja


>
>>
>> + */
>> +struct AwWdtState {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +
>> +    /*< public >*/
>> +    MemoryRegion iomem;
>> +    struct ptimer_state *timer;
>> +
>> +    uint32_t regs[AW_WDT_REGS_NUM];
>> +};
>> +
>> +/**
>> + * Allwinner WDT 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.
>> + */
>> +struct AwWdtClass {
>> +    /*< private >*/
>> +    SysBusDeviceClass parent_class;
>> +    /*< public >*/
>> +
>> +    /** Defines device specific register map */
>> +    const uint8_t *regmap;
>> +
>> +    /** Size of the regmap in bytes */
>> +    size_t regmap_size;
>> +
>> +    /**
>> +     * Read device specific register
>> +     *
>> +     * @offset: register offset to read
>> +     * @return true if register read successful, false otherwise
>> +     */
>> +    bool (*read)(AwWdtState *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)(AwWdtState *s, uint32_t offset, uint32_t data);
>> +
>> +    /**
>> +     * Check if watchdog can generate system reset
>> +     *
>> +     * @return true if watchdog can generate system reset
>> +     */
>> +    bool (*can_reset_system)(AwWdtState *s);
>> +
>> +    /**
>> +     * Check if provided key is valid
>> +     *
>> +     * @value: value written to register
>> +     * @return true if key is valid, false otherwise
>> +     */
>> +    bool (*is_key_valid)(AwWdtState *s, uint32_t val);
>> +
>> +    /**
>> +     * Get current INTV_VALUE setting
>> +     *
>> +     * @return current INTV_VALUE (0-15)
>> +     */
>> +    uint8_t (*get_intv_value)(AwWdtState *s);
>> +};
>> +
>> +#endif /* HW_WATCHDOG_ALLWINNER_WDT_H */
>> --
>> 2.30.2
>>
>
> Regards,
> Niek
>
> --
> Niek Linnenbank
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard
  2023-03-14 21:21   ` Niek Linnenbank
@ 2023-03-15 22:01     ` Strahinja Jankovic
  0 siblings, 0 replies; 13+ messages in thread
From: Strahinja Jankovic @ 2023-03-15 22:01 UTC (permalink / raw)
  To: Niek Linnenbank; +Cc: Peter Maydell, Beniamino Galvani, qemu-arm, qemu-devel

Hi Niek,

On Tue, Mar 14, 2023 at 10:21 PM Niek Linnenbank
<nieklinnenbank@gmail.com> wrote:
>
> Hi Strahinja,
>
>
> On Sat, Mar 11, 2023 at 3:42 PM Strahinja Jankovic <strahinjapjankovic@gmail.com> wrote:
>>
>> This patch adds WDT to Allwinner-A10 and Cubieboard.
>> WDT is added as an overlay to the Timer module memory map.
>>
>> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
>> ---
>>  docs/system/arm/cubieboard.rst | 1 +
>>  hw/arm/Kconfig                 | 1 +
>>  hw/arm/allwinner-a10.c         | 7 +++++++
>>  include/hw/arm/allwinner-a10.h | 2 ++
>>  4 files changed, 11 insertions(+)
>>
>> diff --git a/docs/system/arm/cubieboard.rst b/docs/system/arm/cubieboard.rst
>> index 8d485f5435..58c4a2d3ea 100644
>> --- a/docs/system/arm/cubieboard.rst
>> +++ b/docs/system/arm/cubieboard.rst
>> @@ -15,3 +15,4 @@ Emulated devices:
>>  - USB controller
>>  - SATA controller
>>  - TWI (I2C) controller
>> +- Watchdog timer
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index b5aed4aff5..ec15248536 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -325,6 +325,7 @@ config ALLWINNER_A10
>>      select ALLWINNER_A10_PIC
>>      select ALLWINNER_A10_CCM
>>      select ALLWINNER_A10_DRAMC
>> +    select ALLWINNER_WDT
>>      select ALLWINNER_EMAC
>>      select ALLWINNER_I2C
>>      select AXP209_PMU
>> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>> index b7ca795c71..b0ea3f7f66 100644
>> --- a/hw/arm/allwinner-a10.c
>> +++ b/hw/arm/allwinner-a10.c
>> @@ -38,6 +38,7 @@
>>  #define AW_A10_EHCI_BASE        0x01c14000
>>  #define AW_A10_OHCI_BASE        0x01c14400
>>  #define AW_A10_SATA_BASE        0x01c18000
>> +#define AW_A10_WDT_BASE         0x01c20c90
>
>
> Unfortunately I couldn't find any details about the watchdog in the Allwinner A10 datasheet "A10_Datasheet.pdf", except for a very brief
> summary in chapter 9.1 in the Timer Controller. But I did find that linux is using this same base address and registers with the shared driver code in drivers/watchdog/sunxi_wdt.c.

Thanks for the review. The User Manual for Allwinner A10 is available
at https://linux-sunxi.org/images/1/1e/Allwinner_A10_User_manual_V1.5.pdf,
watchdog registers are on pages 99-100.

Best regards,
Strahinja


>
> Looks good to me.
>
> Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>
>>
>>  #define AW_A10_RTC_BASE         0x01c20d00
>>  #define AW_A10_I2C0_BASE        0x01c2ac00
>>
>> @@ -92,6 +93,8 @@ static void aw_a10_init(Object *obj)
>>      object_initialize_child(obj, "mmc0", &s->mmc0, TYPE_AW_SDHOST_SUN4I);
>>
>>      object_initialize_child(obj, "rtc", &s->rtc, TYPE_AW_RTC_SUN4I);
>> +
>> +    object_initialize_child(obj, "wdt", &s->wdt, TYPE_AW_WDT_SUN4I);
>>  }
>>
>>  static void aw_a10_realize(DeviceState *dev, Error **errp)
>> @@ -203,6 +206,10 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
>>      sysbus_realize(SYS_BUS_DEVICE(&s->i2c0), &error_fatal);
>>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c0), 0, AW_A10_I2C0_BASE);
>>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c0), 0, qdev_get_gpio_in(dev, 7));
>> +
>> +    /* WDT */
>> +    sysbus_realize(SYS_BUS_DEVICE(&s->wdt), &error_fatal);
>> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->wdt), 0, AW_A10_WDT_BASE, 1);
>>  }
>>
>>  static void aw_a10_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
>> index 095afb225d..cd1465c613 100644
>> --- a/include/hw/arm/allwinner-a10.h
>> +++ b/include/hw/arm/allwinner-a10.h
>> @@ -13,6 +13,7 @@
>>  #include "hw/misc/allwinner-a10-ccm.h"
>>  #include "hw/misc/allwinner-a10-dramc.h"
>>  #include "hw/i2c/allwinner-i2c.h"
>> +#include "hw/watchdog/allwinner-wdt.h"
>>  #include "sysemu/block-backend.h"
>>
>>  #include "target/arm/cpu.h"
>> @@ -41,6 +42,7 @@ struct AwA10State {
>>      AwSdHostState mmc0;
>>      AWI2CState i2c0;
>>      AwRtcState rtc;
>> +    AwWdtState wdt;
>>      MemoryRegion sram_a;
>>      EHCISysBusState ehci[AW_A10_NUM_USB];
>>      OHCISysBusState ohci[AW_A10_NUM_USB];
>> --
>> 2.30.2
>>
>
>
> --
> Niek Linnenbank
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-03-15 22:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-11 14:41 [PATCH 0/4] Basic Allwinner WDT emulation Strahinja Jankovic
2023-03-11 14:41 ` [PATCH 1/4] hw/watchdog: Allwinner WDT emulation for system reset Strahinja Jankovic
2023-03-14 21:14   ` Niek Linnenbank
2023-03-15 21:59     ` Strahinja Jankovic
2023-03-11 14:41 ` [PATCH 2/4] hw/arm: Add WDT to Allwinner-A10 and Cubieboard Strahinja Jankovic
2023-03-14 21:21   ` Niek Linnenbank
2023-03-15 22:01     ` Strahinja Jankovic
2023-03-11 14:41 ` [PATCH 3/4] hw/arm: Add WDT to Allwinner-H3 and Orangepi-PC Strahinja Jankovic
2023-03-13  7:53   ` Philippe Mathieu-Daudé
2023-03-13 15:15     ` Strahinja Jankovic
2023-03-14 21:29   ` Niek Linnenbank
2023-03-11 14:41 ` [PATCH 4/4] tests/avocado: Add reboot tests to Cubieboard Strahinja Jankovic
2023-03-14 20:02   ` Niek Linnenbank

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.