iio: distance: add devantech us ranger srf04
Subsystems (2)
Andreas Klinger
maintainerPATCH 2/2 iio: distance: add devantech us ranger srf04
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 when the measurement should start
- echo: set when the ultrasonic wave is sent out and reset when the echo is recognized; 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.
--- MAINTAINERS | 6 + drivers/iio/proximity/Kconfig | 11 ++ drivers/iio/proximity/Makefile | 1 + drivers/iio/proximity/srf04.c | 269 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 287 insertions(+) create mode 100644 drivers/iio/proximity/srf04.c
MAINTAINERS +6 -0
@@ -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-n176/[email protected]> +L: [email protected] +S: Maintained +F: drivers/iio/proximity/srf*.c + DEVICE COREDUMP (DEV_COREDUMP) M: Johannes Berg <[email protected]> L: [email protected]
drivers/iio/proximity/Kconfig +11 -0
@@ -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
drivers/iio/proximity/Makefile +1 -0
@@ -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
drivers/iio/proximity/srf04.c new file +269 -0
269 lines not on this page · raw message
Signature · 6 lines
2.1.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 2 lines
This patch adds support for the ultrasonic ranger srf04 of devantech.
comments below
Quoted 163 lines
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 when the measurement should start
- echo: set when the ultrasonic wave is sent out and reset when the echo is recognized; needs to be an interrupt inputThe 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 <[email protected]>--- MAINTAINERS | 6 + drivers/iio/proximity/Kconfig | 11 ++ drivers/iio/proximity/Makefile | 1 + drivers/iio/proximity/srf04.c | 269 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 287 insertions(+) create mode 100644 drivers/iio/proximity/srf04.cMAINTAINERS +6
-0@@ -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 <[email protected]> +L: [email protected] +S: Maintained +F: drivers/iio/proximity/srf*.c + DEVICE COREDUMP (DEV_COREDUMP) M: Johannes Berg <[email protected]> L: [email protected]drivers/iio/proximity/Kconfig +11
-0@@ -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_BUFFERdrivers/iio/proximity/Makefile +1
-0@@ -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.odrivers/iio/proximity/srf04.c new file +84
-0@@ -0,0 +1,269 @@ +/* +* SRF04: ultrasonic sensor for distance measuring by using GPIOs +* +* Copyright (c) 2017 Andreas Klinger <[email protected]> +* +* 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 +*/ +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/bitops.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)) { + 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; +u32 distance_mm; + +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 ); + +mutex_unlock (&data->lock); + +/* it cannot take more than 20 ms */ + ret =wait_for_completion_killable_timeout (&data->rising,HZ /50 ); +if (ret <0 ) +return -ETIMEDOUT ;
drivers/iio/proximity/srf04.c line 94
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+ + ret =wait_for_completion_killable_timeout (&data->falling,HZ /50 ); +if (ret <0 ) +return -ETIMEDOUT ; + + ktime_dt =ktime_sub (data->ts_falling, data->ts_rising); + + dt_ns =ktime_to_ns (ktime_dt); +/* +* measuring more than 3 meters is beyond the posibilities of
possibilities, maybe better capabilities
drivers/iio/proximity/srf04.c line 97
Quoted 3 lines
drivers/iio/proximity/srf04.c new file +3
-0+* the sensor +*/ +if (dt_ns >8750000 ) {
magic value?
parenthesis {} not needed
Quoted 14 lines
drivers/iio/proximity/srf04.c new file +14
-0+return -EFAULT ; + } + 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:
drivers/iio/proximity/srf04.c line 121
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+* distance = time / 10^6 * 343 / 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 ; + +dev_info (data->dev,"ns: %llu, dist: %d\n" , dt_ns, distance_mm);
this should be dbg I think; and no space after dev_info
Quoted 44 lines
drivers/iio/proximity/srf04.c new file +44
-0+ +return distance_mm; +} + +static int srf04_read_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *channel,int *val, +int *val2,long mask) +{ +struct srf04_data *data =iio_priv (indio_dev); +int ret; + +if (channel->type !=IIO_DISTANCE ) +return -EINVAL ; + +switch (mask) { +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) |
drivers/iio/proximity/srf04.c line 175
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+ BIT(IIO_CHAN_INFO_SCALE), + }, +}; + +static int srf04_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct srf04_data *data =NULL ; +struct iio_dev *indio_dev; +int ret =0 ;
initialization not needed
Quoted 26 lines
drivers/iio/proximity/srf04.c new file +26
-0+ + 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-gpiod: 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-gpiod: err=%ld\n" , +PTR_ERR (data->gpiod_echo)); +return PTR_ERR (data->gpiod_echo);
drivers/iio/proximity/srf04.c line 211
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+ } + +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" , ret);
ret? should be data->irqnr?
drivers/iio/proximity/srf04.c line 215
Quoted 4 lines
drivers/iio/proximity/srf04.c new file +4
-0+return ret; + } + + ret = request_irq(data->irqnr, srf04_handle_irq,
could use devm_
Quoted 7 lines
drivers/iio/proximity/srf04.c new file +7
-0+IRQF_TRIGGER_RISING |IRQF_TRIGGER_FALLING , + pdev->name, indio_dev); +if (ret <0 ) { +dev_err (data->dev,"request_irq: %d\n" , ret); +return ret; + } +
drivers/iio/proximity/srf04.c line 232
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+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 devm_iio_device_register (dev, indio_dev);
you can't use devm_iio_device_register() if you need/have a _remove()
drivers/iio/proximity/srf04.c line 236
Quoted 4 lines
drivers/iio/proximity/srf04.c new file +4
-0+} + +static int srf04_remove (struct platform_device *pdev) +{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct srf04_data *data = iio_priv(indio_dev); // (but not needed anymore)
drivers/iio/proximity/srf04.c line 243
Quoted 7 lines
drivers/iio/proximity/srf04.c new file +7
-0+struct srf04_data *data =NULL ; +struct iio_dev *indio_dev; + + indio_dev =platform_get_drvdata (pdev); + data =iio_priv (indio_dev); + +free_irq (data->irqnr, indio_dev);
I suggest to use in _probe()devm_request_irq()iio_device_register()
and in _remove() justiio_device_unregister()
Quoted 27 lines
drivers/iio/proximity/srf04.c new file +26
-0+ +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 <[email protected]>" ); +MODULE_DESCRIPTION ("SRF04 ultrasonic sensor for distance measuring using GPIOs" ); +MODULE_LICENSE ("GPL" ); +MODULE_ALIAS ("platform:srf04" );
Signature · 2 lines
Peter Meerwald-Stadler +43-664-2444418 (mobile)
Quoted 1 line · Andreas Klinger
This patch adds support for the ultrasonic ranger srf04 of devantech.
Thanks for the patch. Looks mostly good, a few small comments inline.
Quoted 6 lines
drivers/iio/proximity/srf04.c new file +0
-0@@ -0,0 +1,269 @@
[...]
drivers/iio/proximity/srf04.c line 20
Quoted 2 lines
drivers/iio/proximity/srf04.c new file +2
-0+#include <linux/err.h> +#include <linux/gpio.h>
Your driver is only a GPIO consumer, so the above include should not be necessary.
drivers/iio/proximity/srf04.c line 24
Quoted 4 lines
drivers/iio/proximity/srf04.c new file +4
-0+#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/bitops.h>
As far as I can see nothing from bitops.h is used in this driver.
Quoted 9 lines
drivers/iio/proximity/srf04.c new file +9
-0+#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> +
[...]
drivers/iio/proximity/srf04.c line 70
Quoted 9 lines
drivers/iio/proximity/srf04.c new file +9
-0+static int srf04_read (struct srf04_data *data) +{ +int ret; +ktime_t ktime_dt; +u64 dt_ns; +u32 time_ns; +u32 distance_mm; + +mutex_lock (&data->lock);
Maybe a stupid question, but what does the lock protect?
drivers/iio/proximity/srf04.c line 83
Quoted 13 lines
drivers/iio/proximity/srf04.c new file +13
-0+ +reinit_completion (&data->rising); +reinit_completion (&data->falling); + +gpiod_set_value (data->gpiod_trig,1 ); +udelay (10 ); +gpiod_set_value (data->gpiod_trig,0 ); + +mutex_unlock (&data->lock); + +/* it cannot take more than 20 ms */ + ret =wait_for_completion_killable_timeout (&data->rising,HZ /50 ); +if (ret <0 )
In case of a timeout wait_for_... will return 0. In case of an interrupt (kill event) a negative value. In the later case we should typically propagate the error rather than replacing it.
Quoted 5 lines
drivers/iio/proximity/srf04.c new file +5
-0+return -ETIMEDOUT ; + + ret =wait_for_completion_killable_timeout (&data->falling,HZ /50 ); +if (ret <0 ) +return -ETIMEDOUT ;
drivers/iio/proximity/srf04.c line 98
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+ + ktime_dt =ktime_sub (data->ts_falling, data->ts_rising); + + dt_ns =ktime_to_ns (ktime_dt); +/* +* measuring more than 3 meters is beyond the posibilities of +* the sensor +*/ +if (dt_ns >8750000 ) { +return -EFAULT ;
EFAULT means that "An invalid user space address was specified for an argument". EIO is usually used to indicate that there was a communication issue with the device.
Quoted 13 lines
drivers/iio/proximity/srf04.c new file +13
-0+ } + 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:
drivers/iio/proximity/srf04.c line 121
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+* distance = time / 10^6 * 343 / 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 ; + +dev_info (data->dev,"ns: %llu, dist: %d\n" , dt_ns, distance_mm);
dev_dbg probably.
drivers/iio/proximity/srf04.c line 128
Quoted 7 lines
drivers/iio/proximity/srf04.c new file +7
-0+ +return distance_mm; +} + +static int srf04_read_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *channel,int *val, +int *val2,long mask)
I know lots of drivers use the name 'mask' here, but that is a relict from a long long time ago and the implementation was changed and the old name it is not really appropriate anymore. The recommendation is to use 'info' for new drivers.
Quoted 1 line
+{
[...]
drivers/iio/proximity/srf04.c line 173
Quoted 4 lines
drivers/iio/proximity/srf04.c new file +4
-0+static int srf04_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct srf04_data *data =NULL ;
The NULL initialization is not really necessary. The variable is never used until it is initialized again a few lines below. The compiler will just remove this.
drivers/iio/proximity/srf04.c line 175
Quoted 2 lines
drivers/iio/proximity/srf04.c new file +2
-0+struct iio_dev *indio_dev; +int ret =0 ;
Same here.
Quoted 47 lines
drivers/iio/proximity/srf04.c new file +47
-0+ + 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-gpiod: 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-gpiod: 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" , ret); +return ret; + } + + ret =request_irq (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; + } +
drivers/iio/proximity/srf04.c line 232
Quoted 10 lines
drivers/iio/proximity/srf04.c new file +10
-0+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_chans_spec; + indio_dev->num_channels =ARRAY_SIZE (srf04_chan_spec); + +return devm_iio_device_register (dev, indio_dev);
If this returns an error the interrupt needs to be freed.
drivers/iio/proximity/srf04.c line 241
Quoted 9 lines
drivers/iio/proximity/srf04.c new file +9
-0+} + +static int srf04_remove (struct platform_device *pdev) +{ +struct srf04_data *data =NULL ; +struct iio_dev *indio_dev; + + indio_dev =platform_get_drvdata (pdev); + data =iio_priv (indio_dev);
I'd write this as
struct iio_dev *indio_dev = platform_get_drvdata(pdev); struct srf04_data *data = iio_priv(indio_dev)
Looks a bit cleaner in my opinion.
Quoted 5 lines
drivers/iio/proximity/srf04.c new file +5
-0+ +free_irq (data->irqnr, indio_dev); + +return 0 ; +}
[...]