[1/2] wdt: Add Aspeed watchdog device model

Message ID 1484758531-15093-2-git-send-email-clg@kaod.org
State New
Headers show
Series
  • [1/2] wdt: Add Aspeed watchdog device model
Related show

Commit Message

Cédric Le Goater Jan. 18, 2017, 4:55 p.m.
From: Joel Stanley <joel@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 <joel@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 <clg@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

-- 
2.7.4

Comments

Peter Maydell Jan. 24, 2017, 11:50 a.m. | #1
On 18 January 2017 at 16:55, Cédric Le Goater <clg@kaod.org> wrote:
> From: Joel Stanley <joel@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 <joel@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 <clg@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
Cédric Le Goater Jan. 25, 2017, 4:29 p.m. | #2
On 01/24/2017 12:50 PM, Peter Maydell wrote:
> On 18 January 2017 at 16:55, Cédric Le Goater <clg@kaod.org> wrote:

>> From: Joel Stanley <joel@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 <joel@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 <clg@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.


Well, pclk is for the WDT_RESTART reg also. But, yes, it 
can be improved.

>> +

>> +    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.


yes clearly. I will use a regs array in next version, 
it should be cleaner and also migration friendly. 
I completely left out that part. 

Thanks,

C.  


>> +    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

>

Patch hide | download patch | download mbox

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));
+
+    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;
+    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),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const MemoryRegionOps aspeed_wdt_ops = {
+    .read = aspeed_wdt_read,
+    .write = aspeed_wdt_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+    .valid.unaligned = false,
+};
+
+static void aspeed_wdt_reset(DeviceState *dev)
+{
+    AspeedWDTState *s = ASPEED_WDT(dev);
+
+    s->reg_status = 0x3EF1480;
+    s->reg_reload_value = 0x03EF1480;
+    s->reg_restart = 0;
+    s->reg_ctrl = 0;
+
+    s->enabled = false;
+    timer_del(s->timer);
+}
+
+static void aspeed_wdt_timer_expired(void *dev)
+{
+    AspeedWDTState *s = ASPEED_WDT(dev);
+
+    qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
+    watchdog_perform_action();
+    timer_del(s->timer);
+}
+
+static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    AspeedWDTState *s = ASPEED_WDT(dev);
+
+    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired,
+                            dev);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s,
+                          TYPE_ASPEED_WDT, WDT_IO_REGION_SIZE);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = aspeed_wdt_realize;
+    dc->reset = aspeed_wdt_reset;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    dc->vmsd = &vmstate_aspeed_wdt;
+}
+
+static const TypeInfo aspeed_wdt_info = {
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .name  = TYPE_ASPEED_WDT,
+    .instance_size  = sizeof(AspeedWDTState),
+    .class_init = aspeed_wdt_class_init,
+};
+
+static void wdt_aspeed_register_types(void)
+{
+    watchdog_add_model(&model);
+    type_register_static(&aspeed_wdt_info);
+}
+
+type_init(wdt_aspeed_register_types)
diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
new file mode 100644
index 000000000000..a11b687b6aff
--- /dev/null
+++ b/include/hw/watchdog/wdt_aspeed.h
@@ -0,0 +1,37 @@ 
+/*
+ * 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.
+ */
+#ifndef ASPEED_WDT_H
+#define ASPEED_WDT_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_WDT "aspeed.wdt"
+#define ASPEED_WDT(obj) \
+    OBJECT_CHECK(AspeedWDTState, (obj), TYPE_ASPEED_WDT)
+#define ASPEED_WDT_CLASS(klass) \
+    OBJECT_CLASS_CHECK(AspeedWDTClass, (klass), TYPE_ASPEED_WDT)
+#define ASPEED_WDT_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(AspeedWDTClass, (obj), TYPE_ASPEED_WDT)
+
+typedef struct AspeedWDTState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    QEMUTimer *timer;
+    bool enabled;
+
+    /*< public >*/
+    MemoryRegion iomem;
+
+    uint32_t reg_status;
+    uint32_t reg_reload_value;
+    uint32_t reg_restart;
+    uint32_t reg_ctrl;
+} AspeedWDTState;
+
+#endif  /* ASPEED_WDT_H */