mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] iio: proximity: Add Benewake TF-Luna LiDAR driver
@ 2026-10-19 18:33 Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add Benewake Vidhu Sarwal via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Vidhu Sarwal via B4 Relay @ 2026-10-19 18:33 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vidhu Sarwal,
	Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: devicetree, linux-kernel, linux-iio, Shuah Khan

This series adds support for the Benewake TF-Luna, a single point ToF
 LiDAR distance sensor with a range of 0.2 m to 8 m. The
sensor uses either UART or I2C, selected by a strap pin at power-up.
This series covers the I2C interface only.

Patch 1 adds the vendor prefix, patch 2 adds the binding and patch 3 adds the
driver, which reports distance using an IIO_DISTANCE channel.

The driver keeps the sensor in trigger mode and starts a measurement
per read. The manual warns that reads in continuous mode can race with
the sensor updating its result registers unless they are synchronised
to the data ready output, which the driver does not use yet.

Two issues I faced with the hardware that the manual does not mention:
- The sensor silently drops a register write that arrives right after
  another one, so probe reads the mode back after setting it.
- The boot time is not documented. I was able to measure 350ms after a
  reboot before the registers read back correctly, so probe waits
  500 ms after enabling the supply, keeping some buffer.

Later on I plan to add buffered caputure using data ready interrupt and 
runtime PM in a follow up series. 

This was tested on hardware using Raspberry Pi 4 and with firmware v3.6.0
built out of tree on the 6.18 RPi kernel both via new_device as well as a 
DT Overlay.

---
Vidhu Sarwal (3):
      dt-bindings: vendor-prefixes: Add Benewake
      dt-bindings: iio: proximity: Add Benewake TF-Luna
      iio: proximity: Add Benewake TF-Luna LiDAR driver

 .../bindings/iio/proximity/benewake,tf-luna.yaml   |  58 +++++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 MAINTAINERS                                        |   7 +
 drivers/iio/proximity/Kconfig                      |  11 +
 drivers/iio/proximity/Makefile                     |   1 +
 drivers/iio/proximity/tf-luna.c                    | 254 +++++++++++++++++++++
 6 files changed, 333 insertions(+)
---
base-commit: 43ffda3c7f8456bbe641ae8249a077a361b97880
change-id: 20261019-tf-luna-e2685e0570d5

Best regards,
--  
Vidhu Sarwal <vidhu.linux@gmail.com>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] dt-bindings: vendor-prefixes: Add Benewake
  2026-10-19 18:33 [PATCH 0/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
@ 2026-10-19 18:33 ` Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 2/3] dt-bindings: iio: proximity: Add Benewake TF-Luna Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
  2 siblings, 0 replies; 7+ messages in thread
From: Vidhu Sarwal via B4 Relay @ 2026-10-19 18:33 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vidhu Sarwal,
	Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: devicetree, linux-kernel, linux-iio, Shuah Khan

From: Vidhu Sarwal <vidhu.linux@gmail.com>

Add vendor prefix for Benewake (Beijing) Co., Ltd., a manufacturer of
LiDAR distance sensors.
https://en.benewake.com/

Signed-off-by: Vidhu Sarwal <vidhu.linux@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 0ec4e481c64b..7bc9c071913b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -245,6 +245,8 @@ patternProperties:
     description: BeagleBoard.org Foundation
   "^belling,.*":
     description: Shanghai Belling Co., Ltd.
+  "^benewake,.*":
+    description: Benewake (Beijing) Co., Ltd.
   "^bestar,.*":
     description: Shenzhen Bestar Electronic Technology Co., Ltd.
   "^bhf,.*":

-- 
2.53.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] dt-bindings: iio: proximity: Add Benewake TF-Luna
  2026-10-19 18:33 [PATCH 0/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add Benewake Vidhu Sarwal via B4 Relay
@ 2026-10-19 18:33 ` Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
  2 siblings, 0 replies; 7+ messages in thread
From: Vidhu Sarwal via B4 Relay @ 2026-10-19 18:33 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vidhu Sarwal,
	Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: devicetree, linux-kernel, linux-iio, Shuah Khan

From: Vidhu Sarwal <vidhu.linux@gmail.com>

Add binding for the Benewake TF-Luna, a single point ToF
LiDAR distance sensor with a range of 0.2 m to 8 m.

The sensor talks either UART or I2C, selected by the level of pin 5 at
power-up. This binding covers the I2C interface. Pin 6 is a data ready
output and is described as an interrupt.

Signed-off-by: Vidhu Sarwal <vidhu.linux@gmail.com>
---
 .../bindings/iio/proximity/benewake,tf-luna.yaml   | 58 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/proximity/benewake,tf-luna.yaml b/Documentation/devicetree/bindings/iio/proximity/benewake,tf-luna.yaml
new file mode 100644
index 000000000000..cfc3dff58013
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/benewake,tf-luna.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/benewake,tf-luna.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Benewake TF-Luna LiDAR distance sensor
+
+maintainers:
+  - Vidhu Sarwal <vidhu.linux@gmail.com>
+
+description: |
+  Single-point time-of-flight LiDAR ranging sensor with a range of 0.2 m to
+  8 m. The sensor talks either UART or I2C, selected at power-up by the level
+  of pin 5. Pin 5 must be tied to ground for the I2C interface described here.
+
+  Datasheet: https://en.benewake.com/uploadfiles/2025/04/20250430174501874.pdf
+
+properties:
+  compatible:
+    const: benewake,tf-luna
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Data ready output on pin 6. Goes high when a new measurement is
+      available and goes low again once any register has been read.
+    maxItems: 1
+
+  vcc-supply:
+    description: Power supply, 3.7 V to 5.2 V.
+
+required:
+  - compatible
+  - reg
+  - vcc-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        proximity@10 {
+            compatible = "benewake,tf-luna";
+            reg = <0x10>;
+            vcc-supply = <&reg_5v>;
+            interrupt-parent = <&gpio>;
+            interrupts = <17 IRQ_TYPE_EDGE_RISING>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 52024397dc78..24fa7cdf6a57 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4662,6 +4662,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/luisbg/linux-befs.git
 F:	Documentation/filesystems/befs.rst
 F:	fs/befs/
 
+BENEWAKE TF-LUNA LIDAR IIO DRIVER
+M:	Vidhu Sarwal <vidhu.linux@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/proximity/benewake,tf-luna.yaml
+
 BFQ I/O SCHEDULER
 M:	Yu Kuai <yukuai@fygo.io>
 L:	linux-block@vger.kernel.org

-- 
2.53.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver
  2026-10-19 18:33 [PATCH 0/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add Benewake Vidhu Sarwal via B4 Relay
  2026-10-19 18:33 ` [PATCH 2/3] dt-bindings: iio: proximity: Add Benewake TF-Luna Vidhu Sarwal via B4 Relay
@ 2026-10-19 18:33 ` Vidhu Sarwal via B4 Relay
  2026-10-20  7:23   ` Joshua Crofts
  2026-10-20  8:10   ` Andy Shevchenko
  2 siblings, 2 replies; 7+ messages in thread
From: Vidhu Sarwal via B4 Relay @ 2026-10-19 18:33 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vidhu Sarwal,
	Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko
  Cc: devicetree, linux-kernel, linux-iio, Shuah Khan

From: Vidhu Sarwal <vidhu.linux@gmail.com>

Add a driver for the Benewake TF-Luna, a single-point time-of-flight
LiDAR distance sensor with a range of 0.2 m to 8 m, connected over I2C.

The sensor is run in trigger mode. Each read starts one measurement and
waits for the timestamp register to change before reading the result.
In continuous mode a read can catch the sensor in the middle of updating
its result registers unless it is synchronised to the data ready output,
which this driver does not use yet.

When the signal strength is too low the sensor reports a dummy distance
of 0, so such readings, as well as overexposed ones, return -ENODATA
instead of a distance.

The datasheet is available at:
https://en.benewake.com/uploadfiles/2025/04/20250430174501874.pdf

Signed-off-by: Vidhu Sarwal <vidhu.linux@gmail.com>
---
 MAINTAINERS                     |   1 +
 drivers/iio/proximity/Kconfig   |  11 ++
 drivers/iio/proximity/Makefile  |   1 +
 drivers/iio/proximity/tf-luna.c | 254 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 267 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 24fa7cdf6a57..20e43a003bed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4667,6 +4667,7 @@ M:	Vidhu Sarwal <vidhu.linux@gmail.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/proximity/benewake,tf-luna.yaml
+F:	drivers/iio/proximity/tf-luna.c
 
 BFQ I/O SCHEDULER
 M:	Yu Kuai <yukuai@fygo.io>
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index f3cec799df67..d16458ea4b0f 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -222,6 +222,17 @@ config SRF08
 	  To compile this driver as a module, choose M here: the
 	  module will be called srf08.
 
+config TF_LUNA
+	tristate "Benewake TF-Luna LiDAR distance sensor"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y here to build a driver for the Benewake TF-Luna LiDAR
+	  distance sensor connected over I2C.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called tf-luna.
+
 config VCNL3020
 	tristate "VCNL3020 proximity sensor"
 	select REGMAP_I2C
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 8a65e42fbea0..43741115a921 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SX9324)		+= sx9324.o
 obj-$(CONFIG_SX9360)		+= sx9360.o
 obj-$(CONFIG_SX_COMMON) 	+= sx_common.o
 obj-$(CONFIG_SX9500)		+= sx9500.o
+obj-$(CONFIG_TF_LUNA)		+= tf-luna.o
 obj-$(CONFIG_VCNL3020)		+= vcnl3020.o
 obj-$(CONFIG_VCNL36829)		+= vcnl36829.o
 obj-$(CONFIG_VL53L0X_I2C)	+= vl53l0x-i2c.o
diff --git a/drivers/iio/proximity/tf-luna.c b/drivers/iio/proximity/tf-luna.c
new file mode 100644
index 000000000000..b2f6e664b614
--- /dev/null
+++ b/drivers/iio/proximity/tf-luna.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Benewake TF-Luna LiDAR distance sensor
+ *
+ * Copyright (C) 2026 Vidhu Sarwal <vidhu.linux@gmail.com>
+ *
+ * Datasheet: https://en.benewake.com/uploadfiles/2025/04/20250430174501874.pdf
+ *
+ * The I2C register map is in appendix III of the TF-Luna product manual
+ * (SJ-PM-TF-Luna A05). Default 7-bit I2C address is 0x10.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+#include <linux/time.h>
+#include <linux/types.h>
+
+#include <asm/byteorder.h>
+
+#include <linux/iio/iio.h>
+
+#define TF_LUNA_REG_DIST		0x00
+#define TF_LUNA_REG_TICK		0x06
+#define TF_LUNA_REG_VERSION		0x0A
+#define TF_LUNA_REG_MODE		0x23
+#define TF_LUNA_REG_TRIG_ONE_SHOT	0x24
+#define TF_LUNA_REG_SIGNATURE		0x3C
+
+#define TF_LUNA_MODE_TRIGGER		0x01
+#define TF_LUNA_TRIG_ONE_SHOT		0x01
+
+/* Below this the sensor reports a dummy distance of 0 */
+#define TF_LUNA_AMP_MIN			100
+#define TF_LUNA_AMP_OVEREXPOSED		0xFFFF
+
+/* A measurement takes 10-20 ms in trigger mode */
+#define TF_LUNA_POLL_US			(2 * USEC_PER_MSEC)
+#define TF_LUNA_TIMEOUT_US		(100 * USEC_PER_MSEC)
+
+struct tf_luna_data {
+	struct regmap *regmap;
+	/* Serialises the trigger, poll and read of a single measurement */
+	struct mutex lock;
+};
+
+static const struct regmap_config tf_luna_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = TF_LUNA_REG_SIGNATURE + 3,
+};
+
+static int tf_luna_read_tick(struct tf_luna_data *data, u16 *tick)
+{
+	__le16 val;
+	int ret;
+
+	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_TICK, &val, sizeof(val));
+	if (ret)
+		return ret;
+
+	*tick = le16_to_cpu(val);
+	return 0;
+}
+
+static int tf_luna_measure(struct tf_luna_data *data, int *dist)
+{
+	__le16 buf[2];	/* distance in cm, signal strength */
+	u16 old_tick, tick, amp;
+	int ret, err;
+
+	guard(mutex)(&data->lock);
+
+	ret = tf_luna_read_tick(data, &old_tick);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(data->regmap, TF_LUNA_REG_TRIG_ONE_SHOT,
+			   TF_LUNA_TRIG_ONE_SHOT);
+	if (ret)
+		return ret;
+
+	/* The tick register is a timestamp, it changes with each measurement */
+	ret = read_poll_timeout(tf_luna_read_tick, err, err || tick != old_tick,
+				TF_LUNA_POLL_US, TF_LUNA_TIMEOUT_US, true,
+				data, &tick);
+	if (ret)
+		return ret;
+	if (err)
+		return err;
+
+	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_DIST, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	amp = le16_to_cpu(buf[1]);
+	if (amp < TF_LUNA_AMP_MIN || amp == TF_LUNA_AMP_OVEREXPOSED)
+		return -ENODATA;
+
+	*dist = le16_to_cpu(buf[0]);
+	return 0;
+}
+
+static int tf_luna_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct tf_luna_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = tf_luna_measure(data, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* Distance is in cm */
+		*val = 0;
+		*val2 = 10000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info tf_luna_info = {
+	.read_raw = tf_luna_read_raw,
+};
+
+static const struct iio_chan_spec tf_luna_channels[] = {
+	{
+		.type = IIO_DISTANCE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static int tf_luna_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct tf_luna_data *data;
+	struct iio_dev *indio_dev;
+	unsigned int mode;
+	u8 signature[4];
+	u8 version[3];
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
+
+	data->regmap = devm_regmap_init_i2c(client, &tf_luna_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "Failed to initialize regmap\n");
+
+	ret = devm_regulator_get_enable(dev, "vcc");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable vcc regulator\n");
+
+	/* Boot time is not documented, about 350 ms was measured */
+	fsleep(500 * USEC_PER_MSEC);
+
+	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_SIGNATURE, signature,
+			       sizeof(signature));
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read signature\n");
+
+	if (memcmp(signature, "LUNA", sizeof(signature)))
+		dev_info(dev, "Unknown signature %*ph, continuing\n",
+			 (int)sizeof(signature), signature);
+
+	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_VERSION, version,
+			       sizeof(version));
+	if (ret)
+		return ret;
+
+	dev_dbg(dev, "Firmware version %u.%u.%u\n",
+		version[2], version[1], version[0]);
+
+	/*
+	 * In continuous mode a read can catch the sensor updating its result
+	 * registers unless it is synchronised to the data ready output. Use
+	 * trigger mode and start each measurement on demand instead.
+	 */
+	ret = regmap_write(data->regmap, TF_LUNA_REG_MODE, TF_LUNA_MODE_TRIGGER);
+	if (ret)
+		return ret;
+
+	/*
+	 * The sensor silently drops writes it is not ready for, so check that
+	 * the mode was set. A write takes up to 100 ms to read back.
+	 */
+	fsleep(100 * USEC_PER_MSEC);
+
+	ret = regmap_read(data->regmap, TF_LUNA_REG_MODE, &mode);
+	if (ret)
+		return ret;
+
+	if (mode != TF_LUNA_MODE_TRIGGER)
+		return dev_err_probe(dev, -EIO, "Failed to set trigger mode\n");
+
+	indio_dev->name = "tf-luna";
+	indio_dev->info = &tf_luna_info;
+	indio_dev->channels = tf_luna_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tf_luna_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct i2c_device_id tf_luna_id[] = {
+	{ .name = "tf-luna" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tf_luna_id);
+
+static const struct of_device_id tf_luna_of_match[] = {
+	{ .compatible = "benewake,tf-luna" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tf_luna_of_match);
+
+static struct i2c_driver tf_luna_driver = {
+	.driver = {
+		.name = "tf-luna",
+		.of_match_table = tf_luna_of_match,
+	},
+	.probe = tf_luna_probe,
+	.id_table = tf_luna_id,
+};
+module_i2c_driver(tf_luna_driver);
+
+MODULE_AUTHOR("Vidhu Sarwal <vidhu.linux@gmail.com>");
+MODULE_DESCRIPTION("Benewake TF-Luna LiDAR distance sensor driver");
+MODULE_LICENSE("GPL");

-- 
2.53.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver
  2026-10-19 18:33 ` [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
@ 2026-10-20  7:23   ` Joshua Crofts
  2026-10-20  7:42     ` Andy Shevchenko
  2026-10-20  8:10   ` Andy Shevchenko
  1 sibling, 1 reply; 7+ messages in thread
From: Joshua Crofts @ 2026-10-20  7:23 UTC (permalink / raw)
  To: Vidhu Sarwal via B4 Relay
  Cc: vidhu.linux, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, devicetree, linux-kernel, linux-iio, Shuah Khan

On Tue, 20 Oct 2026 00:03:43 +0530
Vidhu Sarwal via B4 Relay <devnull+vidhu.linux.gmail.com@kernel.org> wrote:

> From: Vidhu Sarwal <vidhu.linux@gmail.com>
> 
> Add a driver for the Benewake TF-Luna, a single-point time-of-flight
> LiDAR distance sensor with a range of 0.2 m to 8 m, connected over I2C.
> 
> The sensor is run in trigger mode. Each read starts one measurement and
> waits for the timestamp register to change before reading the result.
> In continuous mode a read can catch the sensor in the middle of updating
> its result registers unless it is synchronised to the data ready output,
> which this driver does not use yet.
> 
> When the signal strength is too low the sensor reports a dummy distance
> of 0, so such readings, as well as overexposed ones, return -ENODATA
> instead of a distance.
> 
> The datasheet is available at:
> https://en.benewake.com/uploadfiles/2025/04/20250430174501874.pdf
> 
> Signed-off-by: Vidhu Sarwal <vidhu.linux@gmail.com>
> ---

Hi Vidhu,

very good for a v1, even Sashiko reported no issues. Minor style issues from
me.

Josh

> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/iopoll.h>
> +#include <linux/mod_devicetable.h>

Remove this, this is included by i2c.h

> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/string.h>
> +#include <linux/time.h>
> +#include <linux/types.h>
> +
> +#include <asm/byteorder.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define TF_LUNA_REG_DIST		0x00
> +#define TF_LUNA_REG_TICK		0x06
> +#define TF_LUNA_REG_VERSION		0x0A
> +#define TF_LUNA_REG_MODE		0x23
> +#define TF_LUNA_REG_TRIG_ONE_SHOT	0x24
> +#define TF_LUNA_REG_SIGNATURE		0x3C
> +
> +#define TF_LUNA_MODE_TRIGGER		0x01
> +#define TF_LUNA_TRIG_ONE_SHOT		0x01
> +
> +/* Below this the sensor reports a dummy distance of 0 */
> +#define TF_LUNA_AMP_MIN			100
> +#define TF_LUNA_AMP_OVEREXPOSED		0xFFFF
> +
> +/* A measurement takes 10-20 ms in trigger mode */
> +#define TF_LUNA_POLL_US			(2 * USEC_PER_MSEC)
> +#define TF_LUNA_TIMEOUT_US		(100 * USEC_PER_MSEC)

uS is better instead of US.

> +
> +struct tf_luna_data {
> +	struct regmap *regmap;
> +	/* Serialises the trigger, poll and read of a single measurement */
> +	struct mutex lock;
> +};
> +
> +static const struct regmap_config tf_luna_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = TF_LUNA_REG_SIGNATURE + 3,
> +};
> +
> +static int tf_luna_read_tick(struct tf_luna_data *data, u16 *tick)
> +{
> +	__le16 val;
> +	int ret;
> +
> +	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_TICK, &val, sizeof(val));
> +	if (ret)
> +		return ret;
> +
> +	*tick = le16_to_cpu(val);
> +	return 0;
> +}
> +
> +static int tf_luna_measure(struct tf_luna_data *data, int *dist)
> +{
> +	__le16 buf[2];	/* distance in cm, signal strength */
> +	u16 old_tick, tick, amp;
> +	int ret, err;
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = tf_luna_read_tick(data, &old_tick);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(data->regmap, TF_LUNA_REG_TRIG_ONE_SHOT,
> +			   TF_LUNA_TRIG_ONE_SHOT);

Why not have a local variable for regmap? It would allow the previous call
to fit on one line.

...

> +static int tf_luna_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct tf_luna_data *data;
> +	struct iio_dev *indio_dev;
> +	unsigned int mode;
> +	u8 signature[4];
> +	u8 version[3];
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +
> +	ret = devm_mutex_init(dev, &data->lock);
> +	if (ret)
> +		return ret;
> +
> +	data->regmap = devm_regmap_init_i2c(client, &tf_luna_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return dev_err_probe(dev, PTR_ERR(data->regmap),
> +				     "Failed to initialize regmap\n");
> +
> +	ret = devm_regulator_get_enable(dev, "vcc");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable vcc regulator\n");
> +
> +	/* Boot time is not documented, about 350 ms was measured */
> +	fsleep(500 * USEC_PER_MSEC);

msleep() is fine here, saves you the x * USEC_PER_MSEC format.

-- 
Kind regards,
Joshua Crofts

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver
  2026-10-20  7:23   ` Joshua Crofts
@ 2026-10-20  7:42     ` Andy Shevchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-10-20  7:42 UTC (permalink / raw)
  To: Joshua Crofts
  Cc: Vidhu Sarwal via B4 Relay, vidhu.linux, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
	David Lechner, Nuno Sá,
	Andy Shevchenko, devicetree, linux-kernel, linux-iio, Shuah Khan

On Tue, Oct 20, 2026 at 09:23:16AM +0200, Joshua Crofts wrote:
> On Tue, 20 Oct 2026 00:03:43 +0530
> Vidhu Sarwal via B4 Relay <devnull+vidhu.linux.gmail.com@kernel.org> wrote:

...

> > When the signal strength is too low the sensor reports a dummy distance
> > of 0, so such readings, as well as overexposed ones, return -ENODATA
> > instead of a distance.
> > 
> > The datasheet is available at:
> > https://en.benewake.com/uploadfiles/2025/04/20250430174501874.pdf

Make it Datasheet tag.

> > Signed-off-by: Vidhu Sarwal <vidhu.linux@gmail.com>

...

> > +/* A measurement takes 10-20 ms in trigger mode */
> > +#define TF_LUNA_POLL_US			(2 * USEC_PER_MSEC)
> > +#define TF_LUNA_TIMEOUT_US		(100 * USEC_PER_MSEC)
> 
> uS is better instead of US.

Currently _US is established suffix, unfortunately. But I have no strong
opinion here, I'm full to support _uS, but OTOH the _US will be consistent.

...

> > +static const struct regmap_config tf_luna_regmap_config = {
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +	.max_register = TF_LUNA_REG_SIGNATURE + 3,

No cache?

> > +};

...

> > +static int tf_luna_measure(struct tf_luna_data *data, int *dist)
> > +{
> > +	__le16 buf[2];	/* distance in cm, signal strength */
> > +	u16 old_tick, tick, amp;
> > +	int ret, err;
> > +
> > +	guard(mutex)(&data->lock);
> > +
> > +	ret = tf_luna_read_tick(data, &old_tick);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(data->regmap, TF_LUNA_REG_TRIG_ONE_SHOT,
> > +			   TF_LUNA_TRIG_ONE_SHOT);

> Why not have a local variable for regmap? It would allow the previous call
> to fit on one line.

Almost, but I agree.

	ret = regmap_write(map, TF_LUNA_REG_TRIG_ONE_SHOT, TF_LUNA_TRIG_ONE_SHOT);

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver
  2026-10-19 18:33 ` [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
  2026-10-20  7:23   ` Joshua Crofts
@ 2026-10-20  8:10   ` Andy Shevchenko
  1 sibling, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-10-20  8:10 UTC (permalink / raw)
  To: vidhu.linux
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
	David Lechner, Nuno Sá,
	Andy Shevchenko, devicetree, linux-kernel, linux-iio, Shuah Khan

On Tue, Oct 20, 2026 at 12:03:43AM +0530, Vidhu Sarwal via B4 Relay wrote:
> 
> Add a driver for the Benewake TF-Luna, a single-point time-of-flight
> LiDAR distance sensor with a range of 0.2 m to 8 m, connected over I2C.
> 
> The sensor is run in trigger mode. Each read starts one measurement and
> waits for the timestamp register to change before reading the result.
> In continuous mode a read can catch the sensor in the middle of updating
> its result registers unless it is synchronised to the data ready output,
> which this driver does not use yet.
> 
> When the signal strength is too low the sensor reports a dummy distance
> of 0, so such readings, as well as overexposed ones, return -ENODATA
> instead of a distance.

On top of my previous reply.

...

> +static int tf_luna_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct tf_luna_data *data;
> +	struct iio_dev *indio_dev;
> +	unsigned int mode;
> +	u8 signature[4];
> +	u8 version[3];
> +	int ret;

It also makes sense to have local variable here

	struct regmap *map;


> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +
> +	ret = devm_mutex_init(dev, &data->lock);
> +	if (ret)
> +		return ret;

> +	data->regmap = devm_regmap_init_i2c(client, &tf_luna_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return dev_err_probe(dev, PTR_ERR(data->regmap),
> +				     "Failed to initialize regmap\n");

	map = devm_regmap_init_i2c(client, &tf_luna_regmap_config);
	if (IS_ERR(map))
		return dev_err_probe(dev, PTR_ERR(map), "Failed to initialize regmap\n");
	data->regmap = map;

> +	ret = devm_regulator_get_enable(dev, "vcc");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable vcc regulator\n");
> +
> +	/* Boot time is not documented, about 350 ms was measured */
> +	fsleep(500 * USEC_PER_MSEC);
> +
> +	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_SIGNATURE, signature,
> +			       sizeof(signature));

Make it split logically and taking into account the above either

	ret = regmap_bulk_read(map, TF_LUNA_REG_SIGNATURE,
			       signature, sizeof(signature));

OR

	ret = regmap_bulk_read(map, TF_LUNA_REG_SIGNATURE, signature, sizeof(signature));

(perhaps you can shorten the 'signature' to 'sign' to fit).

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read signature\n");
> +
> +	if (memcmp(signature, "LUNA", sizeof(signature)))

Hmm... No. Please, make signature to be __?e32 and use a definition for this
FourCC [1] in the hexadecimal with the comment, and compare as regular integers...

> +		dev_info(dev, "Unknown signature %*ph, continuing\n",
> +			 (int)sizeof(signature), signature);

...and here use %*p4c?.

#define	TF_LUNA_SIGN	0x... /* "LUNA" in ASCII */

	__?e32 signature;
	u32 fourcc;
	...
	ret = regmap_bulk_read(map, TF_LUNA_REG_SIGNATURE, signature, sizeof(signature));
	...
	fourcc = ?e32_to_cpu(signature);
	if (fourcc != TF_LUNA_SIGN)
		dev_info(dev, "Unknown signature %p4c?, continuing\n", &fourcc);

where '?' represents either 'b' or 'l' depending on what you have and what you
want to see printed.

> +	ret = regmap_bulk_read(data->regmap, TF_LUNA_REG_VERSION, version,
> +			       sizeof(version));

	ret = regmap_bulk_read(map, TF_LUNA_REG_VERSION, version, sizeof(version));

(or 'version' --> 'ver', for example).

> +	if (ret)
> +		return ret;
>
> +	dev_dbg(dev, "Firmware version %u.%u.%u\n",
> +		version[2], version[1], version[0]);

Yeah, I need to post my series to also make this to be a pointer extension.

> +	/*
> +	 * In continuous mode a read can catch the sensor updating its result
> +	 * registers unless it is synchronised to the data ready output. Use
> +	 * trigger mode and start each measurement on demand instead.
> +	 */
> +	ret = regmap_write(data->regmap, TF_LUNA_REG_MODE, TF_LUNA_MODE_TRIGGER);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The sensor silently drops writes it is not ready for, so check that
> +	 * the mode was set. A write takes up to 100 ms to read back.
> +	 */
> +	fsleep(100 * USEC_PER_MSEC);

msleep() as it's known to be big enough.

> +	ret = regmap_read(data->regmap, TF_LUNA_REG_MODE, &mode);
> +	if (ret)
> +		return ret;
> +
> +	if (mode != TF_LUNA_MODE_TRIGGER)
> +		return dev_err_probe(dev, -EIO, "Failed to set trigger mode\n");
> +
> +	indio_dev->name = "tf-luna";
> +	indio_dev->info = &tf_luna_info;
> +	indio_dev->channels = tf_luna_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tf_luna_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

[1]: https://en.wikipedia.org/wiki/FourCC

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-20  8:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 18:33 [PATCH 0/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
2026-10-19 18:33 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add Benewake Vidhu Sarwal via B4 Relay
2026-10-19 18:33 ` [PATCH 2/3] dt-bindings: iio: proximity: Add Benewake TF-Luna Vidhu Sarwal via B4 Relay
2026-10-19 18:33 ` [PATCH 3/3] iio: proximity: Add Benewake TF-Luna LiDAR driver Vidhu Sarwal via B4 Relay
2026-10-20  7:23   ` Joshua Crofts
2026-10-20  7:42     ` Andy Shevchenko
2026-10-20  8:10   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®