On 18 January 2017 at 16:55, Cédric Le Goater <c...@kaod.org> wrote:
> From: Joel Stanley <j...@jms.id.au>
>
> The Aspeed SoC includes a set of watchdog timers using 32-bit
> decrement counters, which can be based either on the APB clock or
> a 1 MHz clock.
>
> The watchdog timer is designed to prevent system deadlock and, in
> general, it should be restarted before timeout. When a timeout occurs,
> different types of signals can be generated, ARM reset, SOC reset,
> System reset, CPU Interrupt, external signal or boot from alternate
> block. The current model only performs the system reset function as
> this is used by U-Boot and Linux.
>
> Signed-off-by: Joel Stanley <j...@jms.id.au>
> [clg: - fixed compile breakage
> - fixed io region size
> - added watchdog_perform_action() on timer expiry
> - wrote a commit log
> - merged fixes from Andrew Jeffery to scale the reload value ]
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/watchdog/Makefile.objs | 1 +
> hw/watchdog/wdt_aspeed.c | 214
> +++++++++++++++++++++++++++++++++++++++
> include/hw/watchdog/wdt_aspeed.h | 37 +++++++
> 3 files changed, 252 insertions(+)
> create mode 100644 hw/watchdog/wdt_aspeed.c
> create mode 100644 include/hw/watchdog/wdt_aspeed.h
>
> diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
> index 72e3ffd93c59..9589bed63a3d 100644
> --- a/hw/watchdog/Makefile.objs
> +++ b/hw/watchdog/Makefile.objs
> @@ -2,3 +2,4 @@ common-obj-y += watchdog.o
> common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o
> common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o
> common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o
> +common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> new file mode 100644
> index 000000000000..96e62c54dc04
> --- /dev/null
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -0,0 +1,214 @@
> +/*
> + * ASPEED Watchdog Controller
> + *
> + * Copyright (C) 2016-2017 IBM Corp.
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "sysemu/watchdog.h"
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"
> +#include "hw/watchdog/wdt_aspeed.h"
> +
> +#define WDT_IO_REGION_SIZE 0x20
> +
> +#define WDT_STATUS 0x00
> +#define WDT_RELOAD_VALUE 0x04
> +#define WDT_RESTART 0x08
> +#define WDT_CTRL 0x0C
> +#define WDT_TIMEOUT_STATUS 0x10
> +#define WDT_TIMEOUT_CLEAR 0x14
> +#define WDT_RESET_WDITH 0x18
> +
> +#define WDT_RESTART_MAGIC 0x4755
> +
> +static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + AspeedWDTState *s = ASPEED_WDT(opaque);
> +
> + switch (offset) {
> + case WDT_STATUS:
> + return s->reg_status;
> + case WDT_RELOAD_VALUE:
> + return s->reg_reload_value;
> + case WDT_RESTART:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: read from write-only reg at offset 0x%"
> + HWADDR_PRIx "\n", __func__, offset);
> + return 0;
> + case WDT_CTRL:
> + return s->reg_ctrl;
> + case WDT_TIMEOUT_STATUS:
> + case WDT_TIMEOUT_CLEAR:
> + case WDT_RESET_WDITH:
> + qemu_log_mask(LOG_UNIMP,
> + "%s: uninmplemented read at offset 0x%" HWADDR_PRIx
> "\n",
> + __func__, offset);
> + return 0;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx
> "\n",
> + __func__, offset);
> + return 0;
> + }
> +
> +}
> +
> +#define PCLK_HZ 24000000
> +
> +static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
> + unsigned size)
> +{
> + AspeedWDTState *s = ASPEED_WDT(opaque);
> + bool en = data & BIT(0);
> + bool pclk = !(data & BIT(4));
These are only valid for WDT_CTRL, right? That being so,
initialising them up here is a bit odd.
> +
> + switch (offset) {
> + case WDT_STATUS:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: write to read-only reg at offset 0x%"
> + HWADDR_PRIx "\n", __func__, offset);
> + break;
> + case WDT_RELOAD_VALUE:
> + s->reg_reload_value = data;
> + break;
> + case WDT_RESTART:
> + if ((data & 0xFFFF) == 0x4755) {
> + uint32_t reload;
> +
> + s->reg_status = s->reg_reload_value;
> +
> + if (pclk) {
> + reload = muldiv64(s->reg_reload_value,
> NANOSECONDS_PER_SECOND,
> + PCLK_HZ) ;
> + } else {
> + reload = s->reg_reload_value * 1000;
> + }
> +
> + if (s->enabled) {
> + timer_mod(s->timer,
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + reload);
> + }
> + }
> + break;
> + case WDT_CTRL:
> + if (en && !s->enabled) {
> + uint32_t reload;
> +
> + if (pclk) {
> + reload = muldiv64(s->reg_reload_value,
> NANOSECONDS_PER_SECOND,
> + PCLK_HZ);
> + } else {
> + reload = s->reg_reload_value * 1000;
> + }
> +
> + s->enabled = true;
> + timer_mod(s->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> reload);
> + } else if (!en && s->enabled) {
> + s->enabled = false;
> + timer_del(s->timer);
> + }
> + break;
Shouldn't this write to s->reg_ctrl ?
s->enabled seems to duplicate state also in s->reg_ctrl.
> + case WDT_TIMEOUT_STATUS:
> + case WDT_TIMEOUT_CLEAR:
> + case WDT_RESET_WDITH:
> + qemu_log_mask(LOG_UNIMP,
> + "%s: uninmplemented write at offset 0x%" HWADDR_PRIx
> "\n",
> + __func__, offset);
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx
> "\n",
> + __func__, offset);
> + }
> + return;
> +}
> +
> +static WatchdogTimerModel model = {
> + .wdt_name = TYPE_ASPEED_WDT,
> + .wdt_description = "aspeed watchdog device",
> +};
> +
> +static const VMStateDescription vmstate_aspeed_wdt = {
> + .name = "vmstate_aspeed_wdt",
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_TIMER_PTR(timer, AspeedWDTState),
> + VMSTATE_BOOL(enabled, AspeedWDTState),
This doesn't seem to have fields for most of the register state.
> + VMSTATE_END_OF_LIST()
> + }
> +};
thanks
-- PMM
