lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8051b9f7-ef91-8746-1a13-693ec5cdc9e4@kernel.org>
Date:   Mon, 30 Jan 2017 20:36:17 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Andreas Klinger <ak@...klinger.de>, knaack.h@....de,
        lars@...afoo.de, pmeerw@...erw.net, gregkh@...uxfoundation.org,
        dledford@...hat.com, akpm@...ux-foundation.org,
        shraddha.6596@...il.com, w@....eu, balbi@...nel.org,
        mtk.manpages@...il.com, sjenning@...hat.com,
        ksenija.stanojevic@...il.com, vilhelm.gray@...il.com,
        robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
        ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-iio@...r.kernel.org
Subject: Re: [PATCH v2 2/2] iio: distance: add devantech us ranger srf04

On 30/01/17 12:51, Andreas Klinger wrote:
> This patch adds support for the ultrasonic ranger srf04 of devantech.
> 
> This device is measuring the distance of objects in a range between 1 cm
> and 3 meters and a theoretical resolution of 3 mm.
> 
> There are two GPIOs used:
>   - trigger: set as output to the device when the measurement should start
>   - echo: set by the device when the ultrasonic wave is sent out and reset
>     when the echo is recognized; this needs to be an interrupt input
> 
> The time between setting and resetting the echo pin is the time the
> waveform needed for one round trip. This time is recorded in the interrupt
> handler.
> 
> The distance is calculated in the read function by using the ultrasonic
> speed at 20 degrees celsius which is about 343 m/s.
> 
> Signed-off-by: Andreas Klinger <ak@...klinger.de>
A few minor bits and bobs inline.  Nice if somewhat unusual little driver.
The ideal using one of these would be to connect them up to a hardware
timer to get better precision but obviously they aren't always available.
(not to mention we don't have a good framework to handle them when they
are!)

Jonathan
> ---
>  MAINTAINERS                    |   6 +
>  drivers/iio/proximity/Kconfig  |  11 ++
>  drivers/iio/proximity/Makefile |   1 +
>  drivers/iio/proximity/srf04.c  | 313 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 331 insertions(+)
>  create mode 100644 drivers/iio/proximity/srf04.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fa6af014fd72..ba979cc48fd8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3469,6 +3469,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
>  S:	Maintained
>  F:	drivers/usb/dwc3/
>  
> +DEVANTECH SRF ULTRASONIC RANGER IIO DRIVER
> +M:	Andreas Klinger <ak@...klinger.de>
> +L:	linux-iio@...r.kernel.org
> +S:	Maintained
> +F:	drivers/iio/proximity/srf*.c
> +
>  DEVICE COREDUMP (DEV_COREDUMP)
>  M:	Johannes Berg <johannes@...solutions.net>
>  L:	linux-kernel@...r.kernel.org
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> index ef4c73db5b53..0bcbc66eae6e 100644
> --- a/drivers/iio/proximity/Kconfig
> +++ b/drivers/iio/proximity/Kconfig
> @@ -32,6 +32,17 @@ config LIDAR_LITE_V2
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called pulsedlight-lite-v2
>  
> +config SRF04
> +	tristate "Devantech SRF04 ultrasonic ranger sensor"
> +	depends on GPIOLIB
> +	help
> +	  Say Y here to build a driver for Devantech SRF04 ultrasonic
> +	  ranger sensor. This driver can be used to measure the distance
> +	  of objects. It is using two GPIOs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called srf04.
> +
>  config SX9500
>  	tristate "SX9500 Semtech proximity sensor"
>  	select IIO_BUFFER
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> index 9aadd9a8ee99..97c046c1dfc5 100644
> --- a/drivers/iio/proximity/Makefile
> +++ b/drivers/iio/proximity/Makefile
> @@ -5,4 +5,5 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_AS3935)		+= as3935.o
>  obj-$(CONFIG_LIDAR_LITE_V2)	+= pulsedlight-lidar-lite-v2.o
> +obj-$(CONFIG_SRF04)		+= srf04.o
>  obj-$(CONFIG_SX9500)		+= sx9500.o
> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> new file mode 100644
> index 000000000000..2cbb4dd6f175
> --- /dev/null
> +++ b/drivers/iio/proximity/srf04.c
> @@ -0,0 +1,313 @@
> +/*
> + * SRF04: ultrasonic sensor for distance measuring by using GPIOs
> + *
> + * Copyright (c) 2017 Andreas Klinger <ak@...klinger.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * For details about the device see:
> + * http://www.robot-electronics.co.uk/htm/srf04tech.htm
> + *
> + * the measurement cycle as timing diagram looks like:
> + *
> + *          +---+
> + * GPIO     |   |
> + * trig:  --+   +------------------------------------------------------
> + *          ^   ^
> + *          |<->|
> + *         udelay(10)
> + *
> + * ultra           +-+ +-+ +-+
> + * sonic           | | | | | |
> + * burst: ---------+ +-+ +-+ +-----------------------------------------
> + *                           .
> + * ultra                     .              +-+ +-+ +-+
> + * sonic                     .              | | | | | |
> + * echo:  ----------------------------------+ +-+ +-+ +----------------
> + *                           .                        .
> + *                           +------------------------+
> + * GPIO                      |                        |
> + * echo:  -------------------+                        +---------------
> + *                           ^                        ^
> + *                           interrupt                interrupt
> + *                           (ts_rising)              (ts_falling)
> + *                           |<---------------------->|
> + *                              pulse time measured
> + *                              --> one round trip of ultra sonic waves
Nice art work ;)
> + */
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/sched.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +struct srf04_data {
> +	struct device		*dev;
> +	struct gpio_desc	*gpiod_trig;
> +	struct gpio_desc	*gpiod_echo;
> +	struct mutex		lock;
> +	int			irqnr;
> +	ktime_t			ts_rising;
> +	ktime_t			ts_falling;
> +	struct completion	rising;
> +	struct completion	falling;
> +};
> +
> +static irqreturn_t srf04_handle_irq(int irq, void *dev_id)
> +{
> +	struct iio_dev *indio_dev = dev_id;
> +	struct srf04_data *data = iio_priv(indio_dev);
> +
> +	if (gpiod_get_value(data->gpiod_echo)) {
Why not grab the time first?  We can't sleep but we can take a while
over getting the gpio value and that time could in theory be variable.
> +		data->ts_rising = ktime_get();
> +		complete(&data->rising);
> +	} else {
> +		data->ts_falling = ktime_get();
> +		complete(&data->falling);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int srf04_read(struct srf04_data *data)
> +{
> +	int ret;
> +	ktime_t ktime_dt;
> +	u64 dt_ns;
> +	u32 time_ns, distance_mm;
> +
> +	/*
> +	 * just one read-echo-cycle can take place at a time
> +	 * ==> lock against concurrent reading calls
> +	 */
> +	mutex_lock(&data->lock);
> +
> +	reinit_completion(&data->rising);
> +	reinit_completion(&data->falling);
> +
> +	gpiod_set_value(data->gpiod_trig, 1);
> +	udelay(10);
> +	gpiod_set_value(data->gpiod_trig, 0);
> +
> +	/* it cannot take more than 20 ms */
> +	ret = wait_for_completion_killable_timeout(&data->rising, HZ/50);
> +	if (ret < 0) {
> +		mutex_unlock(&data->lock);
> +		return ret;
> +	} else if (ret == 0) {
> +		mutex_unlock(&data->lock);
> +		return -ETIMEDOUT;
> +	}
> +
> +	ret = wait_for_completion_killable_timeout(&data->falling, HZ/50);
> +	if (ret < 0) {
> +		mutex_unlock(&data->lock);
> +		return ret;
> +	} else if (ret == 0) {
> +		mutex_unlock(&data->lock);
> +		return -ETIMEDOUT;
> +	}
> +
> +	ktime_dt = ktime_sub(data->ts_falling, data->ts_rising);
> +
> +	mutex_unlock(&data->lock);
> +
> +	dt_ns = ktime_to_ns(ktime_dt);
> +	/*
> +	 * measuring more than 3 meters is beyond the capabilities of
> +	 * the sensor
> +	 * ==> filter out invalid results for not measuring echos of
> +	 *     another us sensor
> +	 *
> +	 * formula:
> +	 *         distance       3 m
> +	 * time = ---------- = --------- = 9404389 ns
> +	 *          speed       319 m/s
> +	 *
> +	 * using a minimum speed at -20 °C of 319 m/s
> +	 */
> +	if (dt_ns > 9404389)
> +		return -EIO;
> +
> +	time_ns = dt_ns;
> +
> +	/*
> +	 * the speed as function of the temperature is approximately:
> +	 *
> +	 * speed = 331,5 + 0,6 * Temp
> +	 *   with Temp in °C
> +	 *   and speed in m/s
> +	 *
> +	 * use 343 m/s as ultrasonic speed at 20 °C here in absence of the
> +	 * temperature
> +	 *
> +	 * therefore:
> +	 *             time     343
> +	 * distance = ------ * -----
> +	 *             10^6       2
> +	 *   with time in ns
> +	 *   and distance in mm (one way)
> +	 *
> +	 * because we limit to 3 meters the multiplication with 343 just
> +	 * fits into 32 bit
> +	 */
> +	distance_mm = time_ns * 343 / 2000000;
> +
> +	return distance_mm;
> +}
> +
> +static int srf04_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *channel, int *val,
> +			    int *val2, long info)
> +{
> +	struct srf04_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (channel->type != IIO_DISTANCE)
> +		return -EINVAL;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = srf04_read(data);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * theoretical maximum resolution is 3 mm
> +		 * 1 LSB is 1 mm
> +		 */
> +		*val = 0;
> +		*val2 = 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info srf04_iio_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= srf04_read_raw,
> +};
> +
> +static const struct iio_chan_spec srf04_chan_spec[] = {
> +	{
> +		.type = IIO_DISTANCE,
> +		.info_mask_separate =
> +				BIT(IIO_CHAN_INFO_RAW) |
> +				BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static int srf04_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct srf04_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(struct srf04_data));
> +	if (!indio_dev) {
> +		dev_err(dev, "failed to allocate IIO device\n");
> +		return -ENOMEM;
> +	}
> +
> +	data = iio_priv(indio_dev);
> +	data->dev = dev;
> +
> +	mutex_init(&data->lock);
> +	init_completion(&data->rising);
> +	init_completion(&data->falling);
> +
> +	data->gpiod_trig = devm_gpiod_get(dev, "trig", GPIOD_OUT_LOW);
> +	if (IS_ERR(data->gpiod_trig)) {
> +		dev_err(dev, "failed to get trig-gpios: err=%ld\n",
> +					PTR_ERR(data->gpiod_trig));
> +		return PTR_ERR(data->gpiod_trig);
> +	}
> +
> +	data->gpiod_echo = devm_gpiod_get(dev, "echo", GPIOD_IN);
> +	if (IS_ERR(data->gpiod_echo)) {
> +		dev_err(dev, "failed to get echo-gpios: err=%ld\n",
> +					PTR_ERR(data->gpiod_echo));
> +		return PTR_ERR(data->gpiod_echo);
> +	}
> +
> +	if (gpiod_cansleep(data->gpiod_echo)) {
> +		dev_err(data->dev, "cansleep-GPIOs not supported\n");
> +		return -ENODEV;
> +	}
> +
> +	data->irqnr = gpiod_to_irq(data->gpiod_echo);
> +	if (data->irqnr < 0) {
> +		dev_err(data->dev, "gpiod_to_irq: %d\n", data->irqnr);
> +		return data->irqnr;
> +	}
> +
> +	ret = devm_request_irq(dev, data->irqnr, srf04_handle_irq,
> +			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			pdev->name, indio_dev);
> +	if (ret < 0) {
> +		dev_err(data->dev, "request_irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	indio_dev->name = "srf04";
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->info = &srf04_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = srf04_chan_spec;
> +	indio_dev->num_channels = ARRAY_SIZE(srf04_chan_spec);
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int srf04_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +
> +	iio_device_unregister(indio_dev);
With nothing else in here, you can use devm_iio_device_register
and drop the remove entirely.
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_srf04_match[] = {
> +	{ .compatible = "devantech,srf04", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, of_srf04_match);
> +
> +static struct platform_driver srf04_driver = {
> +	.probe		= srf04_probe,
> +	.remove		= srf04_remove,
> +	.driver		= {
> +		.name		= "srf04-gpio",
> +		.of_match_table	= of_srf04_match,
> +	},
> +};
> +
> +module_platform_driver(srf04_driver);
> +
> +MODULE_AUTHOR("Andreas Klinger <ak@...klinger.de>");
> +MODULE_DESCRIPTION("SRF04 ultrasonic sensor for distance measuring using GPIOs");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:srf04");
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ