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;
        }

Reply via email to