Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Jonathan Cameron]

On Wed, 26 Jun 2019 20:59:08 +0200
ZbynÄk Kocur <zbynek.kocur@xxxxxxxxxxx> wrote:

> Hello, Andreas,
> 
> Your patch seems to solve my question. I haven't had time to test it because of holidays. I'll test it as soon as possible.

No rush, it's a clear fix anyway so we can queue this up
in the rc phase of the next kernel cycle!

Thanks,

Jonathan

> 
> ZbynÄk
> ---
> email: zbynek.kocur@xxxxxxxxxxx
> phone: +420 224 354 054
> web: http://www.fel.cvut.cz
> Department of Telecommunications Engineering
> Faculty of Electrical Engineering
> 
> > On 26 Jun 2019, at 20:21, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > 
> > On Sun, 23 Jun 2019 14:29:10 +0200
> > Andreas Klinger <ak@xxxxxxxxxxxxx> wrote:
> >   
> >> The measured time value in the driver is limited to the maximum distance
> >> which can be read by the sensor. This limitation was wrong and is fixed
> >> by this patch.
> >> 
> >> It also takes into account that we are supporting a variety of sensors
> >> today and that the recently added sensors have a higher maximum
> >> distance range.
> >> 
> >> Suggested-by: ZbynÄk Kocur <zbynek.kocur@xxxxxxxxxxx>
> >> Signed-off-by: Andreas Klinger <ak@xxxxxxxxxxxxx>  
> > Ideally I'm looking for ZbynÄk to confirm that this addresses the
> > original question.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> >> ---
> >> drivers/iio/proximity/srf04.c | 29 +++++++++++++++--------------
> >> 1 file changed, 15 insertions(+), 14 deletions(-)
> >> 
> >> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> >> index 8b50d56b0a03..01eb8cc63076 100644
> >> --- a/drivers/iio/proximity/srf04.c
> >> +++ b/drivers/iio/proximity/srf04.c
> >> @@ -110,7 +110,7 @@ static int srf04_read(struct srf04_data *data)
> >> 	udelay(data->cfg->trigger_pulse_us);
> >> 	gpiod_set_value(data->gpiod_trig, 0);
> >> 
> >> -	/* it cannot take more than 20 ms */
> >> +	/* it should not take more than 20 ms until echo is rising */
> >> 	ret = wait_for_completion_killable_timeout(&data->rising, HZ/50);
> >> 	if (ret < 0) {
> >> 		mutex_unlock(&data->lock);
> >> @@ -120,7 +120,8 @@ static int srf04_read(struct srf04_data *data)
> >> 		return -ETIMEDOUT;
> >> 	}
> >> 
> >> -	ret = wait_for_completion_killable_timeout(&data->falling, HZ/50);
> >> +	/* it cannot take more than 50 ms until echo is falling */
> >> +	ret = wait_for_completion_killable_timeout(&data->falling, HZ/20);
> >> 	if (ret < 0) {
> >> 		mutex_unlock(&data->lock);
> >> 		return ret;
> >> @@ -135,19 +136,19 @@ static int srf04_read(struct srf04_data *data)
> >> 
> >> 	dt_ns = ktime_to_ns(ktime_dt);
> >> 	/*
> >> -	 * measuring more than 3 meters is beyond the capabilities of
> >> -	 * the sensor
> >> +	 * measuring more than 6,45 meters is beyond the capabilities of
> >> +	 * the supported sensors
> >> 	 * ==> filter out invalid results for not measuring echos of
> >> 	 *     another us sensor
> >> 	 *
> >> 	 * formula:
> >> -	 *         distance       3 m
> >> -	 * time = ---------- = --------- = 9404389 ns
> >> -	 *          speed       319 m/s
> >> +	 *         distance     6,45 * 2 m
> >> +	 * time = ---------- = ------------ = 40438871 ns
> >> +	 *          speed         319 m/s
> >> 	 *
> >> 	 * using a minimum speed at -20 ÂC of 319 m/s
> >> 	 */
> >> -	if (dt_ns > 9404389)
> >> +	if (dt_ns > 40438871)
> >> 		return -EIO;
> >> 
> >> 	time_ns = dt_ns;
> >> @@ -159,20 +160,20 @@ static int srf04_read(struct srf04_data *data)
> >> 	 *   with Temp in ÂC
> >> 	 *   and speed in m/s
> >> 	 *
> >> -	 * use 343 m/s as ultrasonic speed at 20 ÂC here in absence of the
> >> +	 * use 343,5 m/s as ultrasonic speed at 20 ÂC here in absence of the
> >> 	 * temperature
> >> 	 *
> >> 	 * therefore:
> >> -	 *             time     343
> >> -	 * distance = ------ * -----
> >> -	 *             10^6       2
> >> +	 *             time     343,5     time * 106
> >> +	 * distance = ------ * ------- = ------------
> >> +	 *             10^6         2         617176
> >> 	 *   with time in ns
> >> 	 *   and distance in mm (one way)
> >> 	 *
> >> -	 * because we limit to 3 meters the multiplication with 343 just
> >> +	 * because we limit to 6,45 meters the multiplication with 106 just
> >> 	 * fits into 32 bit
> >> 	 */
> >> -	distance_mm = time_ns * 343 / 2000000;
> >> +	distance_mm = time_ns * 106 / 617176;
> >> 
> >> 	return distance_mm;
> >> }  
> >   
>