* [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor
@ 2026-03-11 22:40 Siratul Islam
2026-03-11 22:40 ` [PATCH v3 1/2] dt-bindings: iio: proximity: add " Siratul Islam
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Siratul Islam @ 2026-03-11 22:40 UTC (permalink / raw)
To: linux-iio, devicetree
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
linux-kernel, Siratul Islam
This series adds support for the STMicroelectronics VL53L1X
Time-of-Flight ranging sensor.
The VL53L1X is a ToF laser-ranging sensor with I2C interface,
capable of measuring distances up to 4 meters. The driver
supports both interrupt-driven and polled operation.
Why a separate driver is needed (instead of extending vl53l0x-i2c.c):
The VL53L1X is fundamentally different from the VL53L0X despite the
similar naming. Extending the existing driver would require rewriting
the majority of it.
Key differences include:
- A different register map (16-bit addresses vs. 8-bit addresses).
- Requires a 91-byte firmware configuration blob to be loaded at boot.
- Requires a VHV calibration cycle.
- Has distance mode and timing budget configurations.
- Uses the regmap API rather than raw i2c_smbus calls.
I also reviewed other drivers in drivers/iio/proximity/ and can
confirm this IP block does not appear to be shared by any other
existing driver.
Tested on Raspberry Pi 5 with a VL53L1X breakout board.
Note on vdd-supply:
In v2, I added vdd-supply as required. Since I'm using the
shared st,vl53l0x.yaml binding now, I had to drop that requirement
to avoid breaking backward compatibility for existing st,vl53l0x
devicetrees. The driver itself still uses non-optional
devm_regulator_get() so no change there.
---
Changes in v3:
- Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof).
- Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name.
- Replace manual polling loops in chip_init and read_proximity with regmap_read_poll_timeout().
- Remove irq_get_trigger_type() and pass IRQF_NO_THREAD directly (per Andy).
- Drop struct i2c_client from private data and store irq as int instead. Derive struct device from regmap where needed.
- Add dev_err_probe() to first devm_request_irq() error path.
- Replace linux/device.h with linux/dev_printk.h, add linux/array_size.h, linux/err.h, linux/types.h (per Andy).
- Use USEC_PER_MSEC for poll timeouts.
- Remove unnecessary casts.
- Divide long config blob to 8 values per line.
- Rename goto label to notify_and_clear_irq and drop unused dev_dbg.
- Add datasheet section reference for boot delay comment.
- vdd-supply no longer required in shared binding to avoid breaking existing st,vl53l0x devicetrees.
Changes in v2:
- Skip software reset in chip_init when xshut GPIO is available, since the device was already hardware-reset during power-on.
- Rename "reset" GPIO to "xshut" to match the datasheet pin name and updated DT binding accordingly.
- Make vdd-supply required in DT binding.
- Use reg_sequence arrays and regmap_multi_reg_write() for distance mode configuration.
- Switch to a hardirq handler with iio_trigger_poll() and IRQF_NO_THREAD instead of a threaded handler.
- Drop IRQF_TRIGGER_FALLING fallback. Leave trigger type to firmware/DT.
- Use iio_validate_own_trigger() instead of a custom validate_trigger, drop trig pointer from driver data.
- Switch usleep_range() to fsleep() throughout and add comments for sleep values.
- Don't fail probe on unknown model ID, just log with dev_info().
- Split stop_ranging cleanup into its own devm action, separate from power_off.
- Add missing includes: device.h, bitfield.h, completion.h, mod_devicetable.h.
- Use FIELD_GET() for range status checks.
- Move configure_irq() closer to probe(), use dev_err_probe() for its error paths.
- Fix buffer ops symmetry: postdisable -> predisable.
- Drop reg_format_endian from regmap config.
- Various alignment and formatting fixes.
Link to v2: https://lore.kernel.org/linux-iio/20260308113728.40860-1-email@sirat.me
Link to v1: https://lore.kernel.org/linux-iio/20260303090253.42076-1-email@sirat.me
Siratul Islam (2):
dt-bindings: iio: proximity: add ST VL53L1X ToF sensor
iio: proximity: add driver for ST VL53L1X ToF sensor
.../bindings/iio/proximity/st,vl53l0x.yaml | 7 +-
MAINTAINERS | 7 +
drivers/iio/proximity/Kconfig | 14 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/vl53l1x-i2c.c | 782 ++++++++++++++++++
5 files changed, 809 insertions(+), 2 deletions(-)
create mode 100644 drivers/iio/proximity/vl53l1x-i2c.c
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v3 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor 2026-03-11 22:40 [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor Siratul Islam @ 2026-03-11 22:40 ` Siratul Islam 2026-03-11 22:40 ` [PATCH v3 2/2] iio: proximity: add driver for " Siratul Islam 2026-03-12 14:27 ` [PATCH v3 0/2] " Andy Shevchenko 2 siblings, 0 replies; 16+ messages in thread From: Siratul Islam @ 2026-03-11 22:40 UTC (permalink / raw) To: linux-iio, devicetree Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel, Siratul Islam Add device tree binding documentation for the STMicroelectronics VL53L1X Time-of-Flight ranging sensor connected via I2C. Signed-off-by: Siratul Islam <email@sirat.me> --- .../devicetree/bindings/iio/proximity/st,vl53l0x.yaml | 7 +++++-- MAINTAINERS | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml b/Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml index 322befc41de6..0852259754f5 100644 --- a/Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml +++ b/Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml @@ -4,14 +4,17 @@ $id: http://devicetree.org/schemas/iio/proximity/st,vl53l0x.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: ST VL53L0X ToF ranging sensor +title: ST VL53L0X/VL53L1X ToF ranging sensor maintainers: - Song Qiang <songqiang1304521@gmail.com> + - Siratul Islam <email@sirat.me> properties: compatible: - const: st,vl53l0x + enum: + - st,vl53l0x + - st,vl53l1x reg: maxItems: 1 diff --git a/MAINTAINERS b/MAINTAINERS index 61bf550fd37c..a142a97be4cb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25093,6 +25093,12 @@ S: Maintained F: Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml F: drivers/iio/proximity/vl53l0x-i2c.c +ST VL53L1X ToF RANGER(I2C) IIO DRIVER +M: Siratul Islam <email@sirat.me> +L: linux-iio@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml + STABLE BRANCH M: Greg Kroah-Hartman <gregkh@linuxfoundation.org> M: Sasha Levin <sashal@kernel.org> -- 2.53.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-11 22:40 [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor Siratul Islam 2026-03-11 22:40 ` [PATCH v3 1/2] dt-bindings: iio: proximity: add " Siratul Islam @ 2026-03-11 22:40 ` Siratul Islam 2026-03-12 14:58 ` Andy Shevchenko 2026-03-12 14:27 ` [PATCH v3 0/2] " Andy Shevchenko 2 siblings, 1 reply; 16+ messages in thread From: Siratul Islam @ 2026-03-11 22:40 UTC (permalink / raw) To: linux-iio, devicetree Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel, Siratul Islam Add support for the STMicroelectronics VL53L1X Time-of-Flight ranging sensor with I2C interface. Signed-off-by: Siratul Islam <email@sirat.me> --- MAINTAINERS | 1 + drivers/iio/proximity/Kconfig | 14 + drivers/iio/proximity/Makefile | 1 + drivers/iio/proximity/vl53l1x-i2c.c | 782 ++++++++++++++++++++++++++++ 4 files changed, 798 insertions(+) create mode 100644 drivers/iio/proximity/vl53l1x-i2c.c diff --git a/MAINTAINERS b/MAINTAINERS index a142a97be4cb..50531a87bf2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25098,6 +25098,7 @@ M: Siratul Islam <email@sirat.me> L: linux-iio@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml +F: drivers/iio/proximity/vl53l1x-i2c.c STABLE BRANCH M: Greg Kroah-Hartman <gregkh@linuxfoundation.org> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig index 6070974c2c85..3b0fdeb03ecd 100644 --- a/drivers/iio/proximity/Kconfig +++ b/drivers/iio/proximity/Kconfig @@ -244,6 +244,20 @@ config VL53L0X_I2C To compile this driver as a module, choose M here: the module will be called vl53l0x-i2c. +config VL53L1X_I2C + tristate "STMicroelectronics VL53L1X ToF ranger sensor (I2C)" + depends on I2C + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER + select REGMAP_I2C + help + Say Y here to build a driver for STMicroelectronics VL53L1X + ToF ranger sensors with i2c interface. + This driver can be used to measure the distance of objects. + + To compile this driver as a module, choose M here: the + module will be called vl53l1x-i2c. + config AW96103 tristate "AW96103/AW96105 Awinic proximity sensor" select REGMAP_I2C diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile index 152034d38c49..4352833dd8a4 100644 --- a/drivers/iio/proximity/Makefile +++ b/drivers/iio/proximity/Makefile @@ -23,5 +23,6 @@ obj-$(CONFIG_SX_COMMON) += sx_common.o obj-$(CONFIG_SX9500) += sx9500.o obj-$(CONFIG_VCNL3020) += vcnl3020.o obj-$(CONFIG_VL53L0X_I2C) += vl53l0x-i2c.o +obj-$(CONFIG_VL53L1X_I2C) += vl53l1x-i2c.o obj-$(CONFIG_AW96103) += aw96103.o diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c new file mode 100644 index 000000000000..e6c3efeb6051 --- /dev/null +++ b/drivers/iio/proximity/vl53l1x-i2c.c @@ -0,0 +1,782 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +/* + * Support for ST VL53L1X FlightSense ToF Ranging Sensor on a i2c bus. + * + * Copyright (C) 2026 Siratul Islam <email@sirat.me> + * + * Datasheet available at + * <https://www.st.com/resource/en/datasheet/vl53l1x.pdf> + * + * Default 7-bit i2c slave address 0x29. + * + * The VL53L1X requires a firmware configuration blob to be loaded at boot. + * Register values for the default configuration are taken from + * ST's VL53L1X Ultra Lite Driver (STSW-IMG009). + */ + +#include <linux/array_size.h> +#include <linux/bits.h> +#include <linux/bitfield.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/dev_printk.h> +#include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/types.h> + +#include <linux/iio/buffer.h> +#include <linux/iio/iio.h> +#include <linux/iio/trigger.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +#define VL53L1X_SOFT_RESET 0x0000 +#define VL53L1X_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND 0x0008 +#define VL53L1X_VHV_CONFIG__INIT 0x000B +#define VL53L1X_GPIO_HV_MUX__CTRL 0x0030 +#define VL53L1X_GPIO__TIO_HV_STATUS 0x0031 +#define VL53L1X_SYSTEM__INTERRUPT_CONFIG_GPIO 0x0046 +#define VL53L1X_PHASECAL_CONFIG__TIMEOUT_MACROP 0x004B +#define VL53L1X_RANGE_CONFIG__TIMEOUT_MACROP_A 0x005E +#define VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_A 0x0060 +#define VL53L1X_RANGE_CONFIG__TIMEOUT_MACROP_B 0x0061 +#define VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_B 0x0063 +#define VL53L1X_RANGE_CONFIG__VALID_PHASE_HIGH 0x0069 +#define VL53L1X_SYSTEM__INTERMEASUREMENT_PERIOD 0x006C +#define VL53L1X_SD_CONFIG__WOI_SD0 0x0078 +#define VL53L1X_SD_CONFIG__WOI_SD1 0x0079 +#define VL53L1X_SD_CONFIG__INITIAL_PHASE_SD0 0x007A +#define VL53L1X_SD_CONFIG__INITIAL_PHASE_SD1 0x007B +#define VL53L1X_SYSTEM__INTERRUPT_CLEAR 0x0086 +#define VL53L1X_SYSTEM__MODE_START 0x0087 +#define VL53L1X_RESULT__RANGE_STATUS 0x0089 +#define VL53L1X_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0 0x0096 +#define VL53L1X_RESULT__OSC_CALIBRATE_VAL 0x00DE +#define VL53L1X_FIRMWARE__SYSTEM_STATUS 0x00E5 +#define VL53L1X_IDENTIFICATION__MODEL_ID 0x010F + +#define VL53L1X_MODEL_ID_VAL 0xEACC + +#define VL53L1X_DEFAULT_CONFIG_ADDR 0x2D + +#define VL53L1X_MODE_START_TIMED 0x40 +#define VL53L1X_MODE_START_STOP 0x00 + +#define VL53L1X_INT_NEW_SAMPLE_READY 0x02 + +#define VL53L1X_GPIO_HV_MUX_POLARITY BIT(4) + +#define VL53L1X_VHV_LOOP_BOUND_TWO 0x09 + +#define VL53L1X_RANGE_STATUS_MASK GENMASK(4, 0) +#define VL53L1X_RANGE_STATUS_VALID 9 + +/* Inter-measurement period uses PLL divider with 1.075 oscillator correction */ +#define VL53L1X_OSC_CALIBRATE_MASK GENMASK(9, 0) +#define VL53L1X_OSC_CORRECTION_FACTOR 1075 +#define VL53L1X_OSC_CORRECTION_DIVISOR 1000 + +enum vl53l1x_distance_mode { + VL53L1X_SHORT, + VL53L1X_LONG, +}; + +struct vl53l1x_data { + struct regmap *regmap; + struct completion completion; + struct regulator *vdd_supply; + struct gpio_desc *xshut_gpio; + enum vl53l1x_distance_mode distance_mode; + u16 osc_calibrate_val; + u8 gpio_polarity; + int irq; +}; + +static const struct regmap_config vl53l1x_regmap_config = { + .reg_bits = 16, + .val_bits = 8, +}; + +static int vl53l1x_read_u16(struct vl53l1x_data *data, u16 reg, u16 *val) +{ + __be16 buf; + int ret; + + ret = regmap_bulk_read(data->regmap, reg, &buf, 2); + if (ret) + return ret; + + *val = be16_to_cpu(buf); + return 0; +} + +static int vl53l1x_write_u16(struct vl53l1x_data *data, u16 reg, u16 val) +{ + __be16 buf = cpu_to_be16(val); + + return regmap_bulk_write(data->regmap, reg, &buf, 2); +} + +static int vl53l1x_write_u32(struct vl53l1x_data *data, u16 reg, u32 val) +{ + __be32 buf = cpu_to_be32(val); + + return regmap_bulk_write(data->regmap, reg, &buf, 4); +} + +static int vl53l1x_clear_irq(struct vl53l1x_data *data) +{ + return regmap_write(data->regmap, VL53L1X_SYSTEM__INTERRUPT_CLEAR, + 0x01); +} + +static int vl53l1x_start_ranging(struct vl53l1x_data *data) +{ + int ret; + + ret = vl53l1x_clear_irq(data); + if (ret) + return ret; + + return regmap_write(data->regmap, VL53L1X_SYSTEM__MODE_START, + VL53L1X_MODE_START_TIMED); +} + +static int vl53l1x_stop_ranging(struct vl53l1x_data *data) +{ + return regmap_write(data->regmap, VL53L1X_SYSTEM__MODE_START, + VL53L1X_MODE_START_STOP); +} + +/* + * Default configuration blob from ST's VL53L1X ULD (STSW-IMG009). + */ +static const u8 vl53l1x_default_config[] = { + /* 0x2D, 0x2E, 0x2F, 0x30, 0x31, 0x32, 0x33, 0x34 */ + 0x00, 0x00, 0x00, 0x01, 0x02, 0x00, 0x02, 0x08, + /* 0x35, 0x36, 0x37, 0x38, 0x39, 0x3A, 0x3B, 0x3C */ + 0x00, 0x08, 0x10, 0x01, 0x01, 0x00, 0x00, 0x00, + /* 0x3D, 0x3E, 0x3F, 0x40, 0x41, 0x42, 0x43, 0x44 */ + 0x00, 0xFF, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00, + /* 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C */ + 0x00, 0x20, 0x0B, 0x00, 0x00, 0x02, 0x0A, 0x21, + /* 0x4D, 0x4E, 0x4F, 0x50, 0x51, 0x52, 0x53, 0x54 */ + 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0xC8, + /* 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x5B, 0x5C */ + 0x00, 0x00, 0x38, 0xFF, 0x01, 0x00, 0x08, 0x00, + /* 0x5D, 0x5E, 0x5F, 0x60, 0x61, 0x62, 0x63, 0x64 */ + 0x00, 0x01, 0xCC, 0x0F, 0x01, 0xF1, 0x0D, 0x01, + /* 0x65, 0x66, 0x67, 0x68, 0x69, 0x6A, 0x6B, 0x6C */ + 0x68, 0x00, 0x80, 0x08, 0xB8, 0x00, 0x00, 0x00, + /* 0x6D, 0x6E, 0x6F, 0x70, 0x71, 0x72, 0x73, 0x74 */ + 0x00, 0x0F, 0x89, 0x00, 0x00, 0x00, 0x00, 0x00, + /* 0x75, 0x76, 0x77, 0x78, 0x79, 0x7A, 0x7B, 0x7C */ + 0x00, 0x00, 0x01, 0x0F, 0x0D, 0x0E, 0x0E, 0x00, + /* 0x7D, 0x7E, 0x7F, 0x80, 0x81, 0x82, 0x83, 0x84 */ + 0x00, 0x02, 0xC7, 0xFF, 0x9B, 0x00, 0x00, 0x00, + /* 0x85, 0x86, 0x87 */ + 0x01, 0x00, 0x00, +}; + +static int vl53l1x_chip_init(struct vl53l1x_data *data) +{ + struct device *dev = regmap_get_device(data->regmap); + unsigned int val; + u16 model_id; + int ret; + + if (!data->xshut_gpio) { + ret = regmap_write(data->regmap, VL53L1X_SOFT_RESET, 0x00); + if (ret) + return ret; + fsleep(100); /* conservative reset pulse, no spec */ + + ret = regmap_write(data->regmap, VL53L1X_SOFT_RESET, 0x01); + if (ret) + return ret; + fsleep(1000); /* conservative boot wait, no spec */ + } + + ret = regmap_read_poll_timeout(data->regmap, + VL53L1X_FIRMWARE__SYSTEM_STATUS, val, + val & BIT(0), + 1 * USEC_PER_MSEC, + 100 * USEC_PER_MSEC); + if (ret) { + dev_err(dev, "firmware boot timeout\n"); + return ret; + } + + ret = vl53l1x_read_u16(data, VL53L1X_IDENTIFICATION__MODEL_ID, + &model_id); + if (ret) + return ret; + + if (model_id != VL53L1X_MODEL_ID_VAL) + dev_info(dev, "unknown model id: 0x%04x, continuing\n", model_id); + + ret = vl53l1x_read_u16(data, VL53L1X_RESULT__OSC_CALIBRATE_VAL, + &data->osc_calibrate_val); + if (ret) + return ret; + + ret = regmap_bulk_write(data->regmap, VL53L1X_DEFAULT_CONFIG_ADDR, + vl53l1x_default_config, + sizeof(vl53l1x_default_config)); + if (ret) + return ret; + + ret = regmap_read(data->regmap, VL53L1X_GPIO_HV_MUX__CTRL, &val); + if (ret) + return ret; + data->gpio_polarity = !!(val & VL53L1X_GPIO_HV_MUX_POLARITY); + + /* Initial ranging cycle for VHV calibration */ + ret = vl53l1x_start_ranging(data); + if (ret) + return ret; + + /* 1ms poll, 1s timeout covers max timing budgets (per ST ULD) */ + ret = regmap_read_poll_timeout(data->regmap, + VL53L1X_GPIO__TIO_HV_STATUS, val, + (val & 0x01) == (!data->gpio_polarity), + 1 * USEC_PER_MSEC, + 1000 * USEC_PER_MSEC); + if (ret) + return ret; + + ret = vl53l1x_clear_irq(data); + if (ret) + return ret; + + ret = vl53l1x_stop_ranging(data); + if (ret) + return ret; + + ret = regmap_write(data->regmap, + VL53L1X_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND, + VL53L1X_VHV_LOOP_BOUND_TWO); + if (ret) + return ret; + + return regmap_write(data->regmap, VL53L1X_VHV_CONFIG__INIT, 0x00); +} + +static const struct reg_sequence vl53l1x_mode_short[] = { + { VL53L1X_PHASECAL_CONFIG__TIMEOUT_MACROP, 0x14 }, + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_A, 0x07 }, + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_B, 0x05 }, + { VL53L1X_RANGE_CONFIG__VALID_PHASE_HIGH, 0x38 }, + { VL53L1X_SD_CONFIG__WOI_SD0, 0x07 }, + { VL53L1X_SD_CONFIG__WOI_SD1, 0x05 }, + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD0, 6 }, + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD1, 6 }, +}; + +static const struct reg_sequence vl53l1x_mode_long[] = { + { VL53L1X_PHASECAL_CONFIG__TIMEOUT_MACROP, 0x0A }, + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_A, 0x0F }, + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_B, 0x0D }, + { VL53L1X_RANGE_CONFIG__VALID_PHASE_HIGH, 0xB8 }, + { VL53L1X_SD_CONFIG__WOI_SD0, 0x0F }, + { VL53L1X_SD_CONFIG__WOI_SD1, 0x0D }, + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD0, 14 }, + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD1, 14 }, +}; + +static const struct { + const struct reg_sequence *regs; + size_t num_regs; +} vl53l1x_mode_configs[] = { + [VL53L1X_SHORT] = { vl53l1x_mode_short, ARRAY_SIZE(vl53l1x_mode_short) }, + [VL53L1X_LONG] = { vl53l1x_mode_long, ARRAY_SIZE(vl53l1x_mode_long) }, +}; + +static int vl53l1x_set_distance_mode(struct vl53l1x_data *data, + enum vl53l1x_distance_mode mode) +{ + int ret; + + if (mode >= ARRAY_SIZE(vl53l1x_mode_configs)) + return -EINVAL; + + ret = regmap_multi_reg_write(data->regmap, + vl53l1x_mode_configs[mode].regs, + vl53l1x_mode_configs[mode].num_regs); + if (ret) + return ret; + + data->distance_mode = mode; + return 0; +} + +/* + * The timing budget controls how long the sensor spends collecting + * a single range measurement. Pre-computed TIMEOUT_MACROP register + * values from ST's VL53L1X ULD. + */ +static int vl53l1x_set_timing_budget(struct vl53l1x_data *data, u16 budget_ms) +{ + u16 timeout_a, timeout_b; + int ret; + + switch (data->distance_mode) { + case VL53L1X_SHORT: + switch (budget_ms) { + case 15: + timeout_a = 0x001D; + timeout_b = 0x0027; + break; + case 20: + timeout_a = 0x0051; + timeout_b = 0x006E; + break; + case 33: + timeout_a = 0x00D6; + timeout_b = 0x006E; + break; + case 50: + timeout_a = 0x01AE; + timeout_b = 0x01E8; + break; + case 100: + timeout_a = 0x02E1; + timeout_b = 0x0388; + break; + case 200: + timeout_a = 0x03E1; + timeout_b = 0x0496; + break; + case 500: + timeout_a = 0x0591; + timeout_b = 0x05C1; + break; + default: + return -EINVAL; + } + break; + case VL53L1X_LONG: + switch (budget_ms) { + case 20: + timeout_a = 0x001E; + timeout_b = 0x0022; + break; + case 33: + timeout_a = 0x0060; + timeout_b = 0x006E; + break; + case 50: + timeout_a = 0x00AD; + timeout_b = 0x00C6; + break; + case 100: + timeout_a = 0x01CC; + timeout_b = 0x01EA; + break; + case 200: + timeout_a = 0x02D9; + timeout_b = 0x02F8; + break; + case 500: + timeout_a = 0x048F; + timeout_b = 0x04A4; + break; + default: + return -EINVAL; + } + break; + default: + return -EINVAL; + } + + ret = vl53l1x_write_u16(data, VL53L1X_RANGE_CONFIG__TIMEOUT_MACROP_A, + timeout_a); + if (ret) + return ret; + + return vl53l1x_write_u16(data, VL53L1X_RANGE_CONFIG__TIMEOUT_MACROP_B, + timeout_b); +} + +static int vl53l1x_set_inter_measurement_ms(struct vl53l1x_data *data, + u16 period_ms) +{ + u16 clock_pll = data->osc_calibrate_val & VL53L1X_OSC_CALIBRATE_MASK; + u32 inter_meas; + + inter_meas = clock_pll * period_ms; + inter_meas = (inter_meas * VL53L1X_OSC_CORRECTION_FACTOR) / + VL53L1X_OSC_CORRECTION_DIVISOR; + + return vl53l1x_write_u32(data, VL53L1X_SYSTEM__INTERMEASUREMENT_PERIOD, + inter_meas); +} + +static int vl53l1x_read_proximity(struct vl53l1x_data *data, int *val) +{ + unsigned long time_left; + unsigned int range_status; + u16 distance; + int ret; + + if (data->irq) { + reinit_completion(&data->completion); + + ret = vl53l1x_clear_irq(data); + if (ret) + return ret; + + time_left = wait_for_completion_timeout(&data->completion, HZ); + if (time_left == 0) + return -ETIMEDOUT; + } else { + unsigned int rdy; + + /* 1ms poll, 1s timeout covers max timing budgets (per ST ULD) */ + ret = regmap_read_poll_timeout(data->regmap, + VL53L1X_GPIO__TIO_HV_STATUS, rdy, + (rdy & 0x01) == (!data->gpio_polarity), + 1 * USEC_PER_MSEC, + 1000 * USEC_PER_MSEC); + if (ret) + return ret; + } + + ret = regmap_read(data->regmap, VL53L1X_RESULT__RANGE_STATUS, + &range_status); + if (ret) + goto clear_irq; + + if (FIELD_GET(VL53L1X_RANGE_STATUS_MASK, range_status) != VL53L1X_RANGE_STATUS_VALID) { + ret = -EIO; + goto clear_irq; + } + + ret = vl53l1x_read_u16(data, + VL53L1X_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0, + &distance); + if (ret) + goto clear_irq; + + *val = distance; + +clear_irq: + vl53l1x_clear_irq(data); + return ret; +} + +static const struct iio_chan_spec vl53l1x_channels[] = { + { + .type = IIO_DISTANCE, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 0, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + }, + }, + IIO_CHAN_SOFT_TIMESTAMP(1), +}; + +static int vl53l1x_read_raw(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, int *val, + int *val2, long mask) +{ + struct vl53l1x_data *data = iio_priv(indio_dev); + int ret; + + if (chan->type != IIO_DISTANCE) + return -EINVAL; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + if (!iio_device_claim_direct(indio_dev)) + return -EBUSY; + ret = vl53l1x_read_proximity(data, val); + iio_device_release_direct(indio_dev); + if (ret) + return ret; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = 0; + *val2 = 1000; + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } +} + +static const struct iio_info vl53l1x_info = { + .read_raw = vl53l1x_read_raw, + .validate_trigger = iio_validate_own_trigger, +}; + +static irqreturn_t vl53l1x_trigger_handler(int irq, void *priv) +{ + struct iio_poll_func *pf = priv; + struct iio_dev *indio_dev = pf->indio_dev; + struct vl53l1x_data *data = iio_priv(indio_dev); + struct { + u16 distance; + aligned_s64 timestamp; + } scan = {}; + unsigned int range_status; + int ret; + + ret = regmap_read(data->regmap, VL53L1X_RESULT__RANGE_STATUS, + &range_status); + if (ret || FIELD_GET(VL53L1X_RANGE_STATUS_MASK, range_status) != + VL53L1X_RANGE_STATUS_VALID) + goto notify_and_clear_irq; + + ret = vl53l1x_read_u16(data, + VL53L1X_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0, + &scan.distance); + if (ret) + goto notify_and_clear_irq; + + iio_push_to_buffers_with_timestamp(indio_dev, &scan, + iio_get_time_ns(indio_dev)); + +notify_and_clear_irq: + iio_trigger_notify_done(indio_dev->trig); + vl53l1x_clear_irq(data); + + return IRQ_HANDLED; +} + +static irqreturn_t vl53l1x_irq_handler(int irq, void *priv) +{ + struct iio_dev *indio_dev = priv; + struct vl53l1x_data *data = iio_priv(indio_dev); + + if (iio_buffer_enabled(indio_dev)) + iio_trigger_poll(indio_dev->trig); + else + complete(&data->completion); + + return IRQ_HANDLED; +} + +static int vl53l1x_buffer_postenable(struct iio_dev *indio_dev) +{ + struct vl53l1x_data *data = iio_priv(indio_dev); + + return vl53l1x_start_ranging(data); +} + +static int vl53l1x_buffer_predisable(struct iio_dev *indio_dev) +{ + struct vl53l1x_data *data = iio_priv(indio_dev); + int ret; + + ret = vl53l1x_stop_ranging(data); + if (ret) + return ret; + + reinit_completion(&data->completion); + wait_for_completion_timeout(&data->completion, HZ / 10); + + return vl53l1x_clear_irq(data); +} + +static const struct iio_buffer_setup_ops vl53l1x_buffer_setup_ops = { + .postenable = &vl53l1x_buffer_postenable, + .predisable = &vl53l1x_buffer_predisable, +}; + +static const struct iio_trigger_ops vl53l1x_trigger_ops = { + .validate_device = iio_trigger_validate_own_device, +}; + +static void vl53l1x_stop_ranging_action(void *_data) +{ + vl53l1x_stop_ranging(_data); +} + +static void vl53l1x_power_off(void *_data) +{ + struct vl53l1x_data *data = _data; + + gpiod_set_value_cansleep(data->xshut_gpio, 1); + regulator_disable(data->vdd_supply); +} + +static int vl53l1x_power_on(struct vl53l1x_data *data) +{ + int ret; + + ret = regulator_enable(data->vdd_supply); + if (ret) + return ret; + + gpiod_set_value_cansleep(data->xshut_gpio, 0); + /* + * 1.2 ms max boot duration. + * Datasheet Section 3.6 "Power up and boot sequence". + */ + fsleep(1200); + + return 0; +} + +static int vl53l1x_configure_irq(struct device *dev, int irq, + struct iio_dev *indio_dev) +{ + struct vl53l1x_data *data = iio_priv(indio_dev); + int ret; + + ret = devm_request_irq(dev, irq, vl53l1x_irq_handler, + IRQF_NO_THREAD, + indio_dev->name, indio_dev); + if (ret) + return dev_err_probe(dev, ret, "failed to request IRQ\n"); + + ret = regmap_write(data->regmap, VL53L1X_SYSTEM__INTERRUPT_CONFIG_GPIO, + VL53L1X_INT_NEW_SAMPLE_READY); + if (ret) + return dev_err_probe(dev, ret, + "failed to configure IRQ\n"); + + return 0; +} + +static int vl53l1x_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct vl53l1x_data *data; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + data->irq = client->irq; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_READ_I2C_BLOCK | + I2C_FUNC_SMBUS_BYTE_DATA)) + return -EOPNOTSUPP; + + data->regmap = devm_regmap_init_i2c(client, &vl53l1x_regmap_config); + if (IS_ERR(data->regmap)) + return dev_err_probe(dev, PTR_ERR(data->regmap), + "regmap initialization failed\n"); + + data->vdd_supply = devm_regulator_get(dev, "vdd"); + if (IS_ERR(data->vdd_supply)) + return dev_err_probe(dev, PTR_ERR(data->vdd_supply), + "Unable to get VDD regulator\n"); + + data->xshut_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(data->xshut_gpio)) + return dev_err_probe(dev, PTR_ERR(data->xshut_gpio), + "Cannot get reset GPIO\n"); + + ret = vl53l1x_power_on(data); + if (ret) + return dev_err_probe(dev, ret, + "Failed to power on the chip\n"); + + ret = devm_add_action_or_reset(dev, vl53l1x_power_off, data); + if (ret) + return ret; + + ret = vl53l1x_chip_init(data); + if (ret) + return ret; + + ret = vl53l1x_set_distance_mode(data, VL53L1X_LONG); + if (ret) + return ret; + + ret = vl53l1x_set_timing_budget(data, 50); + if (ret) + return ret; + + ret = vl53l1x_set_inter_measurement_ms(data, 50); + if (ret) + return ret; + + ret = vl53l1x_start_ranging(data); + if (ret) + return ret; + + ret = devm_add_action_or_reset(dev, vl53l1x_stop_ranging_action, data); + if (ret) + return ret; + + indio_dev->name = "vl53l1x"; + indio_dev->info = &vl53l1x_info; + indio_dev->channels = vl53l1x_channels; + indio_dev->num_channels = ARRAY_SIZE(vl53l1x_channels); + indio_dev->modes = INDIO_DIRECT_MODE; + + if (client->irq) { + struct iio_trigger *trig; + + init_completion(&data->completion); + + trig = devm_iio_trigger_alloc(dev, "%s-dev%d", + indio_dev->name, + iio_device_id(indio_dev)); + if (!trig) + return -ENOMEM; + + trig->ops = &vl53l1x_trigger_ops; + iio_trigger_set_drvdata(trig, indio_dev); + ret = devm_iio_trigger_register(dev, trig); + if (ret) + return ret; + + indio_dev->trig = iio_trigger_get(trig); + + ret = vl53l1x_configure_irq(dev, client->irq, indio_dev); + if (ret) + return ret; + + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, + &vl53l1x_trigger_handler, + &vl53l1x_buffer_setup_ops); + if (ret) + return ret; + } + + return devm_iio_device_register(dev, indio_dev); +} + +static const struct i2c_device_id vl53l1x_id[] = { + { "vl53l1x" }, + { } +}; +MODULE_DEVICE_TABLE(i2c, vl53l1x_id); + +static const struct of_device_id st_vl53l1x_dt_match[] = { + { .compatible = "st,vl53l1x" }, + { } +}; +MODULE_DEVICE_TABLE(of, st_vl53l1x_dt_match); + +static struct i2c_driver vl53l1x_driver = { + .driver = { + .name = "vl53l1x-i2c", + .of_match_table = st_vl53l1x_dt_match, + }, + .probe = vl53l1x_probe, + .id_table = vl53l1x_id, +}; +module_i2c_driver(vl53l1x_driver); + +MODULE_AUTHOR("Siratul Islam <email@sirat.me>"); +MODULE_DESCRIPTION("ST VL53L1X ToF ranging sensor driver"); +MODULE_LICENSE("Dual BSD/GPL"); -- 2.53.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-11 22:40 ` [PATCH v3 2/2] iio: proximity: add driver for " Siratul Islam @ 2026-03-12 14:58 ` Andy Shevchenko 2026-03-12 17:37 ` Sirat 0 siblings, 1 reply; 16+ messages in thread From: Andy Shevchenko @ 2026-03-12 14:58 UTC (permalink / raw) To: Siratul Islam Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, Mar 12, 2026 at 04:40:37AM +0600, Siratul Islam wrote: > Add support for the STMicroelectronics VL53L1X Time-of-Flight > ranging sensor with I2C interface. ... > +#include <linux/array_size.h> > +#include <linux/bits.h> > +#include <linux/bitfield.h> > +#include <linux/completion.h> > +#include <linux/delay.h> > +#include <linux/dev_printk.h> > +#include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> + time.h // for USEC_PER_* > +#include <linux/types.h> + asm/byteorder.h // for be16_to_*() et alai ... > +#define VL53L1X_OSC_CORRECTION_FACTOR 1075 > +#define VL53L1X_OSC_CORRECTION_DIVISOR 1000 In math.h we have struct u32_fract. Use that instead /* ...the comment... */ static const struct u32_fract ..._correction = { .numerator = 1075, .denominator = 1000, }; ... > +struct vl53l1x_data { > + struct regmap *regmap; > + struct completion completion; > + struct regulator *vdd_supply; > + struct gpio_desc *xshut_gpio; > + enum vl53l1x_distance_mode distance_mode; > + u16 osc_calibrate_val; > + u8 gpio_polarity; > + int irq; > +}; > +static const struct regmap_config vl53l1x_regmap_config = { > + .reg_bits = 16, > + .val_bits = 8, No cache? > +}; ... > +static int vl53l1x_read_u16(struct vl53l1x_data *data, u16 reg, u16 *val) > +{ > + __be16 buf; > + int ret; > + > + ret = regmap_bulk_read(data->regmap, reg, &buf, 2); Here, and everywhere else in the same situation use sizeof(). > + if (ret) > + return ret; > + > + *val = be16_to_cpu(buf); > + return 0; > +} ... > +static int vl53l1x_clear_irq(struct vl53l1x_data *data) > +{ > + return regmap_write(data->regmap, VL53L1X_SYSTEM__INTERRUPT_CLEAR, > + 0x01); It's perfectly one line (yes, it's fine to have it 81 characters). > +} ... > +static const u8 vl53l1x_default_config[] = { > + /* 0x2D, 0x2E, 0x2F, 0x30, 0x31, 0x32, 0x33, 0x34 */ Just compress this to > + 0x00, 0x00, 0x00, 0x01, 0x02, 0x00, 0x02, 0x08, 0x00, 0x00, 0x00, 0x01, 0x02, 0x00, 0x02, 0x08, /* 0x2d..0x34 */ Same approach to the rest. > + /* 0x35, 0x36, 0x37, 0x38, 0x39, 0x3A, 0x3B, 0x3C */ > + 0x00, 0x08, 0x10, 0x01, 0x01, 0x00, 0x00, 0x00, > + /* 0x3D, 0x3E, 0x3F, 0x40, 0x41, 0x42, 0x43, 0x44 */ > + 0x00, 0xFF, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00, > + /* 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C */ > + 0x00, 0x20, 0x0B, 0x00, 0x00, 0x02, 0x0A, 0x21, > + /* 0x4D, 0x4E, 0x4F, 0x50, 0x51, 0x52, 0x53, 0x54 */ > + 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0xC8, > + /* 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x5B, 0x5C */ > + 0x00, 0x00, 0x38, 0xFF, 0x01, 0x00, 0x08, 0x00, > + /* 0x5D, 0x5E, 0x5F, 0x60, 0x61, 0x62, 0x63, 0x64 */ > + 0x00, 0x01, 0xCC, 0x0F, 0x01, 0xF1, 0x0D, 0x01, > + /* 0x65, 0x66, 0x67, 0x68, 0x69, 0x6A, 0x6B, 0x6C */ > + 0x68, 0x00, 0x80, 0x08, 0xB8, 0x00, 0x00, 0x00, > + /* 0x6D, 0x6E, 0x6F, 0x70, 0x71, 0x72, 0x73, 0x74 */ > + 0x00, 0x0F, 0x89, 0x00, 0x00, 0x00, 0x00, 0x00, > + /* 0x75, 0x76, 0x77, 0x78, 0x79, 0x7A, 0x7B, 0x7C */ > + 0x00, 0x00, 0x01, 0x0F, 0x0D, 0x0E, 0x0E, 0x00, > + /* 0x7D, 0x7E, 0x7F, 0x80, 0x81, 0x82, 0x83, 0x84 */ > + 0x00, 0x02, 0xC7, 0xFF, 0x9B, 0x00, 0x00, 0x00, > + /* 0x85, 0x86, 0x87 */ > + 0x01, 0x00, 0x00, > +}; ... > +static int vl53l1x_chip_init(struct vl53l1x_data *data) > +{ > + struct device *dev = regmap_get_device(data->regmap); > + unsigned int val; > + u16 model_id; > + int ret; > + > + if (!data->xshut_gpio) { > + ret = regmap_write(data->regmap, VL53L1X_SOFT_RESET, 0x00); > + if (ret) > + return ret; > + fsleep(100); /* conservative reset pulse, no spec */ > + > + ret = regmap_write(data->regmap, VL53L1X_SOFT_RESET, 0x01); > + if (ret) > + return ret; > + fsleep(1000); /* conservative boot wait, no spec */ > + } > + > + ret = regmap_read_poll_timeout(data->regmap, > + VL53L1X_FIRMWARE__SYSTEM_STATUS, val, > + val & BIT(0), > + 1 * USEC_PER_MSEC, > + 100 * USEC_PER_MSEC); > + if (ret) { > + dev_err(dev, "firmware boot timeout\n"); > + return ret; This is used solely in probe phase, hence it's fine to return dev_err_probe(...); > + } > + > + ret = vl53l1x_read_u16(data, VL53L1X_IDENTIFICATION__MODEL_ID, > + &model_id); > + if (ret) > + return ret; > + > + if (model_id != VL53L1X_MODEL_ID_VAL) > + dev_info(dev, "unknown model id: 0x%04x, continuing\n", model_id); > + > + ret = vl53l1x_read_u16(data, VL53L1X_RESULT__OSC_CALIBRATE_VAL, > + &data->osc_calibrate_val); > + if (ret) > + return ret; > + > + ret = regmap_bulk_write(data->regmap, VL53L1X_DEFAULT_CONFIG_ADDR, > + vl53l1x_default_config, > + sizeof(vl53l1x_default_config)); > + if (ret) > + return ret; > + > + ret = regmap_read(data->regmap, VL53L1X_GPIO_HV_MUX__CTRL, &val); > + if (ret) > + return ret; > + data->gpio_polarity = !!(val & VL53L1X_GPIO_HV_MUX_POLARITY); > + > + /* Initial ranging cycle for VHV calibration */ > + ret = vl53l1x_start_ranging(data); > + if (ret) > + return ret; > + > + /* 1ms poll, 1s timeout covers max timing budgets (per ST ULD) */ What does ST ULD mean?! Please, avoid being cryptic in the comments. > + ret = regmap_read_poll_timeout(data->regmap, > + VL53L1X_GPIO__TIO_HV_STATUS, val, > + (val & 0x01) == (!data->gpio_polarity), Too many parentheses > + 1 * USEC_PER_MSEC, > + 1000 * USEC_PER_MSEC); > + if (ret) > + return ret; > + > + ret = vl53l1x_clear_irq(data); > + if (ret) > + return ret; > + > + ret = vl53l1x_stop_ranging(data); > + if (ret) > + return ret; > + > + ret = regmap_write(data->regmap, > + VL53L1X_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND, > + VL53L1X_VHV_LOOP_BOUND_TWO); > + if (ret) > + return ret; > + > + return regmap_write(data->regmap, VL53L1X_VHV_CONFIG__INIT, 0x00); > +} ... > +static const struct reg_sequence vl53l1x_mode_short[] = { > + { VL53L1X_PHASECAL_CONFIG__TIMEOUT_MACROP, 0x14 }, > + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_A, 0x07 }, > + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_B, 0x05 }, > + { VL53L1X_RANGE_CONFIG__VALID_PHASE_HIGH, 0x38 }, > + { VL53L1X_SD_CONFIG__WOI_SD0, 0x07 }, > + { VL53L1X_SD_CONFIG__WOI_SD1, 0x05 }, > + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD0, 6 }, > + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD1, 6 }, Why out of a sudden decimal values? Why the rest is not decimal? > +}; > + > +static const struct reg_sequence vl53l1x_mode_long[] = { > + { VL53L1X_PHASECAL_CONFIG__TIMEOUT_MACROP, 0x0A }, > + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_A, 0x0F }, > + { VL53L1X_RANGE_CONFIG__VCSEL_PERIOD_B, 0x0D }, > + { VL53L1X_RANGE_CONFIG__VALID_PHASE_HIGH, 0xB8 }, > + { VL53L1X_SD_CONFIG__WOI_SD0, 0x0F }, > + { VL53L1X_SD_CONFIG__WOI_SD1, 0x0D }, > + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD0, 14 }, > + { VL53L1X_SD_CONFIG__INITIAL_PHASE_SD1, 14 }, Ditto. > +}; ... > + inter_meas = clock_pll * period_ms; > + inter_meas = (inter_meas * VL53L1X_OSC_CORRECTION_FACTOR) / > + VL53L1X_OSC_CORRECTION_DIVISOR; Will become inter_meas = (clock_pll * period_ms * ..._correction.numerator) / ..._correction.denominator. ... > +static int vl53l1x_read_proximity(struct vl53l1x_data *data, int *val) > +{ > + unsigned long time_left; Useless, one time use variable. > + unsigned int range_status; > + u16 distance; > + int ret; > + > + if (data->irq) { > + reinit_completion(&data->completion); > + > + ret = vl53l1x_clear_irq(data); > + if (ret) > + return ret; > + > + time_left = wait_for_completion_timeout(&data->completion, HZ); > + if (time_left == 0) > + return -ETIMEDOUT; if (... == 0) return -ETIMEDOUT; > + } else { > + unsigned int rdy; > + > + /* 1ms poll, 1s timeout covers max timing budgets (per ST ULD) */ > + ret = regmap_read_poll_timeout(data->regmap, > + VL53L1X_GPIO__TIO_HV_STATUS, rdy, > + (rdy & 0x01) == (!data->gpio_polarity), Too many parentheses. > + 1 * USEC_PER_MSEC, > + 1000 * USEC_PER_MSEC); > + if (ret) > + return ret; > + } > + > + ret = regmap_read(data->regmap, VL53L1X_RESULT__RANGE_STATUS, > + &range_status); > + if (ret) > + goto clear_irq; > + > + if (FIELD_GET(VL53L1X_RANGE_STATUS_MASK, range_status) != VL53L1X_RANGE_STATUS_VALID) { Be consistent in the chosen style. Either 80 (which is still preferable in IIO) or 100, but everywhere. > + ret = -EIO; > + goto clear_irq; > + } > + > + ret = vl53l1x_read_u16(data, > + VL53L1X_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0, > + &distance); > + if (ret) > + goto clear_irq; > + > + *val = distance; > + > +clear_irq: > + vl53l1x_clear_irq(data); > + return ret; > +} ... > +static int vl53l1x_read_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, int *val, > + int *val2, long mask) Try to make wraps on a logical borders: static int vl53l1x_read_raw(struct iio_dev *indio_dev, const struct iio_chan_spec *chan, int *val, int *val2, long mask) ... > +static irqreturn_t vl53l1x_trigger_handler(int irq, void *priv) > +{ > + struct iio_poll_func *pf = priv; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct vl53l1x_data *data = iio_priv(indio_dev); > + struct { > + u16 distance; > + aligned_s64 timestamp; > + } scan = {}; > + unsigned int range_status; > + int ret; > + > + ret = regmap_read(data->regmap, VL53L1X_RESULT__RANGE_STATUS, > + &range_status); > + if (ret || FIELD_GET(VL53L1X_RANGE_STATUS_MASK, range_status) != > + VL53L1X_RANGE_STATUS_VALID) Just split for the sake of readability. > + goto notify_and_clear_irq; if (ret) goto notify_and_clear_irq; if (FIELD_GET(VL53L1X_RANGE_STATUS_MASK, range_status) != VL53L1X_RANGE_STATUS_VALID) goto notify_and_clear_irq; > + ret = vl53l1x_read_u16(data, > + VL53L1X_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0, > + &scan.distance); > + if (ret) > + goto notify_and_clear_irq; > + > + iio_push_to_buffers_with_timestamp(indio_dev, &scan, > + iio_get_time_ns(indio_dev)); > + > +notify_and_clear_irq: > + iio_trigger_notify_done(indio_dev->trig); > + vl53l1x_clear_irq(data); > + > + return IRQ_HANDLED; > +} ... > +static int vl53l1x_buffer_predisable(struct iio_dev *indio_dev) > +{ > + struct vl53l1x_data *data = iio_priv(indio_dev); > + int ret; > + > + ret = vl53l1x_stop_ranging(data); > + if (ret) > + return ret; > + > + reinit_completion(&data->completion); > + wait_for_completion_timeout(&data->completion, HZ / 10); No error condition check? > + return vl53l1x_clear_irq(data); > +} ... > +static int vl53l1x_power_on(struct vl53l1x_data *data) > +{ > + int ret; > + > + ret = regulator_enable(data->vdd_supply); > + if (ret) > + return ret; > + > + gpiod_set_value_cansleep(data->xshut_gpio, 0); > + /* > + * 1.2 ms max boot duration. > + * Datasheet Section 3.6 "Power up and boot sequence". > + */ > + fsleep(1200); > + > + return 0; > +} ... > +static int vl53l1x_configure_irq(struct device *dev, int irq, > + struct iio_dev *indio_dev) > +{ > + struct vl53l1x_data *data = iio_priv(indio_dev); > + int ret; > + > + ret = devm_request_irq(dev, irq, vl53l1x_irq_handler, > + IRQF_NO_THREAD, > + indio_dev->name, indio_dev); There is more room on the previous lines, use it. > + if (ret) > + return dev_err_probe(dev, ret, "failed to request IRQ\n"); Dup. IRQ core prints this for you. > + > + ret = regmap_write(data->regmap, VL53L1X_SYSTEM__INTERRUPT_CONFIG_GPIO, > + VL53L1X_INT_NEW_SAMPLE_READY); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to configure IRQ\n"); > + > + return 0; > +} ... > +static int vl53l1x_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct vl53l1x_data *data; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + data->irq = client->irq; > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_SMBUS_READ_I2C_BLOCK | > + I2C_FUNC_SMBUS_BYTE_DATA)) > + return -EOPNOTSUPP; Why not checking for this before even doing any allocations? > + data->regmap = devm_regmap_init_i2c(client, &vl53l1x_regmap_config); > + if (IS_ERR(data->regmap)) > + return dev_err_probe(dev, PTR_ERR(data->regmap), > + "regmap initialization failed\n"); > + > + data->vdd_supply = devm_regulator_get(dev, "vdd"); > + if (IS_ERR(data->vdd_supply)) > + return dev_err_probe(dev, PTR_ERR(data->vdd_supply), > + "Unable to get VDD regulator\n"); > + data->xshut_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(data->xshut_gpio)) > + return dev_err_probe(dev, PTR_ERR(data->xshut_gpio), > + "Cannot get reset GPIO\n"); If it's a reset (semantically), use reset-gpio driver and reset APIs here. > + ret = vl53l1x_power_on(data); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to power on the chip\n"); Single line. > + > + ret = devm_add_action_or_reset(dev, vl53l1x_power_off, data); > + if (ret) > + return ret; > + > + ret = vl53l1x_chip_init(data); > + if (ret) > + return ret; > + > + ret = vl53l1x_set_distance_mode(data, VL53L1X_LONG); > + if (ret) > + return ret; > + > + ret = vl53l1x_set_timing_budget(data, 50); > + if (ret) > + return ret; > + > + ret = vl53l1x_set_inter_measurement_ms(data, 50); > + if (ret) > + return ret; > + > + ret = vl53l1x_start_ranging(data); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(dev, vl53l1x_stop_ranging_action, data); > + if (ret) > + return ret; > + > + indio_dev->name = "vl53l1x"; > + indio_dev->info = &vl53l1x_info; > + indio_dev->channels = vl53l1x_channels; > + indio_dev->num_channels = ARRAY_SIZE(vl53l1x_channels); > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + if (client->irq) { > + struct iio_trigger *trig; > + > + init_completion(&data->completion); > + > + trig = devm_iio_trigger_alloc(dev, "%s-dev%d", > + indio_dev->name, > + iio_device_id(indio_dev)); More room on the previous lines, use it. > + if (!trig) > + return -ENOMEM; > + > + trig->ops = &vl53l1x_trigger_ops; > + iio_trigger_set_drvdata(trig, indio_dev); > + ret = devm_iio_trigger_register(dev, trig); > + if (ret) > + return ret; > + > + indio_dev->trig = iio_trigger_get(trig); > + > + ret = vl53l1x_configure_irq(dev, client->irq, indio_dev); > + if (ret) > + return ret; > + > + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, > + &vl53l1x_trigger_handler, > + &vl53l1x_buffer_setup_ops); > + if (ret) > + return ret; > + } > + > + return devm_iio_device_register(dev, indio_dev); > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-12 14:58 ` Andy Shevchenko @ 2026-03-12 17:37 ` Sirat 2026-03-13 9:35 ` Andy Shevchenko 2026-03-14 12:27 ` Jonathan Cameron 0 siblings, 2 replies; 16+ messages in thread From: Sirat @ 2026-03-12 17:37 UTC (permalink / raw) To: Andy Shevchenko Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, Mar 12, 2026 at 8:58 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Thu, Mar 12, 2026 at 04:40:37AM +0600, Siratul Islam wrote: > > Add support for the STMicroelectronics VL53L1X Time-of-Flight > > ranging sensor with I2C interface. > ... > > > +static const struct regmap_config vl53l1x_regmap_config = { > > + .reg_bits = 16, > > + .val_bits = 8, > > No cache? > The result registers change on every measurement, so caching doesn't apply well here. Will add .cache_type = REGCACHE_NONE with a comment to clarify. > > +}; > > ... > > + > > + reinit_completion(&data->completion); > > > + wait_for_completion_timeout(&data->completion, HZ / 10); > > No error condition check? > This is only a short drain to make sure any pending IRQ has been handled before we stop the device and clear the interrupt line. A timeout here is not treated as a failure and the device is being shut down anyway, so I intentionally do not check the return value. > > ... > > > + data->xshut_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > + if (IS_ERR(data->xshut_gpio)) > > + return dev_err_probe(dev, PTR_ERR(data->xshut_gpio), > > + "Cannot get reset GPIO\n"); > > If it's a reset (semantically), use reset-gpio driver and reset APIs here. > The XSHUT pin is a shutdown/enable pin rather than a true reset line, and the existing shared st,vl53l0x.yaml binding already defines it as reset-gpios. So I am keeping the GPIO consumer API here. > > Sending v4 with all other fixes from your review, shortly. Thanks, Sirat ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-12 17:37 ` Sirat @ 2026-03-13 9:35 ` Andy Shevchenko 2026-03-13 10:55 ` Sirat 2026-03-14 12:27 ` Jonathan Cameron 1 sibling, 1 reply; 16+ messages in thread From: Andy Shevchenko @ 2026-03-13 9:35 UTC (permalink / raw) To: Sirat Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, Mar 12, 2026 at 11:37:26PM +0600, Sirat wrote: > On Thu, Mar 12, 2026 at 8:58 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > On Thu, Mar 12, 2026 at 04:40:37AM +0600, Siratul Islam wrote: ... > > > +static const struct regmap_config vl53l1x_regmap_config = { > > > + .reg_bits = 16, > > > + .val_bits = 8, > > > > No cache? > > > The result registers change on every measurement, so caching doesn't > apply well here. > Will add .cache_type = REGCACHE_NONE with a comment to clarify. OK! > > > +}; ... > > > + reinit_completion(&data->completion); > > > > > + wait_for_completion_timeout(&data->completion, HZ / 10); > > > > No error condition check? > > > This is only a short drain to make sure any pending IRQ has been > handled before we stop > the device and clear the interrupt line. A timeout here is not treated > as a failure and the device > is being shut down anyway, so I intentionally do not check the return value. Based on the above experience, you know how to improve this. ... > > > + data->xshut_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > > + if (IS_ERR(data->xshut_gpio)) > > > + return dev_err_probe(dev, PTR_ERR(data->xshut_gpio), > > > + "Cannot get reset GPIO\n"); > > > > If it's a reset (semantically), use reset-gpio driver and reset APIs here. > > > The XSHUT pin is a shutdown/enable pin rather than a true reset line, > and the existing shared st,vl53l0x.yaml > binding already defines it as reset-gpios. So I am keeping the GPIO > consumer API here. And? reset-gpio driver is exactly for those cases. Please, move your driver code accordingly. > Sending v4 with all other fixes from your review, shortly. Please, take care of the comments first. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-13 9:35 ` Andy Shevchenko @ 2026-03-13 10:55 ` Sirat 0 siblings, 0 replies; 16+ messages in thread From: Sirat @ 2026-03-13 10:55 UTC (permalink / raw) To: Andy Shevchenko Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Fri, Mar 13, 2026 at 3:35 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Thu, Mar 12, 2026 at 11:37:26PM +0600, Sirat wrote: > > On Thu, Mar 12, 2026 at 8:58 PM Andy Shevchenko > > <andriy.shevchenko@intel.com> wrote: > > > On Thu, Mar 12, 2026 at 04:40:37AM +0600, Siratul Islam wrote: > > ... > > > > > > If it's a reset (semantically), use reset-gpio driver and reset APIs here. > > > > > The XSHUT pin is a shutdown/enable pin rather than a true reset line, > > and the existing shared st,vl53l0x.yaml > > binding already defines it as reset-gpios. So I am keeping the GPIO > > consumer API here. > > And? reset-gpio driver is exactly for those cases. Please, move your driver > code accordingly. > Blame is on me for not realizing that reset controller now also supports "reset-gpios", my apologies. My original hesitation was due to not willing to modify the binding to add "resets" I will switch to the reset controller. > > Thanks, Sirat ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-12 17:37 ` Sirat 2026-03-13 9:35 ` Andy Shevchenko @ 2026-03-14 12:27 ` Jonathan Cameron 1 sibling, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2026-03-14 12:27 UTC (permalink / raw) To: Sirat Cc: Andy Shevchenko, linux-iio, devicetree, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, 12 Mar 2026 23:37:26 +0600 Sirat <email@sirat.me> wrote: > On Thu, Mar 12, 2026 at 8:58 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > > > On Thu, Mar 12, 2026 at 04:40:37AM +0600, Siratul Islam wrote: > > > Add support for the STMicroelectronics VL53L1X Time-of-Flight > > > ranging sensor with I2C interface. > > ... > > > > > +static const struct regmap_config vl53l1x_regmap_config = { > > > + .reg_bits = 16, > > > + .val_bits = 8, > > > > No cache? > > > The result registers change on every measurement, so caching doesn't > apply well here. > Will add .cache_type = REGCACHE_NONE with a comment to clarify. That's always the case, so more interesting is what registers don't change? Can just mark the result registers as volatile and get the advantages from caching the rest. Someone else may have commented on it already, but please slow down a bit on sending new versions. Typically 1 a week is a reasonable cadence unless things are very near to merge and you are being asked for minor final tweaks in which case one every few days is fine. Jonathan > > > +}; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-11 22:40 [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor Siratul Islam 2026-03-11 22:40 ` [PATCH v3 1/2] dt-bindings: iio: proximity: add " Siratul Islam 2026-03-11 22:40 ` [PATCH v3 2/2] iio: proximity: add driver for " Siratul Islam @ 2026-03-12 14:27 ` Andy Shevchenko 2026-03-12 15:12 ` Sirat 2 siblings, 1 reply; 16+ messages in thread From: Andy Shevchenko @ 2026-03-12 14:27 UTC (permalink / raw) To: Siratul Islam Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: > This series adds support for the STMicroelectronics VL53L1X > Time-of-Flight ranging sensor. > > The VL53L1X is a ToF laser-ranging sensor with I2C interface, > capable of measuring distances up to 4 meters. The driver > supports both interrupt-driven and polled operation. > > Why a separate driver is needed (instead of extending vl53l0x-i2c.c): > The VL53L1X is fundamentally different from the VL53L0X despite the > similar naming. Extending the existing driver would require rewriting > the majority of it. > Key differences include: > - A different register map (16-bit addresses vs. 8-bit addresses). > - Requires a 91-byte firmware configuration blob to be loaded at boot. > - Requires a VHV calibration cycle. > - Has distance mode and timing budget configurations. > - Uses the regmap API rather than raw i2c_smbus calls. > > I also reviewed other drivers in drivers/iio/proximity/ and can > confirm this IP block does not appear to be shared by any other > existing driver. > --- > Changes in v3: > - Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof). > - Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name. But why is it "reset" in the bindings? If it's a power rail or enable pin, why not call it as a such in the bindings? The connection on PCB level is different story. > - Replace manual polling loops in chip_init and read_proximity with regmap_read_poll_timeout(). > - Remove irq_get_trigger_type() and pass IRQF_NO_THREAD directly (per Andy). > - Drop struct i2c_client from private data and store irq as int instead. Derive struct device from regmap where needed. > - Add dev_err_probe() to first devm_request_irq() error path. > - Replace linux/device.h with linux/dev_printk.h, add linux/array_size.h, linux/err.h, linux/types.h (per Andy). > - Use USEC_PER_MSEC for poll timeouts. > - Remove unnecessary casts. > - Divide long config blob to 8 values per line. > - Rename goto label to notify_and_clear_irq and drop unused dev_dbg. > - Add datasheet section reference for boot delay comment. > - vdd-supply no longer required in shared binding to avoid breaking existing st,vl53l0x devicetrees. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-12 14:27 ` [PATCH v3 0/2] " Andy Shevchenko @ 2026-03-12 15:12 ` Sirat 2026-03-12 15:23 ` Andy Shevchenko 2026-03-14 14:39 ` David Lechner 0 siblings, 2 replies; 16+ messages in thread From: Sirat @ 2026-03-12 15:12 UTC (permalink / raw) To: Andy Shevchenko Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: ... > > --- > > Changes in v3: > > - Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof). > > - Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name. > > But why is it "reset" in the bindings? If it's a power rail or enable pin, why > not call it as a such in the bindings? The connection on PCB level is different story. > In v2, I actually used a separate st,vl53l1x.yaml binding with `xshut-gpios` and a required `vdd-supply`. But Krzysztof pointed out that they share the same pins so the existing st,vl53l0x.yaml should suffice, using reset as the xshut pin. I think a choice has to be made here: 1. Either I use the st,vl53l0x.yaml binding with wrong pin name and optional vdd-supply to not break existing code. 2. Or use a separate binding st,vl53l1x.yaml with 1. correct pin name, and 2. require vdd-supply Krzysztof, do you have a preference on how we should handle this? > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-12 15:12 ` Sirat @ 2026-03-12 15:23 ` Andy Shevchenko 2026-03-14 14:39 ` David Lechner 1 sibling, 0 replies; 16+ messages in thread From: Andy Shevchenko @ 2026-03-12 15:23 UTC (permalink / raw) To: Sirat Cc: linux-iio, devicetree, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Thu, Mar 12, 2026 at 09:12:30PM +0600, Sirat wrote: > On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: ... > > > --- > > > Changes in v3: > > > - Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof). > > > - Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name. > > > > But why is it "reset" in the bindings? If it's a power rail or enable pin, why > > not call it as a such in the bindings? The connection on PCB level is different story. > > > In v2, I actually used a separate st,vl53l1x.yaml binding with > `xshut-gpios` and a required `vdd-supply`. > But Krzysztof pointed out that they share the same pins so the > existing st,vl53l0x.yaml should suffice, > using reset as the xshut pin. > I think a choice has to be made here: > 1. Either I use the st,vl53l0x.yaml binding with wrong pin name and > optional vdd-supply to not break existing code. > 2. Or use a separate binding st,vl53l1x.yaml with 1. correct pin name, > and 2. require vdd-supply > > Krzysztof, do you have a preference on how we should handle this? I am not a DT guy, I only know (?) that the DT should describe HW. In HW this pin seems like power enable which can roughly be considered as reset. So from SW point of view it probably makes little to no difference. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-12 15:12 ` Sirat 2026-03-12 15:23 ` Andy Shevchenko @ 2026-03-14 14:39 ` David Lechner 2026-03-14 15:25 ` Sirat 1 sibling, 1 reply; 16+ messages in thread From: David Lechner @ 2026-03-14 14:39 UTC (permalink / raw) To: Sirat, Andy Shevchenko Cc: linux-iio, devicetree, jic23, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On 3/12/26 10:12 AM, Sirat wrote: > On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: >> >> On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: > ... >>> --- >>> Changes in v3: >>> - Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof). >>> - Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name. >> >> But why is it "reset" in the bindings? If it's a power rail or enable pin, why >> not call it as a such in the bindings? The connection on PCB level is different story. >> > In v2, I actually used a separate st,vl53l1x.yaml binding with > `xshut-gpios` and a required `vdd-supply`. > But Krzysztof pointed out that they share the same pins so the > existing st,vl53l0x.yaml should suffice, > using reset as the xshut pin. > > I think a choice has to be made here: > 1. Either I use the st,vl53l0x.yaml binding with wrong pin name and > optional vdd-supply to not break existing code. > 2. Or use a separate binding st,vl53l1x.yaml with 1. correct pin name, > and 2. require vdd-supply We can modify the existing binding to make the supply required based on the compatible. allOf: # Technically supply is required to power device, but we keep it # optional for "st,vl53l0x" for backwards compatibility. - if: not: properties: compatible: const: "st,vl53l0x" then: required: vdd-supply And we can add a description to reset-gpios to explain that it is actually the XSHUT pin. > > Krzysztof, do you have a preference on how we should handle this? >> >> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-14 14:39 ` David Lechner @ 2026-03-14 15:25 ` Sirat 2026-03-15 18:57 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Sirat @ 2026-03-14 15:25 UTC (permalink / raw) To: David Lechner Cc: Andy Shevchenko, linux-iio, devicetree, jic23, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Sat, Mar 14, 2026 at 8:39 PM David Lechner <dlechner@baylibre.com> wrote: > > On 3/12/26 10:12 AM, Sirat wrote: > > On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko > > <andriy.shevchenko@intel.com> wrote: > >> > >> On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: > > ... > >>> --- > >>> Changes in v3: > >>> - Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof). > >>> - Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name. > >> > > We can modify the existing binding to make the supply required based > on the compatible. > > allOf: > # Technically supply is required to power device, but we keep it > # optional for "st,vl53l0x" for backwards compatibility. > - if: > not: > properties: > compatible: > const: "st,vl53l0x" > then: > required: > vdd-supply > > > And we can add a description to reset-gpios to explain that it is actually the > XSHUT pin. > This will solve the problem, but I'm concerned about someone not following the commit history looking at the code. Logically, it doesn't make sense as both the devices require the supply. In fact it may give the impression that the vl53l0x explicitly doesn't need power. We are having to deal with too much just to save us from writing a new file. That said, I can still implement this if it's still the prefered way to solve our problem. Or leave it optional as it is. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-14 15:25 ` Sirat @ 2026-03-15 18:57 ` Jonathan Cameron 2026-03-15 23:20 ` Sirat 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2026-03-15 18:57 UTC (permalink / raw) To: Sirat Cc: David Lechner, Andy Shevchenko, linux-iio, devicetree, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Sat, 14 Mar 2026 21:25:44 +0600 Sirat <email@sirat.me> wrote: > On Sat, Mar 14, 2026 at 8:39 PM David Lechner <dlechner@baylibre.com> wrote: > > > > On 3/12/26 10:12 AM, Sirat wrote: > > > On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko > > > <andriy.shevchenko@intel.com> wrote: > > >> > > >> On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: > > > ... > > >>> --- > > >>> Changes in v3: > > >>> - Merge DT binding into existing st,vl53l0x.yaml (per Krzysztof). > > >>> - Use "reset-gpios" in the binding but xshut_gpio in the driver since that's the actual pin name. > > >> > > > > We can modify the existing binding to make the supply required based > > on the compatible. > > > > allOf: > > # Technically supply is required to power device, but we keep it > > # optional for "st,vl53l0x" for backwards compatibility. > > - if: > > not: > > properties: > > compatible: > > const: "st,vl53l0x" > > then: > > required: > > vdd-supply > > > > > > And we can add a description to reset-gpios to explain that it is actually the > > XSHUT pin. > > > This will solve the problem, but I'm concerned about someone not > following the commit history looking at the code. > Logically, it doesn't make sense as both the devices require the > supply. In fact it may give the impression that > the vl53l0x explicitly doesn't need power. We are having to deal with > too much just to save us from writing a new file. > > That said, I can still implement this if it's still the prefered way > to solve our problem. Or leave it optional as it is. > > Can make it required in the binding. Just explain clearly why it should be there. The driver should carry on coping without it though (and leave a comment so we don't break that in the future). Jonathan > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-15 18:57 ` Jonathan Cameron @ 2026-03-15 23:20 ` Sirat 2026-03-22 11:57 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Sirat @ 2026-03-15 23:20 UTC (permalink / raw) To: Jonathan Cameron Cc: David Lechner, Andy Shevchenko, linux-iio, devicetree, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Mon, Mar 16, 2026 at 12:57 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 14 Mar 2026 21:25:44 +0600 > Sirat <email@sirat.me> wrote: > > > On Sat, Mar 14, 2026 at 8:39 PM David Lechner <dlechner@baylibre.com> wrote: > > > > > > On 3/12/26 10:12 AM, Sirat wrote: > > > > On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko > > > > <andriy.shevchenko@intel.com> wrote: > > > >> > > > >> On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: > > > > ... > > > >>> --- ... > Can make it required in the binding. Just explain clearly why > it should be there. The driver should carry on coping without > it though (and leave a comment so we don't break that in the future). > > Jonathan > > > > Just to make sure that I understand this correctly, I will - make vdd-supply required and add a description of what it is. - add a comment in the driver like "vdd-supply is required in the DT binding but we continue if missing to support older DTs." - also add reset-gpio description and update the example to include the vdd. I'll send v6 after a few days. Thanks, Sirat > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor 2026-03-15 23:20 ` Sirat @ 2026-03-22 11:57 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2026-03-22 11:57 UTC (permalink / raw) To: Sirat Cc: David Lechner, Andy Shevchenko, linux-iio, devicetree, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-kernel On Mon, 16 Mar 2026 05:20:29 +0600 Sirat <email@sirat.me> wrote: > On Mon, Mar 16, 2026 at 12:57 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Sat, 14 Mar 2026 21:25:44 +0600 > > Sirat <email@sirat.me> wrote: > > > > > On Sat, Mar 14, 2026 at 8:39 PM David Lechner <dlechner@baylibre.com> wrote: > > > > > > > > On 3/12/26 10:12 AM, Sirat wrote: > > > > > On Thu, Mar 12, 2026 at 8:28 PM Andy Shevchenko > > > > > <andriy.shevchenko@intel.com> wrote: > > > > >> > > > > >> On Thu, Mar 12, 2026 at 04:40:35AM +0600, Siratul Islam wrote: > > > > > ... > > > > >>> --- > ... > > Can make it required in the binding. Just explain clearly why > > it should be there. The driver should carry on coping without > > it though (and leave a comment so we don't break that in the future). > > > > Jonathan > > > > > > > > Just to make sure that I understand this correctly, > > I will > - make vdd-supply required and add a description of what it is. > - add a comment in the driver like "vdd-supply is required in the DT > binding but we continue if missing to support older DTs." > - also add reset-gpio description and update the example to include the vdd. > > I'll send v6 after a few days. > Ah sorry. Took me too long to catch up with emails. Bit late now but for the record, what we were aiming for was: Make it required and add a note to the commit message to say why the requirement should always have been there. Devices tend not to work with no power. The driver fallback stuff to provide a stub powersupply is very standard so no specific need to call that out. Jonathan > Thanks, > Sirat > > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-03-22 11:57 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-11 22:40 [PATCH v3 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor Siratul Islam 2026-03-11 22:40 ` [PATCH v3 1/2] dt-bindings: iio: proximity: add " Siratul Islam 2026-03-11 22:40 ` [PATCH v3 2/2] iio: proximity: add driver for " Siratul Islam 2026-03-12 14:58 ` Andy Shevchenko 2026-03-12 17:37 ` Sirat 2026-03-13 9:35 ` Andy Shevchenko 2026-03-13 10:55 ` Sirat 2026-03-14 12:27 ` Jonathan Cameron 2026-03-12 14:27 ` [PATCH v3 0/2] " Andy Shevchenko 2026-03-12 15:12 ` Sirat 2026-03-12 15:23 ` Andy Shevchenko 2026-03-14 14:39 ` David Lechner 2026-03-14 15:25 ` Sirat 2026-03-15 18:57 ` Jonathan Cameron 2026-03-15 23:20 ` Sirat 2026-03-22 11:57 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox