On Sat, 17 Jul 2010, David Young wrote:
ipmi(4) should probably not suspend if its watchdog timer is active.
Would it be sufficient for ipmi(4) to refuse to suspend (return
false from the suspend method) if the watchdog is active?
Yes. I think that's the right thing to do for now.
This is actually fairly easy to do.
Or should it make some sort of effort to disable the watchdog, and
attempt to reenable the watchdog during the subsequent resume?
That defeats the purpose of a watchdog timer, at least for my purposes.
This is a local policy choice that should probably not be hard-coded.
Maybe we need additional watchdog modes?
1 Watchdog countdown stops during suspension, restarts after
Probably a bit more difficult than #3 below, and it assumes that the
remaining time is available.
2 Watchdog keeps counting down during suspend (we'll wake periodically
to tickle it)
This might be rather tricky. Depending on the type of suspend, the time
for a wake-up to occur could be faily long and/or variable. And there
might not even be a way for the system to schedule its own wake event.
3 Watchdog prevents suspension
As I indicated, this is fairly easy to do, simply check if the w-dog is
armed or not.
The attached patch provides an ipmi_suspend() method to implement #3.
But I suspect that this is really a more generic watchdog capability,
and it should be implemented for the entire class of watchdogs rather
than only for ipmi(4)?
Comments?
-------------------------------------------------------------------------
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------
Index: ipmi.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/ipmi.c,v
retrieving revision 1.46
diff -u -p -r1.46 ipmi.c
--- ipmi.c 10 Apr 2010 19:02:39 -0000 1.46
+++ ipmi.c 17 Jul 2010 02:40:27 -0000
@@ -225,6 +225,8 @@ int ipmi_sensor_status(struct ipmi_softc
int add_child_sensors(struct ipmi_softc *, uint8_t *, int, int, int,
int, int, int, const char *);
+bool ipmi_suspend(device_t, const pmf_qual_t *);
+
struct ipmi_if kcs_if = {
"KCS",
IPMI_IF_KCS_NREGS,
@@ -1934,6 +1936,10 @@ ipmi_thread(void *cookie)
sc->sc_wdog.smw_tickle = ipmi_watchdog_tickle;
sysmon_wdog_register(&sc->sc_wdog);
+ /* Set up a power handler so we can possibly sleep */
+ if (!pmf_device_register(self, ipmi_suspend, NULL))
+ aprint_error_dev(self, "couldn't establish a power handler\n");
+
mutex_enter(&sc->sc_poll_mtx);
while (sc->sc_thread_running) {
ipmi_refresh_sensors(sc);
@@ -2093,3 +2099,14 @@ ipmi_dotickle(struct ipmi_softc *sc)
device_xname(sc->sc_dev), rc);
}
}
+
+bool
+ipmi_suspend(device_t dev, const pmf_qual_t *qual)
+{
+ struct ipmi_softc *sc = device_private(dev);
+
+ /* Don't allow suspend if watchdog is armed */
+ if ((sc->sc_wdog.smw_mode & WDOG_MODE_MASK) != WDOG_MODE_DISARMED)
+ return false;
+ return true;
+}
