Module Name:    src
Committed By:   phx
Date:           Wed Sep  9 17:16:20 UTC 2015

Modified Files:
        src/share/man/man4: hythygtemp.4
        src/sys/dev/i2c: hytp14.c hytp14var.h

Log Message:
Tests showed that the sensor needs at least 30ms after a MR (measurement
request) to have valid data ready, so the driver didn't work very well.
Now the MR is sent in configurable intervals (hw.hythygtemp0.interval)
using callout(9), so that valid data is immediately available for
sysmon's sensor refresh. When the refresh comes too close after the last
MR, then the previous values are used.


To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/share/man/man4/hythygtemp.4
cvs rdiff -u -r1.4 -r1.5 src/sys/dev/i2c/hytp14.c
cvs rdiff -u -r1.2 -r1.3 src/sys/dev/i2c/hytp14var.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/share/man/man4/hythygtemp.4
diff -u src/share/man/man4/hythygtemp.4:1.2 src/share/man/man4/hythygtemp.4:1.3
--- src/share/man/man4/hythygtemp.4:1.2	Sun May 18 12:45:03 2014
+++ src/share/man/man4/hythygtemp.4	Wed Sep  9 17:16:20 2015
@@ -1,4 +1,4 @@
-.\"	$NetBSD: hythygtemp.4,v 1.2 2014/05/18 12:45:03 wiz Exp $
+.\"	$NetBSD: hythygtemp.4,v 1.3 2015/09/09 17:16:20 phx Exp $
 .\"
 .\" Copyright (c) 2014 Frank Kardel
 .\" All rights reserved.
@@ -44,13 +44,28 @@ The
 .Ar addr
 argument selects the address at the
 .Xr iic 4
-bus.
+bus. The sampling interval can be changed through the
+.Xr sysctl 8
+node
+.Li hw.hythygtemp0.interval .
+.Pp
 The sensor chips can be reconfigured to respond to other addresses than the
-default value of 0x28.
+default value of 0x28 by external utilities, like for example the
+.Pa pkgsrc/sysutils/hytctl
+package.
+.Sh SYSCTL VARIABLES
+The following
+.Xr sysctl 3
+variables are provided:
+.Bl -tag -width indent
+.It hw.hythygtemp0.interval
+Defines the sensor's sampling interval in seconds. It defaults to 50 seconds.
+.El
 .Sh SEE ALSO
 .Xr envsys 4 ,
 .Xr iic 4 ,
-.Xr envstat 8
+.Xr envstat 8 ,
+.Xr sysctl 8
 .Sh HISTORY
 The
 .Nm
@@ -61,4 +76,6 @@ driver first appeared in
 The
 .Nm
 driver was written by
-.An Frank Kardel Aq Mt kar...@netbsd.org .
+.An Frank Kardel Aq Mt kar...@netbsd.org
+and
+.An Frank Wille Aq p...@netbsd.org .

Index: src/sys/dev/i2c/hytp14.c
diff -u src/sys/dev/i2c/hytp14.c:1.4 src/sys/dev/i2c/hytp14.c:1.5
--- src/sys/dev/i2c/hytp14.c:1.4	Thu Apr 23 23:23:00 2015
+++ src/sys/dev/i2c/hytp14.c	Wed Sep  9 17:16:20 2015
@@ -37,16 +37,16 @@
  */ 
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: hytp14.c,v 1.4 2015/04/23 23:23:00 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: hytp14.c,v 1.5 2015/09/09 17:16:20 phx Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
 #include <sys/device.h>
 #include <sys/module.h>
+#include <sys/sysctl.h>
 
 #include <dev/sysmon/sysmonvar.h>
-
 #include <dev/i2c/i2cvar.h>
 #include <dev/i2c/hytp14reg.h>
 #include <dev/i2c/hytp14var.h>
@@ -54,12 +54,14 @@ __KERNEL_RCSID(0, "$NetBSD: hytp14.c,v 1
 static int hytp14_match(device_t, cfdata_t, void *);
 static void hytp14_attach(device_t, device_t, void *);
 static int hytp14_detach(device_t, int);
+static void hytp14_measurement_request(void *);
 static int hytp14_refresh_sensor(struct hytp14_sc *sc);
 static void hytp14_refresh(struct sysmon_envsys *, envsys_data_t *);
 static void hytp14_refresh_humidity(struct hytp14_sc *, envsys_data_t *);
 static void hytp14_refresh_temp(struct hytp14_sc *, envsys_data_t *);
+static int sysctl_hytp14_interval(SYSCTLFN_ARGS);
 
-/* #define HYT_DEBUG 3 */
+/*#define HYT_DEBUG 3*/
 #ifdef HYT_DEBUG
 volatile int hythygtemp_debug = HYT_DEBUG;
 
@@ -91,7 +93,9 @@ static struct hytp14_sensor hytp14_senso
 static int
 hytp14_match(device_t parent, cfdata_t match, void *aux)
 {
-	struct i2c_attach_args *ia = aux;
+	struct i2c_attach_args *ia;
+
+	ia = aux;
 
 	if (ia->ia_name) {
 		/* direct config - check name */
@@ -108,14 +112,17 @@ hytp14_match(device_t parent, cfdata_t m
 static void
 hytp14_attach(device_t parent, device_t self, void *aux)
 {
-	struct hytp14_sc *sc = device_private(self);
-	struct i2c_attach_args *ia = aux;
+	const struct sysctlnode *rnode, *node;
+	struct hytp14_sc *sc;
+	struct i2c_attach_args *ia;
 	int i;
 
+	ia = aux;
+	sc = device_private(self);
+
 	sc->sc_dev = self;
 	sc->sc_tag = ia->ia_tag;
 	sc->sc_addr = ia->ia_addr;
-	sc->sc_refresh = 0;
 	sc->sc_valid = ENVSYS_SINVALID;
 	sc->sc_numsensors = __arraycount(hytp14_sensors);
 
@@ -134,7 +141,7 @@ hytp14_attach(device_t parent, device_t 
 		sc->sc_sensors[i].state = ENVSYS_SINVALID;
 		
 		DPRINTF(2, ("hytp14_attach: registering sensor %d (%s)\n", i,
-			    sc->sc_sensors[i].desc));
+		    sc->sc_sensors[i].desc));
 		
 		if (sysmon_envsys_sensor_attach(sc->sc_sme, &sc->sc_sensors[i])) {
 			aprint_error_dev(sc->sc_dev,
@@ -157,80 +164,158 @@ hytp14_attach(device_t parent, device_t 
 		return;
 	}
 
+	/* create a sysctl node for setting the measurement interval */
+	rnode = node = NULL;
+	sysctl_createv(NULL, 0, NULL, &rnode,
+	    CTLFLAG_READWRITE,
+	    CTLTYPE_NODE, device_xname(sc->sc_dev), NULL,
+	    NULL, 0, NULL, 0,
+	    CTL_HW, CTL_CREATE, CTL_EOL);
+
+	if (rnode != NULL)
+		sysctl_createv(NULL, 0, NULL, &node,
+		    CTLFLAG_READWRITE | CTLFLAG_OWNDESC,
+		    CTLTYPE_INT, "interval",
+		    SYSCTL_DESCR("Sensor sampling interval in seconds"),
+		    sysctl_hytp14_interval, 0, (void *)sc, 0,
+		    CTL_HW, rnode->sysctl_num, CTL_CREATE, CTL_EOL);
+
 	aprint_normal(": HYT-221/271/939 humidity and temperature sensor\n");
+
+	/* set up callout for the default measurement interval */
+	sc->sc_mrinterval = HYTP14_MR_INTERVAL;
+	callout_init(&sc->sc_mrcallout, 0);
+	callout_setfunc(&sc->sc_mrcallout, hytp14_measurement_request, sc);
+
+	/* issue initial measurement request */
+	hytp14_measurement_request(sc);
 }
 
-static int hytp14_detach(device_t self, int flags)
+static int
+hytp14_detach(device_t self, int flags)
 {
-	struct hytp14_sc *sc = device_private(self);
+	struct hytp14_sc *sc;
+
+	sc = device_private(self);
 
 	if (sc->sc_sme != NULL) {
 		sysmon_envsys_unregister(sc->sc_sme);
 		sc->sc_sme = NULL;
 	}
-	
+
+	/* stop our measurement requests */
+	callout_stop(&sc->sc_mrcallout);
+	callout_destroy(&sc->sc_mrcallout);
+
 	return 0;
 }
 
-static int
-hytp14_refresh_sensor(struct hytp14_sc *sc)
+static void
+hytp14_measurement_request(void *aux)
 {
-	int error = 0;
 	uint8_t buf[I2C_EXEC_MAX_BUFLEN];
+	struct hytp14_sc *sc;
+	int error;
 
-	/* no more than once per second */
-	if (hardclock_ticks - sc->sc_refresh < hz)
-		return sc->sc_valid;
-	
-	DPRINTF(2, ("hytp14_refresh_sensor(%s)\n", device_xname(sc->sc_dev)));
+	sc = aux;
+	DPRINTF(2, ("%s(%s)\n", __func__, device_xname(sc->sc_dev)));
+
+	error = iic_acquire_bus(sc->sc_tag, 0);
+	if (error == 0) {
 
-	if ((error = iic_acquire_bus(sc->sc_tag, 0)) == 0) {
-		DPRINTF(3, ("hytp14_refresh_sensor(%s): bus locked\n", device_xname(sc->sc_dev)));
+		/* send DF command - read last data from sensor */
+		error = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP,
+		    sc->sc_addr, NULL, 0, sc->sc_data, sizeof(sc->sc_data), 0);
+		if (error != 0) {
+			DPRINTF(2, ("%s: %s: failed read from 0x%02x - error %d\n",
+			    device_xname(sc->sc_dev), __func__,
+			    sc->sc_addr, error));
+		} else {
+			DPRINTF(3, ("%s(%s): DF success : "
+			    "0x%02x%02x%02x%02x\n",
+			    __func__, device_xname(sc->sc_dev),
+			    sc->sc_data[0], sc->sc_data[1],
+			    sc->sc_data[2], sc->sc_data[3]));
+
+			/* remember last data, when valid */
+			if (!(sc->sc_data[0] &
+			    (HYTP14_RESP_CMDMODE | HYTP14_RESP_STALE))) {
+				memcpy(sc->sc_last, sc->sc_data,
+				    sizeof(sc->sc_last));
+				sc->sc_valid = ENVSYS_SVALID;
+			}
+		}
 
-		/* send MR command */
-                /* avoid quick read/write by providing a result buffer */
+		/* send MR command to request a new measurement */
 		error = iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP,
-				 sc->sc_addr, NULL, 0, buf, sizeof buf, 0);
-                if (error == 0) {
-			DPRINTF(3, ("hytp14_refresh_sensor(%s): MR sent\n",
-				    device_xname(sc->sc_dev)));
+		    sc->sc_addr, NULL, 0, buf, sizeof(buf), 0);
 
-			/* send DF command - read data from sensor */
-			error = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP,
-					 sc->sc_addr, NULL, 0, sc->sc_data,
-					      sizeof sc->sc_data, 0);
-                        if (error != 0) {
-				DPRINTF(2, ("%s: %s: failed read from 0x%02x - error %d\n",
-					    device_xname(sc->sc_dev),
-					    __func__, sc->sc_addr, error));
-			} else {
-				DPRINTF(2, ("hytp14_refresh_sensor(%s): DF success : 0x%02x%02x%02x%02x\n",
-					    device_xname(sc->sc_dev),
-					    sc->sc_data[0],
-					    sc->sc_data[1],
-					    sc->sc_data[2],
-					    sc->sc_data[3]));
-			}
+                if (error == 0) {
+			DPRINTF(3, ("%s(%s): MR sent\n",
+			    __func__, device_xname(sc->sc_dev)));
 		} else {
 			DPRINTF(2, ("%s: %s: failed read from 0x%02x - error %d\n",
-				    device_xname(sc->sc_dev), __func__,
-				    sc->sc_addr, error));
+			    device_xname(sc->sc_dev), __func__,
+			    sc->sc_addr, error));
 		}
 
 		iic_release_bus(sc->sc_tag, 0);	
-		DPRINTF(3, ("hytp14_refresh_sensor(%s): bus released\n", device_xname(sc->sc_dev)));
+		DPRINTF(3, ("%s(%s): bus released\n",
+		    __func__, device_xname(sc->sc_dev)));
 	} else {
-		DPRINTF(2, ("%s: %s: failed read from 0x%02x - error %d\n",
-			    device_xname(sc->sc_dev), __func__, sc->sc_addr, error));
+		DPRINTF(2, ("%s: %s: failed acquire i2c bus - error %d\n",
+		    device_xname(sc->sc_dev), __func__, error));
 	}
-			
-	sc->sc_refresh = hardclock_ticks;
-	
-	/* skip data if sensor is in command mode */
-	if (error == 0 && (sc->sc_data[0] & HYTP14_RESP_CMDMODE) == 0) {
-		sc->sc_valid = ENVSYS_SVALID;
+
+	/* schedule next measurement interval */
+	callout_schedule(&sc->sc_mrcallout, sc->sc_mrinterval * hz);
+}
+
+static int
+hytp14_refresh_sensor(struct hytp14_sc *sc)
+{
+	int error;
+
+	DPRINTF(2, ("%s(%s)\n", __func__, device_xname(sc->sc_dev)));
+
+	error = iic_acquire_bus(sc->sc_tag, 0);
+	if (error == 0) {
+
+		/* send DF command - read last data from sensor */
+		error = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP,
+		    sc->sc_addr, NULL, 0, sc->sc_data, sizeof(sc->sc_data), 0);
+		if (error != 0) {
+			DPRINTF(2, ("%s: %s: failed read from 0x%02x - error %d\n",
+			    device_xname(sc->sc_dev), __func__,
+			    sc->sc_addr, error));
+		} else {
+			DPRINTF(3, ("%s(%s): DF success : "
+			    "0x%02x%02x%02x%02x\n",
+			    __func__, device_xname(sc->sc_dev),
+			    sc->sc_data[0], sc->sc_data[1],
+			    sc->sc_data[2], sc->sc_data[3]));
+
+			/*
+			 * Use old data from sc_last[] when new data
+			 * is not yet valid (i.e. DF command came too
+			 * quickly after the last command).
+			 */
+			if (!(sc->sc_data[0] &
+			    (HYTP14_RESP_CMDMODE | HYTP14_RESP_STALE))) {
+				memcpy(sc->sc_last, sc->sc_data,
+				    sizeof(sc->sc_last));
+				sc->sc_valid = ENVSYS_SVALID;
+			} else
+				memcpy(sc->sc_data, sc->sc_last,
+				    sizeof(sc->sc_data));
+		}
+
+		iic_release_bus(sc->sc_tag, 0);	
+		DPRINTF(3, ("%s(%s): bus released\n",
+		    __func__, device_xname(sc->sc_dev)));
 	} else {
-		sc->sc_valid = ENVSYS_SINVALID;
+		DPRINTF(2, ("%s: %s: failed acquire i2c bus - error %d\n",
+		    device_xname(sc->sc_dev), __func__, error));
 	}
 	
 	return sc->sc_valid;
@@ -277,11 +362,35 @@ hytp14_refresh_temp(struct hytp14_sc *sc
 static void
 hytp14_refresh(struct sysmon_envsys *sme, envsys_data_t *edata)
 {
-	struct hytp14_sc *sc = sme->sme_cookie;
-	
+	struct hytp14_sc *sc;
+
+	sc = sme->sme_cookie;
 	hytp14_sensors[edata->sensor].refresh(sc, edata);
 }
 
+static int
+sysctl_hytp14_interval(SYSCTLFN_ARGS)
+{
+	struct sysctlnode node;
+	struct hytp14_sc *sc;
+	int32_t t;
+	int error;
+
+	node = *rnode;
+	sc = node.sysctl_data;
+
+	t = sc->sc_mrinterval;
+	node.sysctl_data = &t;
+
+	error = sysctl_lookup(SYSCTLFN_CALL(&node));
+	if (error || newp == NULL)
+		return error;
+	if (t <= 0)
+		return EINVAL;
+
+	sc->sc_mrinterval = t;
+	return 0;
+}
 
 MODULE(MODULE_CLASS_DRIVER, hythygtemp, "i2cexec,sysmon_envsys");
 
@@ -292,7 +401,9 @@ MODULE(MODULE_CLASS_DRIVER, hythygtemp, 
 static int
 hythygtemp_modcmd(modcmd_t cmd, void *opaque)
 {
-	int error = 0;
+	int error;
+
+	error = 0;
 
 	switch (cmd) {
 	case MODULE_CMD_INIT:

Index: src/sys/dev/i2c/hytp14var.h
diff -u src/sys/dev/i2c/hytp14var.h:1.2 src/sys/dev/i2c/hytp14var.h:1.3
--- src/sys/dev/i2c/hytp14var.h:1.2	Sun Aug 10 16:44:35 2014
+++ src/sys/dev/i2c/hytp14var.h	Wed Sep  9 17:16:20 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: hytp14var.h,v 1.2 2014/08/10 16:44:35 tls Exp $ */
+/* $NetBSD: hytp14var.h,v 1.3 2015/09/09 17:16:20 phx Exp $ */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -44,17 +44,23 @@
 
 #define HYTP14_NUM_SENSORS	2
 
+/* the default measurement interval is 50 seconds */
+#define HYTP14_MR_INTERVAL	50
+
 struct hytp14_sc {
 	device_t	sc_dev;
 	i2c_tag_t	sc_tag;
 	i2c_addr_t	sc_addr;
 
-	int		sc_refresh; /* last refresh from hardclock_ticks */
-        int             sc_valid;   /* ENVSYS validity state for this sensor */
-        uint8_t         sc_data[4]; /* last data read */
+	int		sc_valid;   /* ENVSYS validity state for this sensor */
+	uint8_t		sc_data[4]; /* current sensor data */
+	uint8_t		sc_last[4]; /* last sensor data, before MR */
 
 	int		sc_numsensors;
 
+	callout_t	sc_mrcallout;
+	int32_t		sc_mrinterval;
+
 	struct sysmon_envsys *sc_sme;
 	envsys_data_t sc_sensors[HYTP14_NUM_SENSORS];
 };
@@ -66,16 +72,3 @@ struct hytp14_sensor {
 };
 
 #endif
-/*
- * $Log: hytp14var.h,v $
- * Revision 1.2  2014/08/10 16:44:35  tls
- * Merge tls-earlyentropy branch into HEAD.
- *
- * Revision 1.1.4.2  2014/08/10 06:54:51  tls
- * Rebase.
- *
- * Revision 1.1  2014/05/18 11:46:23  kardel
- * add HYT-221/271/939 humidity/temperature I2C sensor
- * extend envsys(4) framework by %rH (relative humidity)
- *
- */

Reply via email to