Module Name:    src
Committed By:   ad
Date:           Sat Nov 30 23:06:52 UTC 2019

Modified Files:
        src/sys/dev/onewire: owtemp.c

Log Message:
Make owtemp reliable for me:

- Don't do the calculation if there is a CRC error.
- If we get any kind of error during a refresh, retry up to three times.
- Add event counters to report what's going on.


To generate a diff of this commit:
cvs rdiff -u -r1.18 -r1.19 src/sys/dev/onewire/owtemp.c

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

Modified files:

Index: src/sys/dev/onewire/owtemp.c
diff -u src/sys/dev/onewire/owtemp.c:1.18 src/sys/dev/onewire/owtemp.c:1.19
--- src/sys/dev/onewire/owtemp.c:1.18	Fri Oct 25 16:25:14 2019
+++ src/sys/dev/onewire/owtemp.c	Sat Nov 30 23:06:52 2019
@@ -1,6 +1,35 @@
-/*	$NetBSD: owtemp.c,v 1.18 2019/10/25 16:25:14 martin Exp $	*/
+/*	$NetBSD: owtemp.c,v 1.19 2019/11/30 23:06:52 ad Exp $	*/
 /*	$OpenBSD: owtemp.c,v 1.1 2006/03/04 16:27:03 grange Exp $	*/
 
+/*-
+ * Copyright (c) 2019 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Andrew Doran.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
 /*
  * Copyright (c) 2006 Alexander Yurchenko <gra...@openbsd.org>
  *
@@ -22,7 +51,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: owtemp.c,v 1.18 2019/10/25 16:25:14 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: owtemp.c,v 1.19 2019/11/30 23:06:52 ad Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -51,14 +80,20 @@ struct owtemp_softc {
 	uint32_t			(*sc_owtemp_decode)(const uint8_t *);
 
 	int				sc_dying;
+
+	struct evcnt			sc_ev_update;
+	struct evcnt			sc_ev_rsterr;
+	struct evcnt			sc_ev_crcerr;
 };
 
 static int	owtemp_match(device_t, cfdata_t, void *);
 static void	owtemp_attach(device_t, device_t, void *);
 static int	owtemp_detach(device_t, int);
 static int	owtemp_activate(device_t, enum devact);
-
-static void	owtemp_update(void *);
+static bool	owtemp_update(struct owtemp_softc *sc, uint32_t *temp);
+static void	owtemp_refresh(struct sysmon_envsys *, envsys_data_t *);
+static uint32_t	owtemp_decode_ds18b20(const uint8_t *);
+static uint32_t	owtemp_decode_ds1920(const uint8_t *);
 
 CFATTACH_DECL_NEW(owtemp, sizeof(struct owtemp_softc),
 	owtemp_match, owtemp_attach, owtemp_detach, owtemp_activate);
@@ -71,10 +106,7 @@ static const struct onewire_matchfam owt
 	{ ONEWIRE_FAMILY_DS1822 },
 };
 
-static void	owtemp_refresh(struct sysmon_envsys *, envsys_data_t *);
-
-static uint32_t	owtemp_decode_ds18b20(const uint8_t *);
-static uint32_t	owtemp_decode_ds1920(const uint8_t *);
+int	owtemp_retries = 3;
 
 static int
 owtemp_match(device_t parent, cfdata_t match, void *aux)
@@ -110,6 +142,13 @@ owtemp_attach(device_t parent, device_t 
 		break;
 	}
 
+	evcnt_attach_dynamic(&sc->sc_ev_update, EVCNT_TYPE_MISC, NULL,
+	   device_xname(self), "update");
+	evcnt_attach_dynamic(&sc->sc_ev_rsterr, EVCNT_TYPE_MISC, NULL,
+	   device_xname(self), "reset error");
+	evcnt_attach_dynamic(&sc->sc_ev_crcerr, EVCNT_TYPE_MISC, NULL,
+	   device_xname(self), "crc error");
+
 	sc->sc_sme = sysmon_envsys_create();
 
 	/* Initialize sensor */
@@ -144,6 +183,9 @@ owtemp_detach(device_t self, int flags)
 	struct owtemp_softc *sc = device_private(self);
 
 	sysmon_envsys_unregister(sc->sc_sme);
+	evcnt_detach(&sc->sc_ev_rsterr);
+	evcnt_detach(&sc->sc_ev_update);
+	evcnt_detach(&sc->sc_ev_crcerr);
 
 	return 0;
 }
@@ -162,18 +204,12 @@ owtemp_activate(device_t self, enum deva
 	}
 }
 
-static void
-owtemp_update(void *arg)
+static bool
+owtemp_update(struct owtemp_softc *sc, uint32_t *temp)
 {
-	struct owtemp_softc *sc = arg;
 	u_int8_t data[9];
 
-	onewire_lock(sc->sc_onewire);
-	if (onewire_reset(sc->sc_onewire) != 0) {
-		aprint_error_dev(sc->sc_dv, "owtemp_update: 1st reset failed\n");
-		goto done;
-	}
-	onewire_matchrom(sc->sc_onewire, sc->sc_rom);
+	sc->sc_ev_update.ev_count++;
 
 	/*
 	 * Start temperature conversion. The conversion takes up to 750ms.
@@ -182,41 +218,54 @@ owtemp_update(void *arg)
 	 * As such, no other activity may take place on the 1-Wire bus for
 	 * at least this period.  Keep the parent bus locked while waiting.
 	 */
-	onewire_write_byte(sc->sc_onewire, DS_CMD_CONVERT);
-	kpause("owtemp", false, mstohz(750 + 10), NULL);
-
 	if (onewire_reset(sc->sc_onewire) != 0) {
-		aprint_error_dev(sc->sc_dv, "owtemp_update: 2nd reset failed\n");
-		goto done;
+		sc->sc_ev_rsterr.ev_count++;
+		return false;
 	}
 	onewire_matchrom(sc->sc_onewire, sc->sc_rom);
+	onewire_write_byte(sc->sc_onewire, DS_CMD_CONVERT);
+	(void)kpause("owtemp", false, mstohz(750 + 10), NULL);
 
 	/*
 	 * The result of the temperature measurement is placed in the
-	 * first two bytes of the scratchpad.
+	 * first two bytes of the scratchpad.  Perform the caculation
+	 * only if the CRC is correct.
 	 */
+	if (onewire_reset(sc->sc_onewire) != 0) {
+		sc->sc_ev_rsterr.ev_count++;
+		return false;
+	}
+	onewire_matchrom(sc->sc_onewire, sc->sc_rom);
 	onewire_write_byte(sc->sc_onewire, DS_CMD_READ_SCRATCHPAD);
 	onewire_read_block(sc->sc_onewire, data, 9);
-#if 0
-	if (onewire_crc(data, 8) == data[8]) {
-		sc->sc_sensor.value = 273150000 +
-		    (int)((u_int16_t)data[1] << 8 | data[0]) * 500000;
+	if (onewire_crc(data, 8) != data[8]) {
+		sc->sc_ev_crcerr.ev_count++;
+		return false;
 	}
-#endif
-
-	sc->sc_sensor.value_cur = sc->sc_owtemp_decode(data);
-	sc->sc_sensor.state = ENVSYS_SVALID;
-
-done:
-	onewire_unlock(sc->sc_onewire);
+	*temp = sc->sc_owtemp_decode(data);
+	return true;
 }
 
 static void
 owtemp_refresh(struct sysmon_envsys *sme, envsys_data_t *edata)
 {
 	struct owtemp_softc *sc = sme->sme_cookie;
+	uint32_t reading;
+	int retry;
 
-	owtemp_update(sc);
+	onewire_lock(sc->sc_onewire);
+	for (retry = 0; retry < owtemp_retries; retry++) {
+		if (owtemp_update(sc, &reading)) {
+			onewire_unlock(sc->sc_onewire);
+			sc->sc_sensor.value_cur = reading;
+			sc->sc_sensor.state = ENVSYS_SVALID;
+			return;
+		}
+	}
+	onewire_unlock(sc->sc_onewire);
+	aprint_error_dev(sc->sc_dv,
+	    "update failed - use vmstat(8) to check event counters\n");
+	sc->sc_sensor.state = ENVSYS_SINVALID;
 }
 
 static uint32_t

Reply via email to