Re: [Qemu-devel] [PATCH] watchdog: add support to emulate winbond w83627thf

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/10/2015 10:49, Daniel Fahlgren wrote:
> Hi,
> 
> On Mon, 2015-09-14 at 22:07 +0200, Daniel Fahlgren wrote:
>> This patch adds support to emulate the watchdog functionality on the
>> Winbond w83627thf chip. The other features of the chip are not emulated.
>> It has been tested with Ubuntu 6.06, 14.04 and 15.04 as guests using the
>> w83627hf_wdt module.
> 
> Ping, who should I poke about this? The maintainers file does not
> mention the watchdog system.

Hi,

sorry for missing this patch.

I have a couple of questions.  First, where can I find a spec for this
chip in order to review the code, and what are the other features?
Second, what are the advantages over the existing watchdog devices?

Paolo

>> Signed-off-by: Daniel Fahlgren <daniel@fahlgren.se>
>> ---
>>  default-configs/i386-softmmu.mak   |   1 +
>>  default-configs/x86_64-softmmu.mak |   1 +
>>  hw/watchdog/Makefile.objs          |   1 +
>>  hw/watchdog/wdt_w83627thf.c        | 255 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 258 insertions(+)
>>  create mode 100644 hw/watchdog/wdt_w83627thf.c
>>
>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>> index 9393cf0..30abc6f 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -35,6 +35,7 @@ CONFIG_MC146818RTC=y
>>  CONFIG_PAM=y
>>  CONFIG_PCI_PIIX=y
>>  CONFIG_WDT_IB700=y
>> +CONFIG_WDT_W83627THF=y
>>  CONFIG_XEN_I386=$(CONFIG_XEN)
>>  CONFIG_ISA_DEBUG=y
>>  CONFIG_ISA_TESTDEV=y
>> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
>> index 28e2099..906d14b 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -35,6 +35,7 @@ CONFIG_MC146818RTC=y
>>  CONFIG_PAM=y
>>  CONFIG_PCI_PIIX=y
>>  CONFIG_WDT_IB700=y
>> +CONFIG_WDT_W83627THF=y
>>  CONFIG_XEN_I386=$(CONFIG_XEN)
>>  CONFIG_ISA_DEBUG=y
>>  CONFIG_ISA_TESTDEV=y
>> diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
>> index 72e3ffd..e021b24 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_WDT_W83627THF) += wdt_w83627thf.o
>> diff --git a/hw/watchdog/wdt_w83627thf.c b/hw/watchdog/wdt_w83627thf.c
>> new file mode 100644
>> index 0000000..143bb8f
>> --- /dev/null
>> +++ b/hw/watchdog/wdt_w83627thf.c
>> @@ -0,0 +1,255 @@
>> +/*
>> + * Virtual hardware watchdog.
>> + *
>> + * 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/>.
>> + *
>> + * By Daniel Fahlgren (daniel@fahlgren.se)
>> + */
>> +
>> +#include <inttypes.h>
>> +
>> +#include "qemu-common.h"
>> +#include "qemu/timer.h"
>> +#include "sysemu/watchdog.h"
>> +#include "hw/isa/isa.h"
>> +
>> +/* #define W83627THF_DEBUG 1 */
>> +
>> +#ifdef W83627THF_DEBUG
>> +#define w83627thf_debug(fs, ...) \
>> +    fprintf(stderr, "w83627thf: %s: "fs, __func__, ##__VA_ARGS__)
>> +#else
>> +#define w83627thf_debug(fs, ...)
>> +#endif
>> +
>> +#define WATCHDOG_W83627THF_DEVICE(obj) \
>> +    OBJECT_CHECK(W83627THFState, (obj), "w83627thf")
>> +
>> +#define CHIP_VERSION           0x82
>> +
>> +#define CHIP_VERSION_REGISTER  0x20
>> +#define PLED_MODE_REGISTER     0xF5
>> +#define TIMEOUT_REGISTER       0xF6
>> +#define TIMER_REGISTER         0xF7
>> +
>> +#define PLED_MINUTE_MODE       0x08
>> +
>> +#define WDT_W83627THF_EFER     0x2E
>> +#define WDT_W83627THF_EFDR     0x2F
>> +
>> +enum {
>> +    normal_mode = 0,
>> +    extended_mode1 = 1,
>> +    extended_mode2 = 2
>> +};
>> +
>> +/* Device state. */
>> +typedef struct W83627THFState {
>> +    ISADevice parent_obj;
>> +
>> +    QEMUTimer *timer;
>> +
>> +    PortioList port_list;
>> +
>> +    uint8_t running_mode;
>> +
>> +    uint8_t selected_register;
>> +
>> +    uint8_t pled_mode_register;
>> +    uint8_t timeout_register;
>> +    uint8_t timer_register;
>> +
>> +} W83627THFState;
>> +
>> +static WatchdogTimerModel model = {
>> +    .wdt_name = "w83627thf",
>> +    .wdt_description = "Winbond w83627thf",
>> +};
>> +
>> +static const VMStateDescription vmstate_w83627thf = {
>> +    .name = "vmstate_w83627thf",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_TIMER_PTR(timer, W83627THFState),
>> +        VMSTATE_UINT8(running_mode, W83627THFState),
>> +        VMSTATE_UINT8(selected_register, W83627THFState),
>> +        VMSTATE_UINT8(pled_mode_register, W83627THFState),
>> +        VMSTATE_UINT8(timeout_register, W83627THFState),
>> +        VMSTATE_UINT8(timer_register, W83627THFState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +/* This function is called when the watchdog has been changed, either when the
>> + * timer has expired or has been keep-alived.
>> + */
>> +static void wdt_w83627thf_restart_timer(W83627THFState *state)
>> +{
>> +    uint64_t timeout = 1000;
>> +
>> +    if (state->timeout_register == 0) {
>> +        timer_del(state->timer);
>> +        return;
>> +    }
>> +
>> +    if (state->pled_mode_register & PLED_MINUTE_MODE) {
>> +        timeout = 60000;
>> +    }
>> +
>> +    timer_mod(state->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + timeout);
>> +}
>> +
>> +/* This function is called when the timer has expired. Will count down the
>> + * counter and possibly fire the watchdog.
>> + */
>> +static void wdt_w83627thf_timer_tick(void *vp)
>> +{
>> +    W83627THFState *state = vp;
>> +
>> +    state->timeout_register--;
>> +    if (state->timeout_register == 0) {
>> +        state->timer_register |= 0x10;
>> +        timer_del(state->timer);
>> +        watchdog_perform_action();
>> +        return;
>> +    }
>> +
>> +    wdt_w83627thf_restart_timer(state);
>> +}
>> +
>> +/* This function is called when writing to the Extended Function Enable
>> + * Registers.
>> + */
>> +static void wdt_w83627thf_write_efer(void *vp, uint32_t addr, uint32_t data)
>> +{
>> +    W83627THFState *state = vp;
>> +
>> +    w83627thf_debug("data = %x\n", data);
>> +
>> +    if (data == 0x87) {
>> +        if (state->running_mode == normal_mode) {
>> +            state->running_mode = extended_mode1;
>> +        } else {
>> +            state->running_mode = extended_mode2;
>> +        }
>> +    } else if (data == 0xAA) {
>> +        state->running_mode = normal_mode;
>> +    } else if (state->running_mode == extended_mode2) {
>> +        state->selected_register = data;
>> +    }
>> +}
>> +
>> +/* This function is called when reading from the Extended Function Data
>> + * Register.
>> + */
>> +static uint32_t wdt_w83627thf_read_efdr(void *vp, uint32_t addr)
>> +{
>> +    uint8_t data = 0;
>> +    const W83627THFState *state = vp;
>> +
>> +    switch (state->selected_register) {
>> +    case CHIP_VERSION_REGISTER:
>> +        data = CHIP_VERSION;
>> +        break;
>> +    case PLED_MODE_REGISTER:
>> +        data = state->pled_mode_register;
>> +        break;
>> +    case TIMEOUT_REGISTER:
>> +        data = state->timeout_register;
>> +        break;
>> +    case TIMER_REGISTER:
>> +        data = state->timer_register;
>> +        break;
>> +    }
>> +
>> +    w83627thf_debug("reg = %x, data = %x\n", state->selected_register, data);
>> +
>> +    return data;
>> +}
>> +
>> +/* This function is called when writing to the Extended Function Data Register.
>> + */
>> +static void wdt_w83627thf_write_efdr(void *vp, uint32_t addr, uint32_t data)
>> +{
>> +    W83627THFState *state = vp;
>> +
>> +    w83627thf_debug("reg = %x, data = %x\n", state->selected_register, data);
>> +
>> +    switch (state->selected_register) {
>> +    case PLED_MODE_REGISTER:
>> +        state->pled_mode_register = data;
>> +        break;
>> +    case TIMEOUT_REGISTER:
>> +        state->timeout_register = data;
>> +        wdt_w83627thf_restart_timer(state);
>> +        break;
>> +    case TIMER_REGISTER:
>> +        if (data & 0x20) {
>> +            timer_del(state->timer);
>> +            watchdog_perform_action();
>> +        }
>> +        state->timer_register = (data & ~0x20);
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionPortio wdt_portio_list[] = {
>> +    { WDT_W83627THF_EFER, 1, 1, .write = wdt_w83627thf_write_efer, },
>> +    { WDT_W83627THF_EFDR, 1, 1, .read  = wdt_w83627thf_read_efdr,
>> +                                .write = wdt_w83627thf_write_efdr },
>> +    PORTIO_END_OF_LIST(),
>> +};
>> +
>> +static void wdt_w83627thf_realize(DeviceState *dev, Error **errp)
>> +{
>> +    W83627THFState *d = WATCHDOG_W83627THF_DEVICE(dev);
>> +
>> +    d->timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, wdt_w83627thf_timer_tick, d);
>> +
>> +    portio_list_init(&d->port_list, OBJECT(d), wdt_portio_list, d, "w83627thf");
>> +    portio_list_add(&d->port_list, isa_address_space_io(&d->parent_obj), 0);
>> +}
>> +
>> +static void wdt_w83627thf_reset(DeviceState *dev)
>> +{
>> +    W83627THFState *d = WATCHDOG_W83627THF_DEVICE(dev);
>> +
>> +    timer_del(d->timer);
>> +}
>> +
>> +static void wdt_w83627thf_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = wdt_w83627thf_realize;
>> +    dc->reset = wdt_w83627thf_reset;
>> +    dc->vmsd = &vmstate_w83627thf;
>> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>> +}
>> +
>> +static const TypeInfo w83627thf_info = {
>> +    .name          = "w83627thf",
>> +    .parent        = TYPE_ISA_DEVICE,
>> +    .instance_size = sizeof(W83627THFState),
>> +    .class_init    = wdt_w83627thf_class_init,
>> +};
>> +
>> +static void w83627thf_register_types(void)
>> +{
>> +    watchdog_add_model(&model);
>> +    type_register_static(&w83627thf_info);
>> +}
>> +
>> +type_init(w83627thf_register_types)
> 
> Best regards,
> Daniel Fahlgren
> 
> 
>