* [RFC] iio: srf04: add parallax ping sensors
@ 2019-10-31 12:13 Andreas Klinger
2019-11-03 11:13 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Andreas Klinger @ 2019-10-31 12:13 UTC (permalink / raw)
To: jic23; +Cc: linux-iio
Hi Jonathan,
i added support for parallax ping and laser ping sensors with just one pin
for trigger and echo signal.
After implementing it turned out that this one pin sensor requires a lot of
conditions and special treatment. But the calculation is still the same as
for srf04.
Should it be integrated into the existing srf04 driver or should i create a
new one for those single pin us sensors? Maybe together with a C module for
the calculations they have in common.
Regards,
Andreas
---
drivers/iio/proximity/srf04.c | 151 ++++++++++++++++++++++++++++++++++--------
1 file changed, 122 insertions(+), 29 deletions(-)
diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
index 01eb8cc63076..8bcc77a45965 100644
--- a/drivers/iio/proximity/srf04.c
+++ b/drivers/iio/proximity/srf04.c
@@ -49,7 +49,12 @@
#include <linux/iio/sysfs.h>
struct srf04_cfg {
- unsigned long trigger_pulse_us;
+ unsigned long trigger_pulse_us; /* length of trigger pulse */
+ int single_pin_mode; /* just one pin for trigger */
+ /* and echo */
+ int laserping_error; /* support error code in */
+ /* pulse width of laser */
+ /* ping sensors */
};
struct srf04_data {
@@ -67,10 +72,26 @@ struct srf04_data {
static const struct srf04_cfg srf04_cfg = {
.trigger_pulse_us = 10,
+ .single_pin_mode = 0,
+ .laserping_error = 0,
};
static const struct srf04_cfg mb_lv_cfg = {
.trigger_pulse_us = 20,
+ .single_pin_mode = 0,
+ .laserping_error = 0,
+};
+
+static const struct srf04_cfg pa_ping_cfg = {
+ .trigger_pulse_us = 5,
+ .single_pin_mode = 1,
+ .laserping_error = 0,
+};
+
+static const struct srf04_cfg pa_laser_ping_cfg = {
+ .trigger_pulse_us = 5,
+ .single_pin_mode = 1,
+ .laserping_error = 1,
};
static irqreturn_t srf04_handle_irq(int irq, void *dev_id)
@@ -96,6 +117,9 @@ static int srf04_read(struct srf04_data *data)
ktime_t ktime_dt;
u64 dt_ns;
u32 time_ns, distance_mm;
+ struct platform_device *pdev = container_of(data->dev,
+ struct platform_device, dev);
+ struct iio_dev *indio_dev = iio_priv_to_dev(data);
/*
* just one read-echo-cycle can take place at a time
@@ -110,30 +134,58 @@ static int srf04_read(struct srf04_data *data)
udelay(data->cfg->trigger_pulse_us);
gpiod_set_value(data->gpiod_trig, 0);
+ if (data->cfg->single_pin_mode) {
+ ret = gpiod_direction_input(data->gpiod_trig);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ 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(data->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;
+ }
+ }
+
/* 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);
- return ret;
- } else if (ret == 0) {
- mutex_unlock(&data->lock);
- return -ETIMEDOUT;
+ if (ret < 0)
+ goto err_reset_direction;
+ else if (ret == 0) {
+ ret = -ETIMEDOUT;
+ goto err_reset_direction;
}
/* 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;
- } else if (ret == 0) {
- mutex_unlock(&data->lock);
- return -ETIMEDOUT;
+ if (ret < 0)
+ goto err_reset_direction;
+ else if (ret == 0) {
+ ret = -ETIMEDOUT;
+ goto err_reset_direction;
}
ktime_dt = ktime_sub(data->ts_falling, data->ts_rising);
mutex_unlock(&data->lock);
+ if (data->cfg->single_pin_mode) {
+ free_irq(data->irqnr, indio_dev);
+
+ ret = gpiod_direction_output(data->gpiod_trig, GPIOD_OUT_LOW);
+ if (ret < 0)
+ return ret;
+ }
+
dt_ns = ktime_to_ns(ktime_dt);
/*
* measuring more than 6,45 meters is beyond the capabilities of
@@ -154,6 +206,25 @@ static int srf04_read(struct srf04_data *data)
time_ns = dt_ns;
/*
+ * read error code of laser ping sensor and give users chance to
+ * figure out error by using dynamic debuggging
+ */
+ if (data->cfg->laserping_error) {
+ if ((time_ns >= 12500000) && (time_ns < 13500000)) {
+ dev_dbg(data->dev, "target too close or to far\n");
+ return -EIO;
+ }
+ if ((time_ns >= 13500000) && (time_ns < 14500000)) {
+ dev_dbg(data->dev, "internal sensor error\n");
+ return -EIO;
+ }
+ if ((time_ns >= 14500000) && (time_ns < 15500000)) {
+ dev_dbg(data->dev, "internal sensor timeout\n");
+ return -EIO;
+ }
+ }
+
+ /*
* the speed as function of the temperature is approximately:
*
* speed = 331,5 + 0,6 * Temp
@@ -176,6 +247,16 @@ static int srf04_read(struct srf04_data *data)
distance_mm = time_ns * 106 / 617176;
return distance_mm;
+
+err_reset_direction:
+ mutex_unlock(&data->lock);
+ if (data->cfg->single_pin_mode) {
+ free_irq(data->irqnr, indio_dev);
+
+ if (gpiod_direction_output(data->gpiod_trig, GPIOD_OUT_LOW))
+ dev_dbg(data->dev, "error in gpiod_direction_output\n");
+ }
+ return ret;
}
static int srf04_read_raw(struct iio_dev *indio_dev,
@@ -228,6 +309,8 @@ static const struct of_device_id of_srf04_match[] = {
{ .compatible = "maxbotix,mb1020", .data = &mb_lv_cfg},
{ .compatible = "maxbotix,mb1030", .data = &mb_lv_cfg},
{ .compatible = "maxbotix,mb1040", .data = &mb_lv_cfg},
+ { .compatible = "parallax,ping", .data = &pa_ping_cfg},
+ { .compatible = "parallax,laserping", .data = &pa_ping_cfg},
{},
};
@@ -261,11 +344,19 @@ static int srf04_probe(struct platform_device *pdev)
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",
+ /*
+ * some sensors come along with just one pin for triggering and the
+ * echo
+ */
+ if (data->cfg->single_pin_mode)
+ data->gpiod_echo = data->gpiod_trig;
+ else {
+ 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);
+ return PTR_ERR(data->gpiod_echo);
+ }
}
if (gpiod_cansleep(data->gpiod_echo)) {
@@ -273,18 +364,20 @@ static int srf04_probe(struct platform_device *pdev)
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;
+ if (!data->cfg->single_pin_mode) {
+ 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);
--
2.11.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC] iio: srf04: add parallax ping sensors
2019-10-31 12:13 [RFC] iio: srf04: add parallax ping sensors Andreas Klinger
@ 2019-11-03 11:13 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2019-11-03 11:13 UTC (permalink / raw)
To: Andreas Klinger; +Cc: linux-iio
On Thu, 31 Oct 2019 13:13:19 +0100
Andreas Klinger <ak@it-klinger.de> wrote:
> Hi Jonathan,
>
> i added support for parallax ping and laser ping sensors with just one pin
> for trigger and echo signal.
>
> After implementing it turned out that this one pin sensor requires a lot of
> conditions and special treatment. But the calculation is still the same as
> for srf04.
>
> Should it be integrated into the existing srf04 driver or should i create a
> new one for those single pin us sensors? Maybe together with a C module for
> the calculations they have in common.
Ultimately it's your choice but I agree this seems like a lot of extra
code on a very small driver. Might be more maintainable to separate them.
Looking at the code the calculation code seems fairly small. I'd just cut
and paste it to avoid the fiddly nature of a shared module.
Jonathan
>
> Regards,
>
> Andreas
>
> ---
> drivers/iio/proximity/srf04.c | 151 ++++++++++++++++++++++++++++++++++--------
> 1 file changed, 122 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> index 01eb8cc63076..8bcc77a45965 100644
> --- a/drivers/iio/proximity/srf04.c
> +++ b/drivers/iio/proximity/srf04.c
> @@ -49,7 +49,12 @@
> #include <linux/iio/sysfs.h>
>
> struct srf04_cfg {
> - unsigned long trigger_pulse_us;
> + unsigned long trigger_pulse_us; /* length of trigger pulse */
> + int single_pin_mode; /* just one pin for trigger */
> + /* and echo */
> + int laserping_error; /* support error code in */
> + /* pulse width of laser */
> + /* ping sensors */
> };
>
> struct srf04_data {
> @@ -67,10 +72,26 @@ struct srf04_data {
>
> static const struct srf04_cfg srf04_cfg = {
> .trigger_pulse_us = 10,
> + .single_pin_mode = 0,
> + .laserping_error = 0,
> };
>
> static const struct srf04_cfg mb_lv_cfg = {
> .trigger_pulse_us = 20,
> + .single_pin_mode = 0,
> + .laserping_error = 0,
> +};
> +
> +static const struct srf04_cfg pa_ping_cfg = {
> + .trigger_pulse_us = 5,
> + .single_pin_mode = 1,
> + .laserping_error = 0,
> +};
> +
> +static const struct srf04_cfg pa_laser_ping_cfg = {
> + .trigger_pulse_us = 5,
> + .single_pin_mode = 1,
> + .laserping_error = 1,
> };
>
> static irqreturn_t srf04_handle_irq(int irq, void *dev_id)
> @@ -96,6 +117,9 @@ static int srf04_read(struct srf04_data *data)
> ktime_t ktime_dt;
> u64 dt_ns;
> u32 time_ns, distance_mm;
> + struct platform_device *pdev = container_of(data->dev,
> + struct platform_device, dev);
> + struct iio_dev *indio_dev = iio_priv_to_dev(data);
>
> /*
> * just one read-echo-cycle can take place at a time
> @@ -110,30 +134,58 @@ static int srf04_read(struct srf04_data *data)
> udelay(data->cfg->trigger_pulse_us);
> gpiod_set_value(data->gpiod_trig, 0);
>
> + if (data->cfg->single_pin_mode) {
> + ret = gpiod_direction_input(data->gpiod_trig);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + 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(data->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;
> + }
> + }
> +
> /* 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);
> - return ret;
> - } else if (ret == 0) {
> - mutex_unlock(&data->lock);
> - return -ETIMEDOUT;
> + if (ret < 0)
> + goto err_reset_direction;
> + else if (ret == 0) {
> + ret = -ETIMEDOUT;
> + goto err_reset_direction;
> }
>
> /* 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;
> - } else if (ret == 0) {
> - mutex_unlock(&data->lock);
> - return -ETIMEDOUT;
> + if (ret < 0)
> + goto err_reset_direction;
> + else if (ret == 0) {
> + ret = -ETIMEDOUT;
> + goto err_reset_direction;
> }
>
> ktime_dt = ktime_sub(data->ts_falling, data->ts_rising);
>
> mutex_unlock(&data->lock);
>
> + if (data->cfg->single_pin_mode) {
> + free_irq(data->irqnr, indio_dev);
> +
> + ret = gpiod_direction_output(data->gpiod_trig, GPIOD_OUT_LOW);
> + if (ret < 0)
> + return ret;
> + }
> +
> dt_ns = ktime_to_ns(ktime_dt);
> /*
> * measuring more than 6,45 meters is beyond the capabilities of
> @@ -154,6 +206,25 @@ static int srf04_read(struct srf04_data *data)
> time_ns = dt_ns;
>
> /*
> + * read error code of laser ping sensor and give users chance to
> + * figure out error by using dynamic debuggging
> + */
> + if (data->cfg->laserping_error) {
> + if ((time_ns >= 12500000) && (time_ns < 13500000)) {
> + dev_dbg(data->dev, "target too close or to far\n");
> + return -EIO;
> + }
> + if ((time_ns >= 13500000) && (time_ns < 14500000)) {
> + dev_dbg(data->dev, "internal sensor error\n");
> + return -EIO;
> + }
> + if ((time_ns >= 14500000) && (time_ns < 15500000)) {
> + dev_dbg(data->dev, "internal sensor timeout\n");
> + return -EIO;
> + }
> + }
> +
> + /*
> * the speed as function of the temperature is approximately:
> *
> * speed = 331,5 + 0,6 * Temp
> @@ -176,6 +247,16 @@ static int srf04_read(struct srf04_data *data)
> distance_mm = time_ns * 106 / 617176;
>
> return distance_mm;
> +
> +err_reset_direction:
> + mutex_unlock(&data->lock);
> + if (data->cfg->single_pin_mode) {
> + free_irq(data->irqnr, indio_dev);
> +
> + if (gpiod_direction_output(data->gpiod_trig, GPIOD_OUT_LOW))
> + dev_dbg(data->dev, "error in gpiod_direction_output\n");
> + }
> + return ret;
> }
>
> static int srf04_read_raw(struct iio_dev *indio_dev,
> @@ -228,6 +309,8 @@ static const struct of_device_id of_srf04_match[] = {
> { .compatible = "maxbotix,mb1020", .data = &mb_lv_cfg},
> { .compatible = "maxbotix,mb1030", .data = &mb_lv_cfg},
> { .compatible = "maxbotix,mb1040", .data = &mb_lv_cfg},
> + { .compatible = "parallax,ping", .data = &pa_ping_cfg},
> + { .compatible = "parallax,laserping", .data = &pa_ping_cfg},
> {},
> };
>
> @@ -261,11 +344,19 @@ static int srf04_probe(struct platform_device *pdev)
> 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",
> + /*
> + * some sensors come along with just one pin for triggering and the
> + * echo
> + */
> + if (data->cfg->single_pin_mode)
> + data->gpiod_echo = data->gpiod_trig;
> + else {
> + 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);
> + return PTR_ERR(data->gpiod_echo);
> + }
> }
>
> if (gpiod_cansleep(data->gpiod_echo)) {
> @@ -273,18 +364,20 @@ static int srf04_probe(struct platform_device *pdev)
> 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;
> + if (!data->cfg->single_pin_mode) {
> + 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);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-11-03 11:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 12:13 [RFC] iio: srf04: add parallax ping sensors Andreas Klinger
2019-11-03 11:13 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).