Re: [PATCH 2/2] iio: proximity: add support for PulsedLight LIDAR

On Sat, Aug 1, 2015 at 2:17 PM, Jonathan Cameron
[off-list ref] wrote:

On 1 August 2015 09:39:11 BST, Vladimir Barinov [off-list ref] wrote:

quoted

Hi Matt,

Find minor comments.

On 01.08.2015 06:58, Matt Ranostay wrote:

quoted

Add support for the PulsedLight LIDAR rangefinder sensor which allows
high speed (over 300Hz) distance measurements using Barker Coding

within

quoted

40 meter range.

Support only tested on the "blue label" rev 2, but may work using

polling

quoted

at low sample frequencies on the original version.

Signed-off-by: Matt Ranostay <redacted>
---
  drivers/iio/proximity/Kconfig  |  13 ++
  drivers/iio/proximity/Makefile |   1 +
  drivers/iio/proximity/lidar.c  | 309

+++++++++++++++++++++++++++++++++++++++++
Add ABI documentation
Documentation/ABI/testing/sysfs-bus-iio-proximity-lidar

Why?  Reading on phone so may have missed something but all ABI is standard.
Covered by top level docs.

Only thing that would be documented that IIO_DISTANCE is in meters.
But seems standard.

quoted

quoted

  3 files changed, 323 insertions(+)
  create mode 100644 drivers/iio/proximity/lidar.c

diff --git a/drivers/iio/proximity/Kconfig

b/drivers/iio/proximity/Kconfig

quoted

index 41a8d8f..8111e11 100644

--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig

@@ -20,6 +20,19 @@ endmenu

  menu "Proximity sensors"

+config LIDAR
+    tristate "PulsedLight LIDAR sensor"
+    select IIO_BUFFER
+    select IIO_TRIGGERED_BUFFER
+    select REGMAP_I2C

You did not use REGMAP in your driver.
Probably it would be better to use this API in your driver since the
chip seems fully SMBUS compatible.

Unless there is a significant gain don't bother. Now if the device also has an spi bus
or the driver does extensive caching it would be worthwhile.

Correctly, none of the the data is cacheable so regmap doesn't make sense here.

quoted

quoted

+    depends on I2C
+    help
+      Say Y to build a driver for PulsedLight LIDAR range finding
+      sensor.
+
+      To compile this driver as a module, choose M here: the
+      module will be called lidar.
+
  config SX9500
     tristate "SX9500 Semtech proximity sensor"
     select IIO_BUFFER

diff --git a/drivers/iio/proximity/Makefile

b/drivers/iio/proximity/Makefile

quoted

index 9818dc5..36a95f3 100644

--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile

@@ -4,4 +4,5 @@

  # When adding new entries keep the list in alphabetical order
  obj-$(CONFIG_AS3935)               += as3935.o
+obj-$(CONFIG_LIDAR)         += lidar.o
  obj-$(CONFIG_SX9500)               += sx9500.o

diff --git a/drivers/iio/proximity/lidar.c

b/drivers/iio/proximity/lidar.c

quoted

new file mode 100644
index 0000000..e56df58

--- /dev/null
+++ b/drivers/iio/proximity/lidar.c

@@ -0,0 +1,309 @@
+/*
+ * lidar.c - Support for PulsedLight LIDAR range finding sensor
+ *
+ * Copyright (C) 2015 Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or

modify

quoted

+ * it under the terms of the GNU General Public License as published

by

quoted

+ * 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.
+ *
+ * TODO: runtime pm, interrupt mode, and signal strength reporting
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>

put it in alphabetic order

quoted

+
+#define LIDAR_REG_CONTROL   0x00
+#define LIDAR_REG_CONTROL_ACQUIRE   BIT(2)

tab aligned

quoted

+
+#define LIDAR_REG_STATUS    0x01
+#define LIDAR_REG_STATUS_RDY        BIT(0)
+
+#define LIDAR_REG_DATA_HBYTE        0x0f
+#define LIDAR_REG_DATA_LBYTE        0x10
+#define LIDAR_REG_DATA_MAX  0xffff
+
+#define LIDAR_DRV_NAME      "lidar"
+
+struct lidar_data {
+    struct mutex lock;
+    struct iio_dev *indio_dev;
+    struct i2c_client *client;
+
+    /* config */
+    int calib_bias;
+
+    u16 buffer[5]; /* 2 byte distance + 8 byte timestamp */
+};
+
+static const struct iio_chan_spec lidar_channels[] = {
+    {
+            .type = IIO_DISTANCE,
+            .info_mask_separate =
+                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_CALIBBIAS),
+            .scan_index = 0,
+            .scan_type = {
+                    .sign = 'u',
+                    .realbits = 16,
+                    .storagebits = 16,
+            },
+    },
+    IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static inline int lidar_read_byte(struct lidar_data *data, int reg)
+{
+    struct i2c_client *client = data->client;
+    int ret;
+
+    ret = i2c_smbus_write_byte(client, reg);
+    if (ret < 0) {
+            dev_err(&client->dev, "cannot write addr value");
+            return ret;
+    }
+
+    ret = i2c_smbus_read_byte(client);
+    if (ret < 0) {
+            dev_err(&client->dev, "cannot read data value");
+            return ret;
+    }
+
+    return ret;
+}
+
+static inline int lidar_write_control(struct lidar_data *data, int

val)

quoted

+{
+    return i2c_smbus_write_byte_data(data->client, LIDAR_REG_CONTROL,

val);

quoted

+}
+
+static int lidar_read_measurement(struct lidar_data *data, u16 *reg)
+{
+    int ret;
+    int val;
+
+    ret = lidar_read_byte(data, LIDAR_REG_DATA_HBYTE);
+    if (ret < 0)
+            return ret;
+    val = ret << 8;
+
+    ret = lidar_read_byte(data, LIDAR_REG_DATA_LBYTE);
+    if (ret < 0)
+            return ret;
+    val |= ret;
+
+    /* correct any possible overflow or underflow */
+    val += data->calib_bias / 10000;
+    if (val < 0)
+            val = 0;
+
+    if (val > LIDAR_REG_DATA_MAX)
+            val = LIDAR_REG_DATA_MAX;
+
+    *reg = val;
+
+    return 0;
+}
+
+static int lidar_get_measurement(struct lidar_data *data, u16 *reg)
+{
+    struct i2c_client *client = data->client;
+    int tries = 10;
+    int ret;
+
+    /* start sample */
+    ret = lidar_write_control(data, LIDAR_REG_CONTROL_ACQUIRE);
+    if (ret < 0) {
+            dev_err(&client->dev, "cannot send start measurement command");
+            return ret;
+    }
+
+    while (tries--) {
+            usleep_range(1000, 2000);
+
+            ret = lidar_read_byte(data, LIDAR_REG_STATUS);
+            if (ret < 0)
+                    break;
+
+            /* sample ready to read */
+            if (!(ret & LIDAR_REG_STATUS_RDY)) {
+                    ret = lidar_read_measurement(data, reg);
+                    break;
+            }
+    }
+
+    return ret;
+}
+
+static int lidar_read_raw(struct iio_dev *indio_dev,
+                      struct iio_chan_spec const *chan,
+                      int *val, int *val2, long mask)
+{
+    struct lidar_data *data = iio_priv(indio_dev);
+    int ret = -EINVAL;
+
+    if (iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW)
+            return -EBUSY;
+
+    mutex_lock(&data->lock);
+
+    switch (mask) {
+    case IIO_CHAN_INFO_RAW: {
+            u16 reg;
+
+            ret = lidar_get_measurement(data, &reg);
+            if (!ret) {
+                    *val = reg / 100;
+                    *val2 = (reg % 100) * 10000;
+                    ret = IIO_VAL_INT_PLUS_MICRO;
+            }
+            break;
+    }
+    case IIO_CHAN_INFO_CALIBBIAS:
+            *val = 0;
+            *val2 = data->calib_bias;
+            ret = IIO_VAL_INT_PLUS_MICRO;
+            break;
+    }
+
+    mutex_unlock(&data->lock);
+
+    return ret;
+}
+
+static int lidar_write_raw(struct iio_dev *indio_dev,
+                       struct iio_chan_spec const *chan,
+                       int val, int val2, long mask)
+{
+    struct lidar_data *data = iio_priv(indio_dev);
+
+    if (mask == IIO_CHAN_INFO_CALIBBIAS) {
+            if (val != 0)
+                    return -EINVAL;
+
+            /* cm increments only */
+            if (val2 % 10000)
+                    return -EINVAL;
+
+            mutex_lock(&data->lock);
+            data->calib_bias = val2;
+            mutex_unlock(&data->lock);
+            return 0;
+    }
+
+    return -EINVAL;
+}
+
+static irqreturn_t lidar_trigger_handler(int irq, void *private)
+{
+    struct iio_poll_func *pf = private;
+    struct iio_dev *indio_dev = pf->indio_dev;
+    struct lidar_data *data = iio_priv(indio_dev);
+    int ret;
+
+    ret = lidar_get_measurement(data, &data->buffer[0]);
+    if (!ret) {
+            iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
+                                               iio_get_time_ns());
+    } else {
+            dev_err(&data->client->dev, "cannot read LIDAR measurement");
+    }
+
+    iio_trigger_notify_done(indio_dev->trig);
+
+    return IRQ_HANDLED;
+}
+
+static struct iio_info lidar_info = {
+    .driver_module = THIS_MODULE,
+    .read_raw = lidar_read_raw,
+    .write_raw = lidar_write_raw,
+};
+
+static int lidar_probe(struct i2c_client *client,
+                   const struct i2c_device_id *id)
+{
+    struct lidar_data *data;
+    struct iio_dev *indio_dev;
+    int ret;
+
+    indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+    if (!indio_dev)
+            return -ENOMEM;
+
+    indio_dev->info = &lidar_info;
+    indio_dev->name = LIDAR_DRV_NAME;
+    indio_dev->channels = lidar_channels;
+    indio_dev->num_channels = ARRAY_SIZE(lidar_channels);
+    indio_dev->modes = INDIO_DIRECT_MODE;
+
+    data = iio_priv(indio_dev);
+    i2c_set_clientdata(client, indio_dev);
+
+    data->client = client;
+    data->indio_dev = indio_dev;
+    mutex_init(&data->lock);
+
+    ret = iio_triggered_buffer_setup(indio_dev, NULL,
+                                     lidar_trigger_handler, NULL);
+    if (ret)
+            return ret;
+
+    ret = iio_device_register(indio_dev);
+    if (ret)
+            goto error_unreg_buffer;
+
+    return 0;
+
+error_unreg_buffer:
+    iio_triggered_buffer_cleanup(indio_dev);
+
+    return ret;
+}
+
+static int lidar_remove(struct i2c_client *client)
+{
+    struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+    iio_device_unregister(indio_dev);
+    iio_triggered_buffer_cleanup(indio_dev);
+
+    return 0;
+}
+
+static const struct i2c_device_id lidar_id[] = {
+    {"lidar", 0},
+    { },
+};
+MODULE_DEVICE_TABLE(i2c, lidar_id);
+
+static struct i2c_driver lidar_driver = {
+    .driver = {
+            .name   = LIDAR_DRV_NAME,
+            .owner  = THIS_MODULE,

the .owner field is not needed since it is overridden in
i2c_register_driver

quoted

+    },
+    .probe          = lidar_probe,
+    .remove         = lidar_remove,
+    .id_table       = lidar_id,
+};
+module_i2c_driver(lidar_driver);
+
+MODULE_AUTHOR("Matt Ranostay [off-list ref]");
+MODULE_DESCRIPTION("PulsedLight LIDAR sensor");
+MODULE_LICENSE("GPL");

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.