Hi Mike,
On Tue, Sep 16, 2014 at 12:43:11AM -0700, Mike Larkin wrote:
> On Mon, Sep 15, 2014 at 04:57:24PM +0200, S??bastien Marie wrote:
> > ping ?
> >
> > Tihs patch is very conservative: it just allow to switch fan OFF if
> > state is unknown.
>
> I finally read through the relevant parts of the spec. The problem
> here is that it looks like we are implementing thermal zone hysteresis
> incorrectly. I think you noticed this in your original diff, but IMO
> we've gone down a bit of a wrong path here with these later diffs.
>
The first diff try to not go back to "unknown" state for ALx
(sc->sc_ac_stat) when notify 0x81 is received by force reread the ALx
value (but the method used is, at least, unelegant).
The later diffs just copte with unknown state by just ignore it (if we
don't known if fan is on or off, and it should be set to off, so set it
off, even if already in this state). Basically because I suppose that if
sc_ac_stat is set to -1, it should be necessary for some hardware...
The real question would be to known if ALx state should be considered
unknown after hysteresis (if ACx change).
At startup it is ok to considere ALx status to -1 (sometimes, after hot
reboot in particular, the ALx could be ON at startup). But it is not
evident that when notify 0x81 is received, ALx should be considered to
unknown: currently the code set sc->sc_ac_stat[i] to -1 every time.
The following patch try to change this:
- copte with unknown state: permit acpitz_setfan(sc, i, "_OFF") when
sc->sc_ac_stat[i] == -1 (unknown)
- init sc->sc_ac_stat[i] to unknown:
- at system startup (ACPITZ_INIT): the system will "naturally"
initialise it to the good value on first active_cooling.
- on notify 0x82 (ACPITZ_DEVLIST): it seems to me it is
valuable to set unknown state to sc_ac_stat when _ALx change, as
sc_ac_stat reflect the _STA value of _ALx
Thanks.
--
Sébastien Marie
Index: acpitz.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpitz.c,v
retrieving revision 1.47
diff -u -p -r1.47 acpitz.c
--- acpitz.c 12 Jul 2014 02:44:49 -0000 1.47
+++ acpitz.c 19 Sep 2014 11:39:07 -0000
@@ -140,7 +142,6 @@ acpitz_init(struct acpitz_softc *sc, int
for (i = 0; i < ACPITZ_MAX_AC; i++) {
snprintf(name, sizeof(name), "_AC%d", i);
sc->sc_ac[i] = acpitz_getreading(sc, name);
- sc->sc_ac_stat[i] = -1;
}
}
@@ -161,6 +162,8 @@ acpitz_init(struct acpitz_softc *sc, int
sc->sc_devnode, &res, 0);
aml_freevalue(&res);
}
+ /* initialize current state to unknown */
+ sc->sc_ac_stat[i] = -1;
}
}
}
@@ -424,7 +439,7 @@ acpitz_refresh(void *arg)
acpitz_setfan(sc, i, "_ON_");
} else if (sc->sc_ac[i] != -1) {
/* turn off fan i */
- if (sc->sc_ac_stat[i] > 0)
+ if ((sc->sc_ac_stat[i] == -1) || (sc->sc_ac_stat[i] >
0))
acpitz_setfan(sc, i, "_OFF");
}
}
