Re: [PATCH] iio: dht11: Read bit stream from IRQ on falling edges only

[Jonathan Cameron]

On Sun, 29 Jan 2023 19:05:23 +0100
Andreas Feldner <pelzi@xxxxxxxxxxxxxxx> wrote:

> Currently, IRQs for both falling and raising edges of the GPIO
> line connected to the DHT11 device are requested. However, the
> low states do not carry information, it is possible to determine
> 0 and 1 bits from the timing of two adjacent falling edges as
> well.
> 
> Doing so does no longer requires to read the GPIO line value
> within the IRQ handler, plus halves the number of IRQs to be
> handled at all.
> 
> Signed-off-by: Andreas Feldner <pelzi@xxxxxxxxxxxxxxx>

+CC Harald  Not been that many years since Harald replied, so address
may still be good.


> ---
>  drivers/iio/humidity/dht11.c | 28 +++++++++++-----------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/humidity/dht11.c b/drivers/iio/humidity/dht11.c
> index c97e25448772..d1cd053c5dd4 100644
> --- a/drivers/iio/humidity/dht11.c
> +++ b/drivers/iio/humidity/dht11.c
> @@ -30,13 +30,13 @@
>  
>  #define DHT11_DATA_VALID_TIME	2000000000  /* 2s in ns */
>  
> -#define DHT11_EDGES_PREAMBLE 2
> +#define DHT11_EDGES_PREAMBLE 1
>  #define DHT11_BITS_PER_READ 40
>  /*
>   * Note that when reading the sensor actually 84 edges are detected, but
>   * since the last edge is not significant, we only store 83:
>   */
> -#define DHT11_EDGES_PER_READ (2 * DHT11_BITS_PER_READ + \
> +#define DHT11_EDGES_PER_READ (DHT11_BITS_PER_READ + \
>  			      DHT11_EDGES_PREAMBLE + 1)
>  
>  /*
> @@ -46,6 +46,7 @@
>   * 1-bit: 68-75uS -- typically 70uS (AM2302)
>   * The acutal timings also depend on the properties of the cable, with
>   * longer cables typically making pulses shorter.
> + * Low time is constant 50uS.
>   *
>   * Our decoding depends on the time resolution of the system:
>   * timeres > 34uS ... don't know what a 1-tick pulse is
> @@ -63,7 +64,8 @@
>  #define DHT11_START_TRANSMISSION_MIN	18000  /* us */
>  #define DHT11_START_TRANSMISSION_MAX	20000  /* us */
>  #define DHT11_MIN_TIMERES	34000  /* ns */
> -#define DHT11_THRESHOLD		49000  /* ns */
> +#define DHT11_LOW		50000  /* ns */
> +#define DHT11_THRESHOLD		(49000 + DHT11_LOW)  /* ns */
>  #define DHT11_AMBIG_LOW		23000  /* ns */
>  #define DHT11_AMBIG_HIGH	30000  /* ns */
>  
> @@ -83,7 +85,7 @@ 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; }	edges[DHT11_EDGES_PER_READ];
>  };
>  
>  #ifdef CONFIG_DYNAMIC_DEBUG
> @@ -99,7 +101,7 @@ static void dht11_edges_print(struct dht11 *dht11)
>  	for (i = 1; i < dht11->num_edges; ++i) {
>  		dev_dbg(dht11->dev, "%d: %lld ns %s\n", i,
>  			dht11->edges[i].ts - dht11->edges[i - 1].ts,
> -			dht11->edges[i - 1].value ? "high" : "low");
> +			"falling");
>  	}
>  }
>  #endif /* CONFIG_DYNAMIC_DEBUG */
> @@ -125,14 +127,8 @@ static int dht11_decode(struct dht11 *dht11, int offset)
>  	unsigned char temp_int, temp_dec, hum_int, hum_dec, checksum;
>  
>  	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) {
> -			dev_dbg(dht11->dev,
> -				"lost synchronisation at edge %d\n",
> -				offset + 2 * i + 1);
> -			return -EIO;
> -		}
> +		t = dht11->edges[offset + i + 1].ts -
> +		    dht11->edges[offset + i].ts;
>  		bits[i] = t > DHT11_THRESHOLD;
>  	}
>  
> @@ -174,9 +170,7 @@ static irqreturn_t dht11_handle_irq(int irq, void *data)
>  	struct dht11 *dht11 = iio_priv(iio);
>  
>  	if (dht11->num_edges < DHT11_EDGES_PER_READ && dht11->num_edges >= 0) {
> -		dht11->edges[dht11->num_edges].ts = ktime_get_boottime_ns();
> -		dht11->edges[dht11->num_edges++].value =
> -						gpiod_get_value(dht11->gpiod);
> +		dht11->edges[dht11->num_edges++].ts = ktime_get_boottime_ns();
>  
>  		if (dht11->num_edges >= DHT11_EDGES_PER_READ)
>  			complete(&dht11->completion);
> @@ -224,7 +218,7 @@ static int dht11_read_raw(struct iio_dev *iio_dev,
>  			goto err;
>  
>  		ret = request_irq(dht11->irq, dht11_handle_irq,
> -				  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +				  IRQF_TRIGGER_FALLING,
>  				  iio_dev->name, iio_dev);
>  		if (ret)
>  			goto err;