Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 26 Jun 2019 20:49:36 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     =?UTF-8?B?WmJ5bsSbaw==?= Kocur <zbynek.kocur@fel.cvut.cz>
Cc:     Andreas Klinger <ak@it-klinger.de>, linux-iio@vger.kernel.org,
        knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring
Message-ID: <20190626204936.2756cefd@archlinux>
In-Reply-To: <03D5EE82-D6C7-4A03-9404-45B48F1EF5B6@fel.cvut.cz>
References: <20190623122909.hhzskp6k5vm4wify@arbad>
        <20190626192134.4b7aed3c@archlinux>
        <03D5EE82-D6C7-4A03-9404-45B48F1EF5B6@fel.cvut.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 26 Jun 2019 20:59:08 +0200
Zbyn=C4=9Bk Kocur <zbynek.kocur@fel.cvut.cz> wrote:

> Hello, Andreas,
>=20
> Your patch seems to solve my question. I haven't had time to test it beca=
use 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

>=20
> Zbyn=C4=9Bk
> ---
> email: zbynek.kocur@fel.cvut.cz
> phone: +420 224 354 054
> web: http://www.fel.cvut.cz
> Department of Telecommunications Engineering
> Faculty of Electrical Engineering
>=20
> > On 26 Jun 2019, at 20:21, Jonathan Cameron <jic23@kernel.org> wrote:
> >=20
> > On Sun, 23 Jun 2019 14:29:10 +0200
> > Andreas Klinger <ak@it-klinger.de> wrote:
> >  =20
> >> The measured time value in the driver is limited to the maximum distan=
ce
> >> which can be read by the sensor. This limitation was wrong and is fixed
> >> by this patch.
> >>=20
> >> 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.
> >>=20
> >> Suggested-by: Zbyn=C4=9Bk Kocur <zbynek.kocur@fel.cvut.cz>
> >> Signed-off-by: Andreas Klinger <ak@it-klinger.de> =20
> > Ideally I'm looking for Zbyn=C4=9Bk to confirm that this addresses the
> > original question.
> >=20
> > Thanks,
> >=20
> > Jonathan
> >  =20
> >> ---
> >> drivers/iio/proximity/srf04.c | 29 +++++++++++++++--------------
> >> 1 file changed, 15 insertions(+), 14 deletions(-)
> >>=20
> >> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf=
04.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);
> >>=20
> >> -	/* it cannot take more than 20 ms */
> >> +	/* it should not take more than 20 ms until echo is rising */
> >> 	ret =3D 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;
> >> 	}
> >>=20
> >> -	ret =3D wait_for_completion_killable_timeout(&data->falling, HZ/50);
> >> +	/* it cannot take more than 50 ms until echo is falling */
> >> +	ret =3D 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)
> >>=20
> >> 	dt_ns =3D 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
> >> 	 * =3D=3D> filter out invalid results for not measuring echos of
> >> 	 *     another us sensor
> >> 	 *
> >> 	 * formula:
> >> -	 *         distance       3 m
> >> -	 * time =3D ---------- =3D --------- =3D 9404389 ns
> >> -	 *          speed       319 m/s
> >> +	 *         distance     6,45 * 2 m
> >> +	 * time =3D ---------- =3D ------------ =3D 40438871 ns
> >> +	 *          speed         319 m/s
> >> 	 *
> >> 	 * using a minimum speed at -20 =C2=B0C of 319 m/s
> >> 	 */
> >> -	if (dt_ns > 9404389)
> >> +	if (dt_ns > 40438871)
> >> 		return -EIO;
> >>=20
> >> 	time_ns =3D dt_ns;
> >> @@ -159,20 +160,20 @@ static int srf04_read(struct srf04_data *data)
> >> 	 *   with Temp in =C2=B0C
> >> 	 *   and speed in m/s
> >> 	 *
> >> -	 * use 343 m/s as ultrasonic speed at 20 =C2=B0C here in absence of =
the
> >> +	 * use 343,5 m/s as ultrasonic speed at 20 =C2=B0C here in absence o=
f the
> >> 	 * temperature
> >> 	 *
> >> 	 * therefore:
> >> -	 *             time     343
> >> -	 * distance =3D ------ * -----
> >> -	 *             10^6       2
> >> +	 *             time     343,5     time * 106
> >> +	 * distance =3D ------ * ------- =3D ------------
> >> +	 *             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 =3D time_ns * 343 / 2000000;
> >> +	distance_mm =3D time_ns * 106 / 617176;
> >>=20
> >> 	return distance_mm;
> >> } =20
> >  =20
>=20