diff mbox

wdt_i6300esb: register a reset function

Message ID 20101212111512.GC2601@amd.home.annexia.org
State New
Headers show

Commit Message

Richard W.M. Jones Dec. 12, 2010, 11:15 a.m. UTC 
On Sun, Dec 12, 2010 at 10:59:59AM +0000, Blue Swirl wrote:
> On Sun, Dec 12, 2010 at 10:08 AM, Richard W.M. Jones <rjones@redhat.com> wrote:
> > On Sat, Dec 11, 2010 at 06:39:03PM +0000, Blue Swirl wrote:
> >> Thanks, applied.
> >
> > Wait!  This patch is incomplete.
> >
> > I already posted a complete patch already some months ago (twice) but
> > it was ignored both times:
> >
> > http://www.mail-archive.com/qemu-devel@nongnu.org/msg42716.html
> > http://www.mail-archive.com/qemu-devel@nongnu.org/msg43142.html
> 
> The difference is that previous_reboot_flag should not be cleared in
> reset, right?

Yes, and:

- Bernhard removed the call to i6300esb_reset after the watchdog
fires.  I'm not sure why this was done, since AFAIK the watchdog
should be completely reset by this event (as in a real machine).

- The same change is needed to IB700 as well.

> Could you make a new patch, please?

New patch attached.

Rich.

Comments

Blue Swirl Dec. 12, 2010, 1:19 p.m. UTC | #1 
Thanks, applied.

On Sun, Dec 12, 2010 at 11:15 AM, Richard W.M. Jones <rjones@redhat.com> wrote:
> On Sun, Dec 12, 2010 at 10:59:59AM +0000, Blue Swirl wrote:
>> On Sun, Dec 12, 2010 at 10:08 AM, Richard W.M. Jones <rjones@redhat.com> wrote:
>> > On Sat, Dec 11, 2010 at 06:39:03PM +0000, Blue Swirl wrote:
>> >> Thanks, applied.
>> >
>> > Wait!  This patch is incomplete.
>> >
>> > I already posted a complete patch already some months ago (twice) but
>> > it was ignored both times:
>> >
>> > http://www.mail-archive.com/qemu-devel@nongnu.org/msg42716.html
>> > http://www.mail-archive.com/qemu-devel@nongnu.org/msg43142.html
>>
>> The difference is that previous_reboot_flag should not be cleared in
>> reset, right?
>
> Yes, and:
>
> - Bernhard removed the call to i6300esb_reset after the watchdog
> fires.  I'm not sure why this was done, since AFAIK the watchdog
> should be completely reset by this event (as in a real machine).
>
> - The same change is needed to IB700 as well.
>
>> Could you make a new patch, please?
>
> New patch attached.
>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> virt-p2v converts physical machines to virtual machines.  Boot with a
> live CD or over the network (PXE) and turn machines into Xen guests.
> http://et.redhat.com/~rjones/virt-p2v
>
diff mbox

Patch

diff --git a/hw/wdt_i6300esb.c b/hw/wdt_i6300esb.c
index 8443b41..90bf5f6 100644
--- a/hw/wdt_i6300esb.c
+++ b/hw/wdt_i6300esb.c
@@ -149,6 +149,8 @@  static void i6300esb_reset(DeviceState *dev)
 
     i6300esb_disable_timer(d);
 
+    /* NB: Don't change d->previous_reboot_flag in this function. */
+
     d->reboot_enabled = 1;
     d->clock_scale = CLOCK_SCALE_1KHZ;
     d->int_type = INT_TYPE_IRQ;
@@ -159,7 +161,6 @@  static void i6300esb_reset(DeviceState *dev)
     d->timer2_preload = 0xfffff;
     d->stage = 1;
     d->unlock_state = 0;
-    d->previous_reboot_flag = 0;
 }
 
 /* This function is called when the watchdog expires.  Note that
@@ -193,6 +194,7 @@  static void i6300esb_timer_expired(void *vp)
         if (d->reboot_enabled) {
             d->previous_reboot_flag = 1;
             watchdog_perform_action(); /* This reboots, exits, etc */
+            i6300esb_reset(&d->dev.qdev);
         }
 
         /* In "free running mode" we start stage 1 again. */
@@ -409,6 +411,7 @@  static int i6300esb_init(PCIDevice *dev)
     i6300esb_debug("I6300State = %p\n", d);
 
     d->timer = qemu_new_timer(vm_clock, i6300esb_timer_expired, d);
+    d->previous_reboot_flag = 0;
 
     pci_conf = d->dev.config;
     pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
diff --git a/hw/wdt_ib700.c b/hw/wdt_ib700.c
index c34687b..b6235eb 100644
--- a/hw/wdt_ib700.c
+++ b/hw/wdt_ib700.c
@@ -97,6 +97,8 @@  static int wdt_ib700_init(ISADevice *dev)
 {
     IB700State *s = DO_UPCAST(IB700State, dev, dev);
 
+    ib700_debug("watchdog init\n");
+
     s->timer = qemu_new_timer(vm_clock, ib700_timer_expired, s);
     register_ioport_write(0x441, 2, 1, ib700_write_disable_reg, s);
     register_ioport_write(0x443, 2, 1, ib700_write_enable_reg, s);
@@ -104,16 +106,26 @@  static int wdt_ib700_init(ISADevice *dev)
     return 0;
 }
 
+static void wdt_ib700_reset(DeviceState *dev)
+{
+    IB700State *s = DO_UPCAST(IB700State, dev.qdev, dev);
+
+    ib700_debug("watchdog reset\n");
+
+    qemu_del_timer(s->timer);
+}
+
 static WatchdogTimerModel model = {
     .wdt_name = "ib700",
     .wdt_description = "iBASE 700",
 };
 
 static ISADeviceInfo wdt_ib700_info = {
-    .qdev.name = "ib700",
-    .qdev.size = sizeof(IB700State),
-    .qdev.vmsd = &vmstate_ib700,
-    .init      = wdt_ib700_init,
+    .qdev.name  = "ib700",
+    .qdev.size  = sizeof(IB700State),
+    .qdev.vmsd  = &vmstate_ib700,
+    .qdev.reset = wdt_ib700_reset,
+    .init       = wdt_ib700_init,
 };
 
 static void wdt_ib700_register_devices(void)