OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
Re: uthum dropping out [was re ugold]

From: Stuart Henderson (sthenopenbsd.org)
Date: Sat May 04 2013 - 08:52:13 CDT


On 2013/05/04 01:49, Stuart Henderson wrote:
> On 2013/05/04 01:40, Stuart Henderson wrote:
> > --- uthum.c 15 Apr 2013 09:23:02 -0000 1.19
> > +++ uthum.c 4 May 2013 00:19:28 -0000
> > -515,7 +515,7 uthum_ntc_getdata(struct uthum_softc *sc
> > return EIO;
> >
> > /* get sensor value */
> > - if (uthum_read_data(sc, CMD_GETDATA_NTC, buf, sizeof(buf), 10) != 0) {
> > + if (uthum_read_data(sc, CMD_GETDATA_NTC, buf, sizeof(buf), 1000) != 0) {
> > DPRINTF(("uthum: data read fail\n"));
> > return EIO;
> > }
> > -600,6 +600,7 uthum_ntc_tuning(struct uthum_softc *sc,
> > }
> > ostate = state;
> > }
> > + tsleep(&sc->sc_sensortask, 0, "uthum", hz * 10);
> >
> > DPRINTF(("uthum: ntc tuning done. state change: 0x%.2x->0x%.2x\n",
> > s->cur_state, state));
> >
>
> ...of course, as soon as I send the diff out, I get a handful of
> messages even with the huge delays
>
> uthum_ntc_getdata: broken ntc data 0x16 0x00 0x31
> uthum_refresh_temperntc: data read fail
> uthum_ntc_getdata: broken ntc data 0x16 0x00 0x31
> uthum_refresh_temperntc: data read fail
> uthum_ntc_getdata: broken ntc data 0x16 0x00 0x31
> uthum_refresh_temperntc: data read fail
>
> but so far it has not got stuck, and the sensors stay attached.
> I'll re-check later...
>

so... each time this happens, the sensor device disappears, which
some userland monitoring programs don't cope with particularly well.
This happens fairly often, e.g. at

11:38:28
11:38:44
11:39:18
11:52:43
11:53:00
11:53:17
12:05:42
12:07:16 ...

I noticed that uthum_ntc_tuning() calls uthum_ntc_getdata() and permits
it to retry 3 times. New diff below takes a different approach: leave
timeouts as they were, and move this retry code up into uthum_ntc_getdata().

With this I do hit some "broken ntc data" DPRINTFs, however after retrying
the read is successful; sensor is updated and things are more robust.
I've also changed some DPRINTF() to make it clear which function
they're called from.

May 4 14:31:16 slate /bsd: uhidev0 at uhub0 port 2 configuration 1 interface 0 "Ten X Technology, Inc. TEMPer sensor" rev 1.10/1.50 addr 2
May 4 14:31:16 slate /bsd: uhidev0: iclass 3/1
May 4 14:31:16 slate /bsd: uthum0 at uhidev0
May 4 14:31:16 slate /bsd: uhidev1 at uhub0 port 2 configuration 1 interface 1 "Ten X Technology, Inc. TEMPer sensor" rev 1.10/1.50 addr 2
May 4 14:31:16 slate /bsd: uhidev1: iclass 3/0
May 4 14:31:16 slate /bsd: uthum1 at uhidev1
May 4 14:31:16 slate /bsd: uthum1: type ds75/12bit (temperature), calibration offset -1.0 degC
May 4 14:31:16 slate /bsd: uthum1: type NTC (temperature), calibration offset 1.0 degC
May 4 14:31:16 slate /bsd: uthum_attach: complete
May 4 14:31:16 slate /bsd: uthum: ntc tuning start. cur state = 0x61, val = 0x83d4
May 4 14:31:17 slate /bsd: uthum: ntc tuning done. state change: 0x61->0x65
May 4 14:34:39 slate /bsd: uthum_ntc_getdata: broken ntc data 0x18 0x80 0x31
May 4 14:38:53 slate /bsd: uthum_ntc_getdata: broken ntc data 0x18 0x80 0x31
May 4 14:39:16 slate /bsd: uthum_ntc_getdata: broken ntc data 0x18 0x90 0x31
May 4 14:39:38 slate /bsd: uthum_ntc_getdata: broken ntc data 0x18 0x80 0x31
May 4 14:40:01 slate /bsd: uthum_ntc_getdata: broken ntc data 0x18 0x80 0x31
May 4 14:40:24 slate /bsd: uthum_ntc_getdata: broken ntc data 0x18 0x80 0x31

any comments? OK?

Index: uthum.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uthum.c,v
retrieving revision 1.20
diff -u -p -r1.20 uthum.c
--- uthum.c 4 May 2013 12:22:14 -0000 1.20
+++ uthum.c 4 May 2013 13:44:54 -0000
-489,21 +489,31 uthum_setup_sensors(struct uthum_softc *
 int
 uthum_ntc_getdata(struct uthum_softc *sc, int *val)
 {
+ int retry = 3;
         uint8_t buf[8];
 
         if (val == NULL)
                 return EIO;
 
- /* get sensor value */
- if (uthum_read_data(sc, CMD_GETDATA_NTC, buf, sizeof(buf), 10) != 0) {
- DPRINTF(("uthum: data read fail\n"));
- return EIO;
+ while (retry) {
+ /* get sensor value */
+ if (uthum_read_data(sc, CMD_GETDATA_NTC,
+ buf, sizeof(buf), 10) != 0) {
+ DPRINTF(("%s: data read fail\n", __FUNCTION__));
+ retry--;
+ continue;
+ }
+ /* check data integrity */
+ if (buf[2] != CMD_GETDATA_EOF2) {
+ DPRINTF(("%s: broken ntc data 0x%.2x 0x%.2x 0x%.2x\n",
+ __FUNCTION__, buf[0], buf[1], buf[2]));
+ retry--;
+ continue;
+ }
+ break;
         }
-
- /* check data integrity */
- if (buf[2] != CMD_GETDATA_EOF2) {
- DPRINTF(("uthum: broken ntc data 0x%.2x 0x%.2x 0x%.2x\n",
- buf[0], buf[1], buf[2]));
+ if (retry <= 0) {
+ DPRINTF(("%s: too many failures", __FUNCTION__));
                 return EIO;
         }
 
-516,21 +526,13 uthum_ntc_tuning(struct uthum_softc *sc,
 {
         struct uthum_sensor *s;
         int done, state, ostate, curval;
- int retry = 3;
 
         s = &sc->sc_sensor[sensor];
         state = s->cur_state;
 
         /* get current sensor value */
         if (val == NULL) {
- while (retry) {
- if (uthum_ntc_getdata(sc, &curval)) {
- retry--;
- continue;
- } else
- break;
- }
- if (retry <= 0)
+ if (uthum_ntc_getdata(sc, &curval))
                         return EIO;
         } else {
                 curval = *val;
-623,7 +625,7 uthum_refresh_temperhum(struct uthum_sof
         int temp, rh;
 
         if (uthum_read_data(sc, CMD_GETDATA, buf, sizeof(buf), 1000) != 0) {
- DPRINTF(("uthum: data read fail\n"));
+ DPRINTF(("%s: data read fail\n", __FUNCTION__));
                 sc->sc_sensor[UTHUM_TEMPERHUM_TEMP].sensor.flags
                     |= SENSOR_FINVALID;
                 sc->sc_sensor[UTHUM_TEMPERHUM_HUM].sensor.flags
-661,15 +663,15 uthum_refresh_temper(struct uthum_softc
 
         /* get sensor value */
         if (uthum_read_data(sc, cmd, buf, sizeof(buf), 1000) != 0) {
- DPRINTF(("uthum: data read fail\n"));
+ DPRINTF(("%s: data read fail\n", __FUNCTION__));
                 sc->sc_sensor[sensor].sensor.flags |= SENSOR_FINVALID;
                 return;
         }
 
         /* check integrity */
         if (buf[2] != CMD_GETDATA_EOF) {
- DPRINTF(("uthum: broken ds75 data: 0x%.2x 0x%.2x 0x%.2x\n",
- buf[0], buf[1], buf[2]));
+ DPRINTF(("%s: broken ds75 data: 0x%.2x 0x%.2x 0x%.2x\n",
+ __FUNCTION__, buf[0], buf[1], buf[2]));
                 sc->sc_sensor[sensor].sensor.flags |= SENSOR_FINVALID;
                 return;
         }
-690,7 +692,7 uthum_refresh_temperntc(struct uthum_sof
 
         /* get sensor data */
         if (uthum_ntc_getdata(sc, &val)) {
- DPRINTF(("uthum: ntc data read fail\n"));
+ DPRINTF(("%s: data read fail\n", __FUNCTION__));
                 sc->sc_sensor[sensor].sensor.flags |= SENSOR_FINVALID;
                 return;
         }