This driver has a poor read success rate when any amount of hard-IRQ
latency is encountered. On the Raspberry Pi platform for example, the
read success rate is ~50% with no hardware activity and practically 0%
with any other significant hardware activity.
Rewrite several aspects of the decode mechanism to make it more robust:
- Cater for GPIO chips that can raise interrupts when I/O pins
are set as output by expanding the maximum number of edges recorded.
- Start decoding the bitstream right-justified to make the driver agnostic
towards whether the start bits generate IRQs.
- Only bother decoding for GPIO "high" bit-times - the high-time is what
encodes the useful information.
- Use simple split-the-difference thresholding to decide if a received
data bit is a "1" or "0", instead of comparing relative bit periods.
- Refine the start bit timing to use usleep_range() to stop the later
sensor models from timing out if the msleep() takes longer than expected.
Also tweak a few error paths to be more informative.
The sensor decode success rate on a Pi after these changes is 99.1% with
no other hardware activity and approximately 78% with an Ethernet ping
flood happening concurrently.
There's still a few rough corners - several versions of the datasheet seem
to suggest that a double-read is required to get an up-to-date value, i.e.
the last value converted is the one returned when a readout is requested.
This can't be easily reconciled with the minimum repeat interval that is
also specified - a 2 second round-trip time is a long time to block for.
As these sensors have a slow (~10s) response time to environment changes,
it's reasonable to assume that a measured value from 2 seconds ago will
not be inaccurate.
Signed-off-by: Jonathan Bell <jonathan@xxxxxxxxxxxxxxx>
---
I only have access to an AM2302/DHT22 variant of the sensor. This patch
could use some testing on the other models in the range.
I am aware of a pending patch that causes the driver to bail out of a
reading if the kernel's timestamp resolution is too large. With the change
above, the driver won't be adversely affected unless the resolution is
very coarse - ~50us per tick.
drivers/iio/humidity/dht11.c | 157
+++++++++++++++++++++++-------------------
1 file changed, 86 insertions(+), 71 deletions(-)
diff --git a/drivers/iio/humidity/dht11.c b/drivers/iio/humidity/dht11.c
index 7d79a1a..855fb30 100644
--- a/drivers/iio/humidity/dht11.c
+++ b/drivers/iio/humidity/dht11.c
@@ -38,22 +38,28 @@
#define DRIVER_NAME "dht11"
-#define DHT11_DATA_VALID_TIME 2000000000 /* 2s in ns */
-
-#define DHT11_EDGES_PREAMBLE 2
+#define DHT11_START_EDGES 2
+#define DHT11_ACK_EDGES 2
#define DHT11_BITS_PER_READ 40
+#define DHT11_STOP_EDGES 2
+
+#define DHT11_MAX_EDGES (DHT11_START_EDGES + DHT11_ACK_EDGES + \
+ DHT11_BITS_PER_READ * 2 + DHT11_STOP_EDGES)
+
/*
- * Note that when reading the sensor actually 84 edges are detected, but
- * since the last edge is not significant, we only store 83:
+ * Sensor variants have differing requirements for the start pulse.
+ * DHT11 is minimum 18ms, DHT21 is 1-10ms, DHT22 is 1ms.
+ * If a >20ms start pulse is driven on a DHT22 then the sensor doesn't
respond
+ * at all. Stick to the minimum that should work for all variants.
*/
-#define DHT11_EDGES_PER_READ (2*DHT11_BITS_PER_READ +
DHT11_EDGES_PREAMBLE + 1)
-
-/* Data transmission timing (nano seconds) */
-#define DHT11_START_TRANSMISSION 18 /* ms */
-#define DHT11_SENSOR_RESPONSE 80000
-#define DHT11_START_BIT 50000
-#define DHT11_DATA_BIT_LOW 27000
-#define DHT11_DATA_BIT_HIGH 70000
+#define DHT11_START_PULSE_MIN 18000 /* us */
+#define DHT11_START_PULSE_MAX 20000 /* us */
+ /* Determinant for 1 or 0 decode. 27uS high-time for a 0, 70uS for a 1.
*/
+#define DHT11_BIT_THRESHOLD 51000 /* ns */
+/* Limit repeat interval. Datasheet specifies minimum 2s interval */
+#define DHT11_REPEAT_INTERVAL 2000000000 /* ns */
+/* This is approx. 2x maximum readout time */
+#define DHT11_CONV_TIMEOUT 20 /* ms */
struct dht11 {
struct device *dev;
@@ -70,10 +76,10 @@ struct dht11 {
/* num_edges: -1 means "no transmission in progress" */
int num_edges;
- struct {s64 ts; int value; } edges[DHT11_EDGES_PER_READ];
+ struct {s64 ts; int value; } edges[DHT11_MAX_EDGES];
};
-static unsigned char dht11_decode_byte(int *timing, int threshold)
+static unsigned char dht11_decode_byte(s64 *timing, int threshold)
{
unsigned char ret = 0;
int i;
@@ -87,46 +93,55 @@ static unsigned char dht11_decode_byte(int *timing,
int threshold)
return ret;
}
-static int dht11_decode(struct dht11 *dht11, int offset)
+static int dht11_decode(struct dht11 *dht11)
{
- int i, t, timing[DHT11_BITS_PER_READ], threshold,
- timeres = DHT11_SENSOR_RESPONSE;
+ int i, t, offset;
+ s64 timing[DHT11_BITS_PER_READ];
unsigned char temp_int, temp_dec, hum_int, hum_dec, checksum;
- /* Calculate timestamp resolution */
- for (i = 1; i < dht11->num_edges; ++i) {
- t = dht11->edges[i].ts - dht11->edges[i-1].ts;
- if (t > 0 && t < timeres)
- timeres = t;
- }
- if (2*timeres > DHT11_DATA_BIT_HIGH) {
- pr_err("dht11: timeresolution %d too bad for decoding\n",
- timeres);
+ dht11->timestamp = iio_get_time_ns();
+ /*
+ * Start our decode taking into account that some GPIO chips
+ * won't interrupt when we drive the start bits.
+ */
+ offset = dht11->num_edges - DHT11_STOP_EDGES - DHT11_BITS_PER_READ * 2;
+ if (offset < 0) {
+ dev_err(dht11->dev,
+ "Decode failure: only %d edges recorded\n",
+ dht11->num_edges);
return -EIO;
}
- threshold = DHT11_DATA_BIT_HIGH / timeres;
- if (DHT11_DATA_BIT_LOW/timeres + 1 >= threshold)
- pr_err("dht11: WARNING: decoding ambiguous\n");
-
- /* scale down with timeres and check validity */
- for (i = 0; i < DHT11_BITS_PER_READ; ++i) {
- t = dht11->edges[offset + 2*i + 2].ts -
- dht11->edges[offset + 2*i + 1].ts;
- if (!dht11->edges[offset + 2*i + 1].value)
- return -EIO; /* lost synchronisation */
- timing[i] = t / timeres;
+ /* Check that our pulse train hasn't been corrupted by latency */
+ for (i = 0; i < dht11->num_edges - 1; i++) {
+ if (dht11->edges[i].value == dht11->edges[i + 1].value) {
+ dev_err(dht11->dev,
+ "Decode failure: missed edges (%d total)\n",
+ dht11->num_edges);
+ return -EIO;
+ }
+ }
+ /* Find pairs of edges that correspond to a high interval */
+ for (i = offset, t = 0; i < dht11->num_edges - DHT11_STOP_EDGES; i++) {
+ if (dht11->edges[i].value == 1 &&
+ dht11->edges[i + 1].value == 0) {
+ if (t == DHT11_BITS_PER_READ)
+ break;
+ timing[t++] = dht11->edges[i + 1].ts -
+ dht11->edges[i].ts;
+ }
}
- hum_int = dht11_decode_byte(timing, threshold);
- hum_dec = dht11_decode_byte(&timing[8], threshold);
- temp_int = dht11_decode_byte(&timing[16], threshold);
- temp_dec = dht11_decode_byte(&timing[24], threshold);
- checksum = dht11_decode_byte(&timing[32], threshold);
+ hum_int = dht11_decode_byte(timing, DHT11_BIT_THRESHOLD);
+ hum_dec = dht11_decode_byte(&timing[8], DHT11_BIT_THRESHOLD);
+ temp_int = dht11_decode_byte(&timing[16], DHT11_BIT_THRESHOLD);
+ temp_dec = dht11_decode_byte(&timing[24], DHT11_BIT_THRESHOLD);
+ checksum = dht11_decode_byte(&timing[32], DHT11_BIT_THRESHOLD);
- if (((hum_int + hum_dec + temp_int + temp_dec) & 0xff) != checksum)
+ if (((hum_int + hum_dec + temp_int + temp_dec) & 0xff) != checksum) {
+ dev_err(dht11->dev, "Decode failure: invalid checksum\n");
return -EIO;
+ }
- dht11->timestamp = iio_get_time_ns();
if (hum_int < 20) { /* DHT22 */
dht11->temperature = (((temp_int & 0x7f) << 8) + temp_dec) *
((temp_int & 0x80) ? -100 : 100);
@@ -153,12 +168,13 @@ static irqreturn_t dht11_handle_irq(int irq, void
*data)
struct dht11 *dht11 = iio_priv(iio);
/* TODO: Consider making the handler safe for IRQ sharing */
- if (dht11->num_edges < DHT11_EDGES_PER_READ && dht11->num_edges >= 0) {
+ if (dht11->num_edges < DHT11_MAX_EDGES && dht11->num_edges >= 0) {
dht11->edges[dht11->num_edges].ts = iio_get_time_ns();
dht11->edges[dht11->num_edges++].value =
gpio_get_value(dht11->gpio);
-
- if (dht11->num_edges >= DHT11_EDGES_PER_READ)
+ /* Ensure other cores can see our array update */
+ smp_mb();
+ if (dht11->num_edges >= DHT11_MAX_EDGES)
complete(&dht11->completion);
}
@@ -173,42 +189,35 @@ static int dht11_read_raw(struct iio_dev *iio_dev,
int ret;
mutex_lock(&dht11->lock);
- if (dht11->timestamp + DHT11_DATA_VALID_TIME < iio_get_time_ns()) {
+ if (dht11->timestamp + DHT11_REPEAT_INTERVAL < iio_get_time_ns()) {
reinit_completion(&dht11->completion);
dht11->num_edges = 0;
+ /* Ensure the IRQ handler knows we've started a reading */
+ smp_mb();
+
ret = gpio_direction_output(dht11->gpio, 0);
if (ret)
goto err;
- msleep(DHT11_START_TRANSMISSION);
- ret = gpio_direction_input(dht11->gpio);
- if (ret)
- goto err;
- ret = request_irq(dht11->irq, dht11_handle_irq,
- IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
- iio_dev->name, iio_dev);
+ usleep_range(DHT11_START_PULSE_MIN, DHT11_START_PULSE_MAX);
+
+ ret = gpio_direction_input(dht11->gpio);
if (ret)
goto err;
ret = wait_for_completion_killable_timeout(&dht11->completion,
- HZ);
-
- free_irq(dht11->irq, iio_dev);
-
- if (ret == 0 && dht11->num_edges < DHT11_EDGES_PER_READ - 1) {
- dev_err(&iio_dev->dev,
- "Only %d signal edges detected\n",
- dht11->num_edges);
- ret = -ETIMEDOUT;
- }
+ msecs_to_jiffies(DHT11_CONV_TIMEOUT));
+ /*
+ * If the completion call times out, it could be the case that
+ * the IRQ handler may have reported data which could contain
+ * sufficient recorded edges to achieve decode.
+ * Test based on the number of edges instead of timeout.
+ */
if (ret < 0)
goto err;
- ret = dht11_decode(dht11,
- dht11->num_edges == DHT11_EDGES_PER_READ ?
- DHT11_EDGES_PREAMBLE :
- DHT11_EDGES_PREAMBLE - 2);
+ ret = dht11_decode(dht11);
if (ret)
goto err;
}
@@ -274,9 +283,15 @@ static int dht11_probe(struct platform_device *pdev)
return -EINVAL;
}
- dht11->timestamp = iio_get_time_ns() - DHT11_DATA_VALID_TIME - 1;
+ dht11->timestamp = iio_get_time_ns() - DHT11_REPEAT_INTERVAL - 1;
dht11->num_edges = -1;
+ ret = devm_request_irq(dev, dht11->irq, dht11_handle_irq,
+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+ pdev->name, iio);
+ if (ret)
+ return ret;
+
platform_set_drvdata(pdev, iio);
init_completion(&dht11->completion);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
