[PATCH v7 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[PATCH v7 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[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:
vdd-supply is now required. The VL53L0X required a physical
regulator, but it did not specify the requirement in the binding.
It should have always been there. The VL53L1X also requires it.
Hence I added the requirement.

---
Changes in v7:
- Add VL53L1X_REG_ prefix for register defines.
- Use iio_push_to_buffers_with_ts().
- Use continuous ranging and drop postenable/predisable.
- Add comments for XSHUT reset, default config values.
- Drop vdd-supply description in binding and comment in driver.
- Add ABI rationale for vdd-supply to commit message (per Krzysztof, Jonathan).


Changes in v6:
- Make vdd-supply required. Add descriptions to vdd-supply and reset-gpios (per Jonathan).
- Check reset_control_deassert() return value (per Andy).
- Use volatile table for result registers and caching for the rest. Also mark write-only registers.


Changes in v5:
- Patch 1: No changes. Collected Reviewed-by tag from Krzysztof Kozlowski.
- Switch XSHUT pin handling from the GPIO consumer API to the Reset controller API.
- Drop <linux/gpio/consumer.h> and include <linux/reset.h>.


Changes in v4:
- Move i2c_check_functionality() to the top of probe before allocations.
- Use struct u32_fract for oscillator correction math.
- Explicitly set .cache_type = REGCACHE_NONE.
- Document intentional predisable drain
- Use dev_err_probe() for firmware boot timeout and remove duplicate IRQ request error message.
- Use sizeof() in regmap bulk read/writes instead of hardcoded lengths.
- Add missing headers (<linux/math.h>, <linux/time.h>, <asm/byteorder.h>).
- Various formatting cleanups per Andy's review.


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 v6: https://lore.kernel.org/linux-iio/20260319190738.151614-1-email@sirat.me
Link to v5: https://lore.kernel.org/linux-iio/20260313113737.151881-1-email@sirat.me
Link to v4: https://lore.kernel.org/linux-iio/20260312183721.40482-1-email@sirat.me
Link to v3: https://lore.kernel.org/linux-iio/20260311224044.21480-1-email@sirat.me
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    |  13 +-
 MAINTAINERS                                   |   7 +
 drivers/iio/proximity/Kconfig                 |  15 +
 drivers/iio/proximity/Makefile                |   1 +
 drivers/iio/proximity/vl53l1x-i2c.c           | 795 ++++++++++++++++++
 5 files changed, 828 insertions(+), 3 deletions(-)
 create mode 100644 drivers/iio/proximity/vl53l1x-i2c.c

--
2.53.0

[PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Siratul Islam]

Add device tree binding documentation for the STMicroelectronics
VL53L1X Time-of-Flight ranging sensor connected via I2C.

Make vdd-supply required. The device requires power to operate
and the property should have been required from the start.

Signed-off-by: Siratul Islam <email@sirat.me>
---
 .../bindings/iio/proximity/st,vl53l0x.yaml          | 13 ++++++++++---
 MAINTAINERS                                         |  6 ++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml b/Documentation/devicetree/bindings/iio/proximity/st,vl53l0x.yaml
index 322befc41de6..09102310abc3 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
@@ -21,12 +24,15 @@ properties:
 
   reset-gpios:
     maxItems: 1
+    description:
+      Phandle to the XSHUT GPIO. Used for hardware reset.
 
   vdd-supply: true
 
 required:
   - compatible
   - reg
+  - vdd-supply
 
 additionalProperties: false
 
@@ -38,8 +44,9 @@ examples:
         #size-cells = <0>;
 
         proximity@29 {
-            compatible = "st,vl53l0x";
+            compatible = "st,vl53l1x";
             reg = <0x29>;
+            vdd-supply = <&reg_3v3>;
             interrupt-parent = <&gpio>;
             interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
         };
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

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Krzysztof Kozlowski]

On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:
> Add device tree binding documentation for the STMicroelectronics
> VL53L1X Time-of-Flight ranging sensor connected via I2C.
> 
> Make vdd-supply required. The device requires power to operate
> and the property should have been required from the start.

That's ABI break and device for many years was working fine, so this
should not be changed.

Best regards,
Krzysztof

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Sirat]

On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:
> > Add device tree binding documentation for the STMicroelectronics
> > VL53L1X Time-of-Flight ranging sensor connected via I2C.
> >
> > Make vdd-supply required. The device requires power to operate
> > and the property should have been required from the start.
>
> That's ABI break and device for many years was working fine, so this
> should not be changed.
>
Jonathan and David asked that vdd-supply be made required. I feel like
there is a conflict here that I am not able to resolve myself.

What I think about it is the binding does not correctly describe the
hardware and we should consider this a bug and fix it.
The driver worked because of a fallback mechanism (dummy/fake
regulator) and not because power was optional.

But I am willing to follow whatever consensus is agreed upon.

Thanks,
Sirat

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Krzysztof Kozlowski]

On 25/03/2026 09:48, Sirat wrote:
> On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:
>>> Add device tree binding documentation for the STMicroelectronics
>>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
>>>
>>> Make vdd-supply required. The device requires power to operate
>>> and the property should have been required from the start.
>>
>> That's ABI break and device for many years was working fine, so this
>> should not be changed.
>>
> Jonathan and David asked that vdd-supply be made required. I feel like
> there is a conflict here that I am not able to resolve myself.
> 
> What I think about it is the binding does not correctly describe the
> hardware and we should consider this a bug and fix it.
> The driver worked because of a fallback mechanism (dummy/fake
> regulator) and not because power was optional.
> 


I looked at v6 and v5 and I do not see such comment for binding that
existing device should change ABI. Can you point me to it?

Best regards,
Krzysztof

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Sirat]

On Wed, Mar 25, 2026 at 2:58 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 25/03/2026 09:48, Sirat wrote:
> > On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:
> >>> Add device tree binding documentation for the STMicroelectronics
> >>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
> >>>
> >>> Make vdd-supply required. The device requires power to operate
> >>> and the property should have been required from the start.
> >>
> >> That's ABI break and device for many years was working fine, so this
> >> should not be changed.
> >>
> > Jonathan and David asked that vdd-supply be made required. I feel like
> > there is a conflict here that I am not able to resolve myself.
> >
> > What I think about it is the binding does not correctly describe the
> > hardware and we should consider this a bug and fix it.
> > The driver worked because of a fallback mechanism (dummy/fake
> > regulator) and not because power was optional.
> >
>
>
> I looked at v6 and v5 and I do not see such comment for binding that
> existing device should change ABI. Can you point me to it?
>
"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." - Jonathan (v3:
https://lore.kernel.org/linux-iio/20260322115704.10b2e0d4@jic23-huawei)

"No, bindings should not depend on driver implementation." - David
(When I asked if I should  drop the hard requirement in the binding,
v6: https://lore.kernel.org/linux-iio/55e92148-b5de-4fb8-af0b-9476235341bc@baylibre.com/)

"From the point of view of the devicetree, it doesn't matter what the
driver does. It matters that the chip can't work without power. ;-)" -
David (v1: https://lore.kernel.org/linux-iio/d0ec6a2f-6d30-4774-8950-15dd3c4b020b@baylibre.com)

I'm not sure if this is the correct way to quote. But I have added the links.

Thanks,
Sirat

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Jonathan Cameron]

On Wed, 25 Mar 2026 15:18:05 +0600
Sirat <email@sirat.me> wrote:

> On Wed, Mar 25, 2026 at 2:58 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 25/03/2026 09:48, Sirat wrote:  
> > > On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:  
> > >>
> > >> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:  
> > >>> Add device tree binding documentation for the STMicroelectronics
> > >>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
> > >>>
> > >>> Make vdd-supply required. The device requires power to operate
> > >>> and the property should have been required from the start.  
> > >>
> > >> That's ABI break and device for many years was working fine, so this
> > >> should not be changed.
> > >>  
> > > Jonathan and David asked that vdd-supply be made required. I feel like
> > > there is a conflict here that I am not able to resolve myself.
> > >
> > > What I think about it is the binding does not correctly describe the
> > > hardware and we should consider this a bug and fix it.
> > > The driver worked because of a fallback mechanism (dummy/fake
> > > regulator) and not because power was optional.
> > >  
> >
> >
> > I looked at v6 and v5 and I do not see such comment for binding that
> > existing device should change ABI. Can you point me to it?
> >  
> "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." - Jonathan (v3:
> https://lore.kernel.org/linux-iio/20260322115704.10b2e0d4@jic23-huawei)
> 
> "No, bindings should not depend on driver implementation." - David
> (When I asked if I should  drop the hard requirement in the binding,
> v6: https://lore.kernel.org/linux-iio/55e92148-b5de-4fb8-af0b-9476235341bc@baylibre.com/)
> 
> "From the point of view of the devicetree, it doesn't matter what the
> driver does. It matters that the chip can't work without power. ;-)" -
> David (v1: https://lore.kernel.org/linux-iio/d0ec6a2f-6d30-4774-8950-15dd3c4b020b@baylibre.com)
> 
> I'm not sure if this is the correct way to quote. But I have added the links.

This came up a few years back - though I doubt I can track down the
exact discussion however.

From a Linux point of view we are breaking binding checks only if the
supply (that should always have been there as chips tend not to work
well without power) is not present.  We absolutely have to
keep the driver running whether or not the supply is specified.
Do other DT users provide such a constraint? I've no idea.

If the DT maintainer preference is leave it not required (perhaps
with a comment saying new users of the binding should supply it)
then that's fine by me. I'll keep it in mind for future similar changes.

Thanks,

Jonathan

> 
> Thanks,
> Sirat
> 
> 

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Krzysztof Kozlowski]

On 25/03/2026 14:38, Jonathan Cameron wrote:
> On Wed, 25 Mar 2026 15:18:05 +0600
> Sirat <email@sirat.me> wrote:
> 
>> On Wed, Mar 25, 2026 at 2:58 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 25/03/2026 09:48, Sirat wrote:  
>>>> On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:  
>>>>>
>>>>> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:  
>>>>>> Add device tree binding documentation for the STMicroelectronics
>>>>>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
>>>>>>
>>>>>> Make vdd-supply required. The device requires power to operate
>>>>>> and the property should have been required from the start.  
>>>>>
>>>>> That's ABI break and device for many years was working fine, so this
>>>>> should not be changed.
>>>>>  
>>>> Jonathan and David asked that vdd-supply be made required. I feel like
>>>> there is a conflict here that I am not able to resolve myself.
>>>>
>>>> What I think about it is the binding does not correctly describe the
>>>> hardware and we should consider this a bug and fix it.
>>>> The driver worked because of a fallback mechanism (dummy/fake
>>>> regulator) and not because power was optional.
>>>>  
>>>
>>>
>>> I looked at v6 and v5 and I do not see such comment for binding that
>>> existing device should change ABI. Can you point me to it?
>>>  
>> "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." - Jonathan (v3:
>> https://lore.kernel.org/linux-iio/20260322115704.10b2e0d4@jic23-huawei)
>>
>> "No, bindings should not depend on driver implementation." - David
>> (When I asked if I should  drop the hard requirement in the binding,
>> v6: https://lore.kernel.org/linux-iio/55e92148-b5de-4fb8-af0b-9476235341bc@baylibre.com/)
>>
>> "From the point of view of the devicetree, it doesn't matter what the
>> driver does. It matters that the chip can't work without power. ;-)" -
>> David (v1: https://lore.kernel.org/linux-iio/d0ec6a2f-6d30-4774-8950-15dd3c4b020b@baylibre.com)
>>
>> I'm not sure if this is the correct way to quote. But I have added the links.
> 
> This came up a few years back - though I doubt I can track down the
> exact discussion however.
> 
> From a Linux point of view we are breaking binding checks only if the
> supply (that should always have been there as chips tend not to work
> well without power) is not present.  We absolutely have to
> keep the driver running whether or not the supply is specified.
> Do other DT users provide such a constraint? I've no idea.
> 
> If the DT maintainer preference is leave it not required (perhaps
> with a comment saying new users of the binding should supply it)
> then that's fine by me. I'll keep it in mind for future similar changes.

If this was other ABI, e.g. clock, then answer would be - do not require
it, because that's ABI break. Therefore I would stick to that also to
regulators. Once Rob also expressed such thoughts, although noting that
it is not that big deal.

New device in this binding of course should require the supply.

Best regards,
Krzysztof

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Jonathan Cameron]

On Wed, 25 Mar 2026 14:44:13 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 25/03/2026 14:38, Jonathan Cameron wrote:
> > On Wed, 25 Mar 2026 15:18:05 +0600
> > Sirat <email@sirat.me> wrote:
> >   
> >> On Wed, Mar 25, 2026 at 2:58 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:  
> >>>
> >>> On 25/03/2026 09:48, Sirat wrote:    
> >>>> On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:    
> >>>>>
> >>>>> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:    
> >>>>>> Add device tree binding documentation for the STMicroelectronics
> >>>>>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
> >>>>>>
> >>>>>> Make vdd-supply required. The device requires power to operate
> >>>>>> and the property should have been required from the start.    
> >>>>>
> >>>>> That's ABI break and device for many years was working fine, so this
> >>>>> should not be changed.
> >>>>>    
> >>>> Jonathan and David asked that vdd-supply be made required. I feel like
> >>>> there is a conflict here that I am not able to resolve myself.
> >>>>
> >>>> What I think about it is the binding does not correctly describe the
> >>>> hardware and we should consider this a bug and fix it.
> >>>> The driver worked because of a fallback mechanism (dummy/fake
> >>>> regulator) and not because power was optional.
> >>>>    
> >>>
> >>>
> >>> I looked at v6 and v5 and I do not see such comment for binding that
> >>> existing device should change ABI. Can you point me to it?
> >>>    
> >> "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." - Jonathan (v3:
> >> https://lore.kernel.org/linux-iio/20260322115704.10b2e0d4@jic23-huawei)
> >>
> >> "No, bindings should not depend on driver implementation." - David
> >> (When I asked if I should  drop the hard requirement in the binding,
> >> v6: https://lore.kernel.org/linux-iio/55e92148-b5de-4fb8-af0b-9476235341bc@baylibre.com/)
> >>
> >> "From the point of view of the devicetree, it doesn't matter what the
> >> driver does. It matters that the chip can't work without power. ;-)" -
> >> David (v1: https://lore.kernel.org/linux-iio/d0ec6a2f-6d30-4774-8950-15dd3c4b020b@baylibre.com)
> >>
> >> I'm not sure if this is the correct way to quote. But I have added the links.  
> > 
> > This came up a few years back - though I doubt I can track down the
> > exact discussion however.
> > 
> > From a Linux point of view we are breaking binding checks only if the
> > supply (that should always have been there as chips tend not to work
> > well without power) is not present.  We absolutely have to
> > keep the driver running whether or not the supply is specified.
> > Do other DT users provide such a constraint? I've no idea.
> > 
> > If the DT maintainer preference is leave it not required (perhaps
> > with a comment saying new users of the binding should supply it)
> > then that's fine by me. I'll keep it in mind for future similar changes.  
> 
> If this was other ABI, e.g. clock, then answer would be - do not require
> it, because that's ABI break. Therefore I would stick to that also to
> regulators. Once Rob also expressed such thoughts, although noting that
> it is not that big deal.
> 
> New device in this binding of course should require the supply.
Seems my memory was less than perfect on this : 
https://lore.kernel.org/linux-iio/20241119140409.GA1093349-robh@kernel.org/#t

Rob expressed that we are inconsistent on this, but he'd rather not
have regulators as a special case.

So let's only make this required for the new device. 

Thanks,

J

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Sirat]

On Wed, Mar 25, 2026 at 8:06 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Wed, 25 Mar 2026 14:44:13 +0100
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > On 25/03/2026 14:38, Jonathan Cameron wrote:
> > > On Wed, 25 Mar 2026 15:18:05 +0600
> > > Sirat <email@sirat.me> wrote:
> > >
> > >> On Wed, Mar 25, 2026 at 2:58 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >>>
> > >>> On 25/03/2026 09:48, Sirat wrote:
> > >>>> On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >>>>>
> > >>>>> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:
> > >>>>>> Add device tree binding documentation for the STMicroelectronics
> > >>>>>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
> > >>>>>>
> > >>>>>> Make vdd-supply required. The device requires power to operate
> > >>>>>> and the property should have been required from the start.
> > >>>>>
> > >>>>> That's ABI break and device for many years was working fine, so this
> > >>>>> should not be changed.
> > >>>>>
> > >>>> Jonathan and David asked that vdd-supply be made required. I feel like
> > >>>> there is a conflict here that I am not able to resolve myself.
> > >>>>
> > >>>> What I think about it is the binding does not correctly describe the
> > >>>> hardware and we should consider this a bug and fix it.
> > >>>> The driver worked because of a fallback mechanism (dummy/fake
> > >>>> regulator) and not because power was optional.
> > >>>>
> > >>>
> > >>>
> > >>> I looked at v6 and v5 and I do not see such comment for binding that
> > >>> existing device should change ABI. Can you point me to it?
> > >>>
> > >> "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." - Jonathan (v3:
> > >> https://lore.kernel.org/linux-iio/20260322115704.10b2e0d4@jic23-huawei)
> > >>
> > >> "No, bindings should not depend on driver implementation." - David
> > >> (When I asked if I should  drop the hard requirement in the binding,
> > >> v6: https://lore.kernel.org/linux-iio/55e92148-b5de-4fb8-af0b-9476235341bc@baylibre.com/)
> > >>
> > >> "From the point of view of the devicetree, it doesn't matter what the
> > >> driver does. It matters that the chip can't work without power. ;-)" -
> > >> David (v1: https://lore.kernel.org/linux-iio/d0ec6a2f-6d30-4774-8950-15dd3c4b020b@baylibre.com)
> > >>
> > >> I'm not sure if this is the correct way to quote. But I have added the links.
> > >
> > > This came up a few years back - though I doubt I can track down the
> > > exact discussion however.
> > >
> > > From a Linux point of view we are breaking binding checks only if the
> > > supply (that should always have been there as chips tend not to work
> > > well without power) is not present.  We absolutely have to
> > > keep the driver running whether or not the supply is specified.
> > > Do other DT users provide such a constraint? I've no idea.
> > >
> > > If the DT maintainer preference is leave it not required (perhaps
> > > with a comment saying new users of the binding should supply it)
> > > then that's fine by me. I'll keep it in mind for future similar changes.
> >
> > If this was other ABI, e.g. clock, then answer would be - do not require
> > it, because that's ABI break. Therefore I would stick to that also to
> > regulators. Once Rob also expressed such thoughts, although noting that
> > it is not that big deal.
> >
> > New device in this binding of course should require the supply.
> Seems my memory was less than perfect on this :
> https://lore.kernel.org/linux-iio/20241119140409.GA1093349-robh@kernel.org/#t
>
> Rob expressed that we are inconsistent on this, but he'd rather not
> have regulators as a special case.
>
> So let's only make this required for the new device.
>
So since it is a new device, how do I require this? should we split to
a new binding (like I had in v1) and make that required. That would
also allow us to correctly name the xshut pin.

Or do we do the "allOf:" exclusion? In that case, I think it wouldn't
make sense to someone reading the binding without the context of
commit history, as it would imply one of the devices explicitly
doesn't need power.

I'm willing to do whichever is prefered tough and move this forward.
>
>
Thanks,

Sirat

Re: [PATCH v7 1/2] dt-bindings: iio: proximity: add ST VL53L1X ToF sensor

[Jonathan Cameron]

On Wed, 25 Mar 2026 20:38:48 +0600
Sirat <email@sirat.me> wrote:

> On Wed, Mar 25, 2026 at 8:06 PM Jonathan Cameron
> <jonathan.cameron@huawei.com> wrote:
> >
> > On Wed, 25 Mar 2026 14:44:13 +0100
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >  
> > > On 25/03/2026 14:38, Jonathan Cameron wrote:  
> > > > On Wed, 25 Mar 2026 15:18:05 +0600
> > > > Sirat <email@sirat.me> wrote:
> > > >  
> > > >> On Wed, Mar 25, 2026 at 2:58 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:  
> > > >>>
> > > >>> On 25/03/2026 09:48, Sirat wrote:  
> > > >>>> On Wed, Mar 25, 2026 at 2:05 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:  
> > > >>>>>
> > > >>>>> On Wed, Mar 25, 2026 at 12:32:22PM +0600, Siratul Islam wrote:  
> > > >>>>>> Add device tree binding documentation for the STMicroelectronics
> > > >>>>>> VL53L1X Time-of-Flight ranging sensor connected via I2C.
> > > >>>>>>
> > > >>>>>> Make vdd-supply required. The device requires power to operate
> > > >>>>>> and the property should have been required from the start.  
> > > >>>>>
> > > >>>>> That's ABI break and device for many years was working fine, so this
> > > >>>>> should not be changed.
> > > >>>>>  
> > > >>>> Jonathan and David asked that vdd-supply be made required. I feel like
> > > >>>> there is a conflict here that I am not able to resolve myself.
> > > >>>>
> > > >>>> What I think about it is the binding does not correctly describe the
> > > >>>> hardware and we should consider this a bug and fix it.
> > > >>>> The driver worked because of a fallback mechanism (dummy/fake
> > > >>>> regulator) and not because power was optional.
> > > >>>>  
> > > >>>
> > > >>>
> > > >>> I looked at v6 and v5 and I do not see such comment for binding that
> > > >>> existing device should change ABI. Can you point me to it?
> > > >>>  
> > > >> "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." - Jonathan (v3:
> > > >> https://lore.kernel.org/linux-iio/20260322115704.10b2e0d4@jic23-huawei)
> > > >>
> > > >> "No, bindings should not depend on driver implementation." - David
> > > >> (When I asked if I should  drop the hard requirement in the binding,
> > > >> v6: https://lore.kernel.org/linux-iio/55e92148-b5de-4fb8-af0b-9476235341bc@baylibre.com/)
> > > >>
> > > >> "From the point of view of the devicetree, it doesn't matter what the
> > > >> driver does. It matters that the chip can't work without power. ;-)" -
> > > >> David (v1: https://lore.kernel.org/linux-iio/d0ec6a2f-6d30-4774-8950-15dd3c4b020b@baylibre.com)
> > > >>
> > > >> I'm not sure if this is the correct way to quote. But I have added the links.  
> > > >
> > > > This came up a few years back - though I doubt I can track down the
> > > > exact discussion however.
> > > >
> > > > From a Linux point of view we are breaking binding checks only if the
> > > > supply (that should always have been there as chips tend not to work
> > > > well without power) is not present.  We absolutely have to
> > > > keep the driver running whether or not the supply is specified.
> > > > Do other DT users provide such a constraint? I've no idea.
> > > >
> > > > If the DT maintainer preference is leave it not required (perhaps
> > > > with a comment saying new users of the binding should supply it)
> > > > then that's fine by me. I'll keep it in mind for future similar changes.  
> > >
> > > If this was other ABI, e.g. clock, then answer would be - do not require
> > > it, because that's ABI break. Therefore I would stick to that also to
> > > regulators. Once Rob also expressed such thoughts, although noting that
> > > it is not that big deal.
> > >
> > > New device in this binding of course should require the supply.  
> > Seems my memory was less than perfect on this :
> > https://lore.kernel.org/linux-iio/20241119140409.GA1093349-robh@kernel.org/#t
> >
> > Rob expressed that we are inconsistent on this, but he'd rather not
> > have regulators as a special case.
> >
> > So let's only make this required for the new device.
> >  
> So since it is a new device, how do I require this? should we split to
> a new binding (like I had in v1) and make that required. That would
> also allow us to correctly name the xshut pin.
> 
> Or do we do the "allOf:" exclusion? In that case, I think it wouldn't
> make sense to someone reading the binding without the context of
> commit history, as it would imply one of the devices explicitly
> doesn't need power.
This + add a comment that it's only not required for other devices
for backwards compatibility reasons.

> 
> I'm willing to do whichever is prefered tough and move this forward.
> >
> >  
> Thanks,
> 
> Sirat

[PATCH v7 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Siratul Islam]

Add support for the STMicroelectronics VL53L1X Time-of-Flight
ranging sensor with I2C interface.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Siratul Islam <email@sirat.me>
---
 MAINTAINERS                         |   1 +
 drivers/iio/proximity/Kconfig       |  15 +
 drivers/iio/proximity/Makefile      |   1 +
 drivers/iio/proximity/vl53l1x-i2c.c | 795 ++++++++++++++++++++++++++++
 4 files changed, 812 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..bb77fad2a1b3 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -244,6 +244,21 @@ 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
+	select RESET_CONTROLLER
+	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..085bff04f5d9
--- /dev/null
+++ b/drivers/iio/proximity/vl53l1x-i2c.c
@@ -0,0 +1,795 @@
+// 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/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/math.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/time.h>
+#include <linux/types.h>
+
+#include <asm/byteorder.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_REG_SOFT_RESET						0x0000
+#define VL53L1X_REG_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND		0x0008
+#define VL53L1X_REG_VHV_CONFIG__INIT					0x000B
+#define VL53L1X_REG_GPIO_HV_MUX__CTRL					0x0030
+#define VL53L1X_REG_GPIO__TIO_HV_STATUS					0x0031
+#define VL53L1X_REG_SYSTEM__INTERRUPT_CONFIG_GPIO			0x0046
+#define VL53L1X_REG_PHASECAL_CONFIG__TIMEOUT_MACROP			0x004B
+#define VL53L1X_REG_RANGE_CONFIG__TIMEOUT_MACROP_A			0x005E
+#define VL53L1X_REG_RANGE_CONFIG__VCSEL_PERIOD_A			0x0060
+#define VL53L1X_REG_RANGE_CONFIG__TIMEOUT_MACROP_B			0x0061
+#define VL53L1X_REG_RANGE_CONFIG__VCSEL_PERIOD_B			0x0063
+#define VL53L1X_REG_RANGE_CONFIG__VALID_PHASE_HIGH			0x0069
+#define VL53L1X_REG_SYSTEM__INTERMEASUREMENT_PERIOD			0x006C
+#define VL53L1X_REG_SD_CONFIG__WOI_SD0					0x0078
+#define VL53L1X_REG_SD_CONFIG__WOI_SD1					0x0079
+#define VL53L1X_REG_SD_CONFIG__INITIAL_PHASE_SD0			0x007A
+#define VL53L1X_REG_SD_CONFIG__INITIAL_PHASE_SD1			0x007B
+#define VL53L1X_REG_SYSTEM__INTERRUPT_CLEAR				0x0086
+#define VL53L1X_REG_SYSTEM__MODE_START					0x0087
+#define VL53L1X_REG_RESULT__RANGE_STATUS				0x0089
+#define VL53L1X_REG_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0	0x0096
+#define VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL				0x00DE
+#define VL53L1X_REG_FIRMWARE__SYSTEM_STATUS				0x00E5
+#define VL53L1X_REG_IDENTIFICATION__MODEL_ID				0x010F
+#define VL53L1X_REG_DEFAULT_CONFIG					0x002D
+
+#define VL53L1X_MODEL_ID_VAL		0xEACC
+
+#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
+
+#define VL53L1X_OSC_CALIBRATE_MASK	GENMASK(9, 0)
+
+/* Inter-measurement period uses PLL divider with 1.075 oscillator correction */
+static const struct u32_fract vl53l1x_osc_correction = {
+	.numerator = 1075,
+	.denominator = 1000,
+};
+
+enum vl53l1x_distance_mode {
+	VL53L1X_SHORT,
+	VL53L1X_LONG,
+};
+
+struct vl53l1x_data {
+	struct regmap *regmap;
+	struct completion completion;
+	struct regulator *vdd_supply;
+	struct reset_control *xshut_reset;
+	enum vl53l1x_distance_mode distance_mode;
+	u8 gpio_polarity;
+	int irq;
+};
+
+static const struct regmap_range vl53l1x_volatile_ranges[] = {
+	regmap_reg_range(VL53L1X_REG_GPIO__TIO_HV_STATUS,
+			 VL53L1X_REG_GPIO__TIO_HV_STATUS),
+	regmap_reg_range(VL53L1X_REG_RESULT__RANGE_STATUS,
+			 VL53L1X_REG_RESULT__RANGE_STATUS),
+	regmap_reg_range(VL53L1X_REG_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0,
+			 VL53L1X_REG_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0 + 1),
+	regmap_reg_range(VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL,
+			 VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL + 1),
+	regmap_reg_range(VL53L1X_REG_FIRMWARE__SYSTEM_STATUS,
+			 VL53L1X_REG_FIRMWARE__SYSTEM_STATUS),
+};
+
+static const struct regmap_access_table vl53l1x_volatile_table = {
+	.yes_ranges = vl53l1x_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(vl53l1x_volatile_ranges),
+};
+
+static const struct regmap_range vl53l1x_write_only_ranges[] = {
+	regmap_reg_range(VL53L1X_REG_SOFT_RESET, VL53L1X_REG_SOFT_RESET),
+	regmap_reg_range(VL53L1X_REG_SYSTEM__INTERRUPT_CLEAR,
+			 VL53L1X_REG_SYSTEM__MODE_START),
+};
+
+static const struct regmap_access_table vl53l1x_readable_table = {
+	.no_ranges = vl53l1x_write_only_ranges,
+	.n_no_ranges = ARRAY_SIZE(vl53l1x_write_only_ranges),
+};
+
+static const struct regmap_config vl53l1x_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	/* MODEL_ID is 16-bit. +1 covers the second byte at 0x0110 */
+	.max_register = VL53L1X_REG_IDENTIFICATION__MODEL_ID + 1,
+	.cache_type = REGCACHE_MAPLE,
+	.volatile_table = &vl53l1x_volatile_table,
+	.rd_table = &vl53l1x_readable_table,
+};
+
+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, sizeof(buf));
+	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, sizeof(buf));
+}
+
+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, sizeof(buf));
+}
+
+static int vl53l1x_clear_irq(struct vl53l1x_data *data)
+{
+	return regmap_write(data->regmap, VL53L1X_REG_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_REG_SYSTEM__MODE_START,
+			    VL53L1X_MODE_START_TIMED);
+}
+
+static int vl53l1x_stop_ranging(struct vl53l1x_data *data)
+{
+	return regmap_write(data->regmap, VL53L1X_REG_SYSTEM__MODE_START,
+			    VL53L1X_MODE_START_STOP);
+}
+
+/*
+ * Default configuration blob from ST's VL53L1X Ultra Lite Driver
+ * (STSW-IMG009).
+ */
+static const u8 vl53l1x_default_config[] = {
+	0x00, 0x00, 0x00, 0x01, 0x02, 0x00, 0x02, 0x08,	/* reg 0x2d..0x34 */
+	0x00, 0x08, 0x10, 0x01, 0x01, 0x00, 0x00, 0x00,	/* reg 0x35..0x3c */
+	0x00, 0xFF, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00,	/* reg 0x3d..0x44 */
+	0x00, 0x20, 0x0B, 0x00, 0x00, 0x02, 0x0A, 0x21,	/* reg 0x45..0x4c */
+	0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0xC8,	/* reg 0x4d..0x54 */
+	0x00, 0x00, 0x38, 0xFF, 0x01, 0x00, 0x08, 0x00,	/* reg 0x55..0x5c */
+	0x00, 0x01, 0xCC, 0x0F, 0x01, 0xF1, 0x0D, 0x01,	/* reg 0x5d..0x64 */
+	0x68, 0x00, 0x80, 0x08, 0xB8, 0x00, 0x00, 0x00,	/* reg 0x65..0x6c */
+	0x00, 0x0F, 0x89, 0x00, 0x00, 0x00, 0x00, 0x00,	/* reg 0x6d..0x74 */
+	0x00, 0x00, 0x01, 0x0F, 0x0D, 0x0E, 0x0E, 0x00,	/* reg 0x75..0x7c */
+	0x00, 0x02, 0xC7, 0xFF, 0x9B, 0x00, 0x00, 0x00,	/* reg 0x7d..0x84 */
+	0x01, 0x00, 0x00,				/* reg 0x85..0x87 */
+};
+
+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_reset) {
+		ret = regmap_write(data->regmap, VL53L1X_REG_SOFT_RESET, 0x00);
+		if (ret)
+			return ret;
+		fsleep(100); /* conservative reset pulse, no spec */
+
+		ret = regmap_write(data->regmap, VL53L1X_REG_SOFT_RESET, 0x01);
+		if (ret)
+			return ret;
+		fsleep(1000); /* conservative boot wait, no spec */
+	}
+
+	ret = regmap_read_poll_timeout(data->regmap,
+				       VL53L1X_REG_FIRMWARE__SYSTEM_STATUS, val,
+				       val & BIT(0),
+				       1 * USEC_PER_MSEC,
+				       100 * USEC_PER_MSEC);
+	if (ret)
+		return dev_err_probe(dev, ret, "firmware boot timeout\n");
+
+	ret = vl53l1x_read_u16(data, VL53L1X_REG_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 = regmap_bulk_write(data->regmap, VL53L1X_REG_DEFAULT_CONFIG,
+				vl53l1x_default_config,
+				sizeof(vl53l1x_default_config));
+	if (ret)
+		return ret;
+
+	ret = regmap_read(data->regmap, VL53L1X_REG_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 Ultra Lite Driver) */
+	ret = regmap_read_poll_timeout(data->regmap,
+				       VL53L1X_REG_GPIO__TIO_HV_STATUS, val,
+				       (val & 1) != 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_REG_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND,
+			   VL53L1X_VHV_LOOP_BOUND_TWO);
+	if (ret)
+		return ret;
+
+	return regmap_write(data->regmap, VL53L1X_REG_VHV_CONFIG__INIT, 0x00);
+}
+
+static const struct reg_sequence vl53l1x_mode_short[] = {
+	{ VL53L1X_REG_PHASECAL_CONFIG__TIMEOUT_MACROP,		0x14 },
+	{ VL53L1X_REG_RANGE_CONFIG__VCSEL_PERIOD_A,		0x07 },
+	{ VL53L1X_REG_RANGE_CONFIG__VCSEL_PERIOD_B,		0x05 },
+	{ VL53L1X_REG_RANGE_CONFIG__VALID_PHASE_HIGH,		0x38 },
+	{ VL53L1X_REG_SD_CONFIG__WOI_SD0,			0x07 },
+	{ VL53L1X_REG_SD_CONFIG__WOI_SD1,			0x05 },
+	{ VL53L1X_REG_SD_CONFIG__INITIAL_PHASE_SD0,		0x06 },
+	{ VL53L1X_REG_SD_CONFIG__INITIAL_PHASE_SD1,		0x06 },
+};
+
+static const struct reg_sequence vl53l1x_mode_long[] = {
+	{ VL53L1X_REG_PHASECAL_CONFIG__TIMEOUT_MACROP,		0x0A },
+	{ VL53L1X_REG_RANGE_CONFIG__VCSEL_PERIOD_A,		0x0F },
+	{ VL53L1X_REG_RANGE_CONFIG__VCSEL_PERIOD_B,		0x0D },
+	{ VL53L1X_REG_RANGE_CONFIG__VALID_PHASE_HIGH,		0xB8 },
+	{ VL53L1X_REG_SD_CONFIG__WOI_SD0,			0x0F },
+	{ VL53L1X_REG_SD_CONFIG__WOI_SD1,			0x0D },
+	{ VL53L1X_REG_SD_CONFIG__INITIAL_PHASE_SD0,		0x0E },
+	{ VL53L1X_REG_SD_CONFIG__INITIAL_PHASE_SD1,		0x0E },
+};
+
+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 Ultra Lite Driver.
+ */
+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_REG_RANGE_CONFIG__TIMEOUT_MACROP_A,
+				timeout_a);
+	if (ret)
+		return ret;
+
+	return vl53l1x_write_u16(data, VL53L1X_REG_RANGE_CONFIG__TIMEOUT_MACROP_B,
+				 timeout_b);
+}
+
+static int vl53l1x_set_inter_measurement_ms(struct vl53l1x_data *data,
+					    u16 period_ms)
+{
+	u16 osc_calibrate_val;
+	u16 clock_pll;
+	u32 inter_meas;
+	int ret;
+
+	ret = vl53l1x_read_u16(data, VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL,
+			       &osc_calibrate_val);
+	if (ret)
+		return ret;
+
+	clock_pll = osc_calibrate_val & VL53L1X_OSC_CALIBRATE_MASK;
+	inter_meas = (clock_pll * period_ms * vl53l1x_osc_correction.numerator) /
+		     vl53l1x_osc_correction.denominator;
+
+	return vl53l1x_write_u32(data,
+				 VL53L1X_REG_SYSTEM__INTERMEASUREMENT_PERIOD,
+				 inter_meas);
+}
+
+static int vl53l1x_read_proximity(struct vl53l1x_data *data, int *val)
+{
+	unsigned int range_status;
+	u16 distance;
+	int ret;
+
+	if (data->irq) {
+		reinit_completion(&data->completion);
+
+		ret = vl53l1x_clear_irq(data);
+		if (ret)
+			return ret;
+
+		if (!wait_for_completion_timeout(&data->completion, HZ))
+			return -ETIMEDOUT;
+	} else {
+		unsigned int rdy;
+
+		/* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
+		ret = regmap_read_poll_timeout(data->regmap,
+					       VL53L1X_REG_GPIO__TIO_HV_STATUS, rdy,
+					       (rdy & 1) != data->gpio_polarity,
+					       1 * USEC_PER_MSEC,
+					       1000 * USEC_PER_MSEC);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_read(data->regmap, VL53L1X_REG_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_REG_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_REG_RESULT__RANGE_STATUS,
+			  &range_status);
+	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_REG_RESULT__FINAL_CROSSTALK_CORRECTED_RANGE_MM_SD0,
+			       &scan.distance);
+	if (ret)
+		goto notify_and_clear_irq;
+
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(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 const struct iio_trigger_ops vl53l1x_trigger_ops = {
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static void vl53l1x_stop_ranging_action(void *priv)
+{
+	vl53l1x_stop_ranging(priv);
+}
+
+static void vl53l1x_power_off(void *priv)
+{
+	struct vl53l1x_data *data = priv;
+
+	reset_control_assert(data->xshut_reset);
+	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;
+
+	ret = reset_control_deassert(data->xshut_reset);
+	if (ret) {
+		regulator_disable(data->vdd_supply);
+		return ret;
+	}
+	/*
+	 * 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 ret;
+
+	ret = regmap_write(data->regmap, VL53L1X_REG_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;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_I2C_BLOCK |
+				     I2C_FUNC_SMBUS_BYTE_DATA))
+		return -EOPNOTSUPP;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->irq = client->irq;
+
+	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");
+
+	/*
+	 * XSHUT held low puts the chip in hardware standby. All register
+	 * state is lost on de-assert so this is functionally a reset.
+	 */
+	data->xshut_reset = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(data->xshut_reset))
+		return dev_err_probe(dev, PTR_ERR(data->xshut_reset),
+				     "Cannot get reset control\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;
+
+	/* 50 ms timing budget (per ST Ultra Lite Driver) */
+	ret = vl53l1x_set_timing_budget(data, 50);
+	if (ret)
+		return ret;
+
+	/* 50 ms inter-measurement period (per ST Ultra Lite Driver) */
+	ret = vl53l1x_set_inter_measurement_ms(data, 50);
+	if (ret)
+		return ret;
+
+	/*
+	 * The hardware only supports "autonomous" continuous ranging mode.
+	 * Start ranging here and leave it running for the lifetime of
+	 * the device. Both direct reads and the buffer path rely on this.
+	 */
+	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,
+						      NULL);
+		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

Re: [PATCH v7 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Jonathan Cameron]

On Wed, 25 Mar 2026 12:32:23 +0600
Siratul Islam <email@sirat.me> wrote:

> Add support for the STMicroelectronics VL53L1X Time-of-Flight
> ranging sensor with I2C interface.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Siratul Islam <email@sirat.me>

Hi Siratul,

I used this as a bit of an experiment as you'd +CC lkml and took a close
read of the feedback google's AI bot produced.
https://sashiko.dev/#/patchset/20260325063254.18062-1-email%40sirat.me

Be very careful when considering the output. There are some things
in here that are not true, or we don't generally defend against (like the
interrupt type related comments).

The iio_trigger_get() is (I think) a more general problem so ignore that
for your driver - will just be a tiny memory leak and stop the
module being easily unloaded.  I'll take a look at that one on a more general
basis. 

The regmap one is where these bots excel in that they sometimes go deeper
in analysis than a typical person focused on one driver.

The stuff on using standard devm_ wasn't from the bot and I think will make
things rather simpler.

Thanks,

Jonathan


> ---
>  MAINTAINERS                         |   1 +
>  drivers/iio/proximity/Kconfig       |  15 +
>  drivers/iio/proximity/Makefile      |   1 +
>  drivers/iio/proximity/vl53l1x-i2c.c | 795 ++++++++++++++++++++++++++++
>  4 files changed, 812 insertions(+)
>  create mode 100644 drivers/iio/proximity/vl53l1x-i2c.c
> 

> diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c
> new file mode 100644
> index 000000000000..085bff04f5d9
> --- /dev/null
> +++ b/drivers/iio/proximity/vl53l1x-i2c.c

> +static void vl53l1x_power_off(void *priv)

See below. I'm not seeing a reason we need to handle this
in a driver specific way.

> +{
> +	struct vl53l1x_data *data = priv;
> +
> +	reset_control_assert(data->xshut_reset);
> +	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;
> +
> +	ret = reset_control_deassert(data->xshut_reset);
> +	if (ret) {
> +		regulator_disable(data->vdd_supply);
> +		return ret;
> +	}
> +	/*
> +	 * 1.2 ms max boot duration.
> +	 * Datasheet Section 3.6 "Power up and boot sequence".
> +	 */
> +	fsleep(1200);
> +
> +	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;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_I2C_BLOCK |
> +				     I2C_FUNC_SMBUS_BYTE_DATA))
Sashiko had an interesting comment on this...
https://sashiko.dev/#/patchset/20260325063254.18062-1-email%40sirat.me
(note in general be careful with this tools feedback, it has a significant
false positive rate!)

"Does this check needlessly reject pure I2C adapters?
The driver configures regmap with reg_bits = 16, which forces regmap to use
raw I2C transfers requiring I2C_FUNC_I2C. Checking for SMBUS_READ_I2C_BLOCK
might prevent the driver from loading on pure I2C adapters that can perfectly
support the device via regmap."

By by reading the comment is actually wrong, however... It did made me look.
With regbits == 16 & valbits == 8
The regmap code checks for I2C_FUNC_SMBUS_I2C_BLOCK
(and if not present fails).

So the I2C_FUNC_SMBUS_BYTE_DATA seems unused and
a broader check on both read and write versions of I2C_BLOCK seems
appropriate.

However, not a lot of point in checking it at all given regmap does
so for us. So I'd drop this check.

If anyone is bored, we should probably take a look to see if there
are other redundant (or wrong) calls for this.

There is another point about trigger references that might be
correct (but not unique to this driver so ignore it).  That warrants
some investigation - I'll take a look in near future.

> +		return -EOPNOTSUPP;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	data->irq = client->irq;
> +
> +	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");

I'm not sure if we had this discussion already but why do you need to keep vdd_supply
around?  Why not
	devm_regulator_get_enabled()?

Given you turn it on before reset and off afterwards and otherwise don't touch
it should end up as effectively the same as you have here.

I was expecting to see use of the regulator elsewhere, but seems not for now.
Note that if you do change this then rename the helpers to reflect they are
only doing reset handling.


> +	if (IS_ERR(data->vdd_supply))
> +		return dev_err_probe(dev, PTR_ERR(data->vdd_supply),
> +				     "Unable to get VDD regulator\n");
> +
> +	/*
> +	 * XSHUT held low puts the chip in hardware standby. All register
> +	 * state is lost on de-assert so this is functionally a reset.
> +	 */
> +	data->xshut_reset = devm_reset_control_get_optional_exclusive(dev, NULL);

If you can switch to devm_regulator_get_enabled() this can I think be
	ret = devm_reset_control_get_optional_exclusive_deasserted()
removing the need to register the action to power off.

If you do that remember we still need the sleep after this call.

> +	if (IS_ERR(data->xshut_reset))
> +		return dev_err_probe(dev, PTR_ERR(data->xshut_reset),
> +				     "Cannot get reset control\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;
> +
> +	/* 50 ms timing budget (per ST Ultra Lite Driver) */
> +	ret = vl53l1x_set_timing_budget(data, 50);
> +	if (ret)
> +		return ret;
> +
> +	/* 50 ms inter-measurement period (per ST Ultra Lite Driver) */
> +	ret = vl53l1x_set_inter_measurement_ms(data, 50);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The hardware only supports "autonomous" continuous ranging mode.
> +	 * Start ranging here and leave it running for the lifetime of
> +	 * the device. Both direct reads and the buffer path rely on this.
> +	 */
> +	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);

So Sashiko had a comment on this that has me thinking.  Don't bother fixing it
in this driver as it true it's a common bug (and leaks a trigger structure).
I'll look into it.


> +
> +		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,
> +						      NULL);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

Re: [PATCH v7 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Sirat]

On Wed, Mar 25, 2026 at 8:47 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Wed, 25 Mar 2026 12:32:23 +0600
> Siratul Islam <email@sirat.me> wrote:
>
> > Add support for the STMicroelectronics VL53L1X Time-of-Flight
> > ranging sensor with I2C interface.
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Siratul Islam <email@sirat.me>
>
> Hi Siratul,
>
> I used this as a bit of an experiment as you'd +CC lkml and took a close
> read of the feedback google's AI bot produced.
> https://sashiko.dev/#/patchset/20260325063254.18062-1-email%40sirat.me
>
> Be very careful when considering the output. There are some things
> in here that are not true, or we don't generally defend against (like the
> interrupt type related comments).
>
> The iio_trigger_get() is (I think) a more general problem so ignore that
> for your driver - will just be a tiny memory leak and stop the
> module being easily unloaded.  I'll take a look at that one on a more general
> basis.
>
> The regmap one is where these bots excel in that they sometimes go deeper
> in analysis than a typical person focused on one driver.
>
> The stuff on using standard devm_ wasn't from the bot and I think will make
> things rather simpler.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> >  MAINTAINERS                         |   1 +
> >  drivers/iio/proximity/Kconfig       |  15 +
> >  drivers/iio/proximity/Makefile      |   1 +
> >  drivers/iio/proximity/vl53l1x-i2c.c | 795 ++++++++++++++++++++++++++++
> >  4 files changed, 812 insertions(+)
> >  create mode 100644 drivers/iio/proximity/vl53l1x-i2c.c
> >
>
> > diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c
> > new file mode 100644
> > index 000000000000..085bff04f5d9
> > --- /dev/null
> > +++ b/drivers/iio/proximity/vl53l1x-i2c.c
...
> > +
> > +static int vl53l1x_probe(struct i2c_client *client)
> > +{
> > +     struct device *dev = &client->dev;
> > +     struct vl53l1x_data *data;
> > +     struct iio_dev *indio_dev;
> > +     int ret;
> > +
> > +     if (!i2c_check_functionality(client->adapter,
> > +                                  I2C_FUNC_SMBUS_READ_I2C_BLOCK |
> > +                                  I2C_FUNC_SMBUS_BYTE_DATA))
> Sashiko had an interesting comment on this...
> https://sashiko.dev/#/patchset/20260325063254.18062-1-email%40sirat.me
> (note in general be careful with this tools feedback, it has a significant
> false positive rate!)
>
> "Does this check needlessly reject pure I2C adapters?
> The driver configures regmap with reg_bits = 16, which forces regmap to use
> raw I2C transfers requiring I2C_FUNC_I2C. Checking for SMBUS_READ_I2C_BLOCK
> might prevent the driver from loading on pure I2C adapters that can perfectly
> support the device via regmap."
>
> By by reading the comment is actually wrong, however... It did made me look.
> With regbits == 16 & valbits == 8
> The regmap code checks for I2C_FUNC_SMBUS_I2C_BLOCK
> (and if not present fails).
>
> So the I2C_FUNC_SMBUS_BYTE_DATA seems unused and
> a broader check on both read and write versions of I2C_BLOCK seems
> appropriate.
>
> However, not a lot of point in checking it at all given regmap does
> so for us. So I'd drop this check.
>
> If anyone is bored, we should probably take a look to see if there
> are other redundant (or wrong) calls for this.
>
> There is another point about trigger references that might be
> correct (but not unique to this driver so ignore it).  That warrants
> some investigation - I'll take a look in near future.
>
I will drop the check altogether here then.
...
> > +     /*
> > +      * XSHUT held low puts the chip in hardware standby. All register
> > +      * state is lost on de-assert so this is functionally a reset.
> > +      */
> > +     data->xshut_reset = devm_reset_control_get_optional_exclusive(dev, NULL);
>
> If you can switch to devm_regulator_get_enabled() this can I think be
>         ret = devm_reset_control_get_optional_exclusive_deasserted()
> removing the need to register the action to power off.
>
> If you do that remember we still need the sleep after this call.
>
So I will move the sleep after
devm_reset_control_get_optional_exclusive_deasserted() then.
>
>
Thanks for the detailed review, Jonathan!

A quick question. Since these are some small  fixes, mostly removals,
and the patch has seen quite some revisions, does the 1 week wait rule
still apply here? I am still getting used to this mailing workflow
coming from the PR workflow of Github and it will help me in the
future.

Thanks
Sirat

Re: [PATCH v7 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Jonathan Cameron]

On Wed, 25 Mar 2026 22:33:22 +0600
Sirat <email@sirat.me> wrote:

> On Wed, Mar 25, 2026 at 8:47 PM Jonathan Cameron
> <jonathan.cameron@huawei.com> wrote:
> >
> > On Wed, 25 Mar 2026 12:32:23 +0600
> > Siratul Islam <email@sirat.me> wrote:
> >  
> > > Add support for the STMicroelectronics VL53L1X Time-of-Flight
> > > ranging sensor with I2C interface.
> > >
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > > Signed-off-by: Siratul Islam <email@sirat.me>  
> >
> > Hi Siratul,
> >
> > I used this as a bit of an experiment as you'd +CC lkml and took a close
> > read of the feedback google's AI bot produced.
> > https://sashiko.dev/#/patchset/20260325063254.18062-1-email%40sirat.me
> >
> > Be very careful when considering the output. There are some things
> > in here that are not true, or we don't generally defend against (like the
> > interrupt type related comments).
> >
> > The iio_trigger_get() is (I think) a more general problem so ignore that
> > for your driver - will just be a tiny memory leak and stop the
> > module being easily unloaded.  I'll take a look at that one on a more general
> > basis.
> >
> > The regmap one is where these bots excel in that they sometimes go deeper
> > in analysis than a typical person focused on one driver.
> >
> > The stuff on using standard devm_ wasn't from the bot and I think will make
> > things rather simpler.
> >
> > Thanks,
> >
> > Jonathan
> >
> >  
> > > ---
> > >  MAINTAINERS                         |   1 +
> > >  drivers/iio/proximity/Kconfig       |  15 +
> > >  drivers/iio/proximity/Makefile      |   1 +
> > >  drivers/iio/proximity/vl53l1x-i2c.c | 795 ++++++++++++++++++++++++++++
> > >  4 files changed, 812 insertions(+)
> > >  create mode 100644 drivers/iio/proximity/vl53l1x-i2c.c
> > >  
> >  
> > > diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c
> > > new file mode 100644
> > > index 000000000000..085bff04f5d9
> > > --- /dev/null
> > > +++ b/drivers/iio/proximity/vl53l1x-i2c.c  
> ...
> > > +
> > > +static int vl53l1x_probe(struct i2c_client *client)
> > > +{
> > > +     struct device *dev = &client->dev;
> > > +     struct vl53l1x_data *data;
> > > +     struct iio_dev *indio_dev;
> > > +     int ret;
> > > +
> > > +     if (!i2c_check_functionality(client->adapter,
> > > +                                  I2C_FUNC_SMBUS_READ_I2C_BLOCK |
> > > +                                  I2C_FUNC_SMBUS_BYTE_DATA))  
> > Sashiko had an interesting comment on this...
> > https://sashiko.dev/#/patchset/20260325063254.18062-1-email%40sirat.me
> > (note in general be careful with this tools feedback, it has a significant
> > false positive rate!)
> >
> > "Does this check needlessly reject pure I2C adapters?
> > The driver configures regmap with reg_bits = 16, which forces regmap to use
> > raw I2C transfers requiring I2C_FUNC_I2C. Checking for SMBUS_READ_I2C_BLOCK
> > might prevent the driver from loading on pure I2C adapters that can perfectly
> > support the device via regmap."
> >
> > By by reading the comment is actually wrong, however... It did made me look.
> > With regbits == 16 & valbits == 8
> > The regmap code checks for I2C_FUNC_SMBUS_I2C_BLOCK
> > (and if not present fails).
> >
> > So the I2C_FUNC_SMBUS_BYTE_DATA seems unused and
> > a broader check on both read and write versions of I2C_BLOCK seems
> > appropriate.
> >
> > However, not a lot of point in checking it at all given regmap does
> > so for us. So I'd drop this check.
> >
> > If anyone is bored, we should probably take a look to see if there
> > are other redundant (or wrong) calls for this.
> >
> > There is another point about trigger references that might be
> > correct (but not unique to this driver so ignore it).  That warrants
> > some investigation - I'll take a look in near future.
> >  
> I will drop the check altogether here then.
> ...
> > > +     /*
> > > +      * XSHUT held low puts the chip in hardware standby. All register
> > > +      * state is lost on de-assert so this is functionally a reset.
> > > +      */
> > > +     data->xshut_reset = devm_reset_control_get_optional_exclusive(dev, NULL);  
> >
> > If you can switch to devm_regulator_get_enabled() this can I think be
> >         ret = devm_reset_control_get_optional_exclusive_deasserted()
> > removing the need to register the action to power off.
> >
> > If you do that remember we still need the sleep after this call.
> >  
> So I will move the sleep after
> devm_reset_control_get_optional_exclusive_deasserted() then.
> >
> >  
> Thanks for the detailed review, Jonathan!
> 
> A quick question. Since these are some small  fixes, mostly removals,
> and the patch has seen quite some revisions, does the 1 week wait rule
> still apply here? I am still getting used to this mailing workflow
> coming from the PR workflow of Github and it will help me in the
> future.

Find to send now. Particularly given my recent email saying
cut off for this cycle is lunch time tomorrow (due to my travel).

If not for the dt-binding which is a little more complex change, I might
otherwise have just taken this version and tweaked it whilst applying.

Thanks

Jonathan



> 
> Thanks
> Sirat

Re: [PATCH v7 2/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Andy Shevchenko]

On Wed, Mar 25, 2026 at 02:47:19PM +0000, Jonathan Cameron wrote:
> On Wed, 25 Mar 2026 12:32:23 +0600
> Siratul Islam <email@sirat.me> wrote:

...

> > +		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);
> 
> So Sashiko had a comment on this that has me thinking.  Don't bother fixing it
> in this driver as it true it's a common bug (and leaks a trigger structure).
> I'll look into it.

This probably can be worked around by introducing a flag devm_allocated and set
it in devm_iio_trigger_alloc(). If set, the release might automatically put the
trigger at the error cases. BUT, do all drivers that allocate trigger actually
bump its reference count?

> > +		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,
> > +						      NULL);
> > +		if (ret)
> > +			return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Krzysztof Kozlowski]

On Wed, Mar 25, 2026 at 12:32:21PM +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.
> 
> Tested on Raspberry Pi 5 with a VL53L1X breakout board.
> 
> Note on vdd-supply:
> vdd-supply is now required. The VL53L0X required a physical
> regulator, but it did not specify the requirement in the binding.
> It should have always been there. The VL53L1X also requires it.
> Hence I added the requirement.
> 
> ---
> Changes in v7:
> - Add VL53L1X_REG_ prefix for register defines.
> - Use iio_push_to_buffers_with_ts().
> - Use continuous ranging and drop postenable/predisable.
> - Add comments for XSHUT reset, default config values.
> - Drop vdd-supply description in binding and comment in driver.
> - Add ABI rationale for vdd-supply to commit message (per Krzysztof, Jonathan).
> 
> 
> Changes in v6:
> - Make vdd-supply required. Add descriptions to vdd-supply and reset-gpios (per Jonathan).
> - Check reset_control_deassert() return value (per Andy).
> - Use volatile table for result registers and caching for the rest. Also mark write-only registers.

So where did you explain dropping tag as I requested? Did you read the
exact paragraph I asked you to read?

Best regards,
Krzysztof

Re: [PATCH v7 0/2] iio: proximity: add driver for ST VL53L1X ToF sensor

[Sirat]

On Wed, Mar 25, 2026 at 1:55 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Mar 25, 2026 at 12:32:21PM +0600, Siratul Islam wrote:
...
> > ---
> > Changes in v7:
> > - Add VL53L1X_REG_ prefix for register defines.
> > - Use iio_push_to_buffers_with_ts().
> > - Use continuous ranging and drop postenable/predisable.
> > - Add comments for XSHUT reset, default config values.
> > - Drop vdd-supply description in binding and comment in driver.
> > - Add ABI rationale for vdd-supply to commit message (per Krzysztof, Jonathan).
> >
> >
> > Changes in v6:
> > - Make vdd-supply required. Add descriptions to vdd-supply and reset-gpios (per Jonathan).
> > - Check reset_control_deassert() return value (per Andy).
> > - Use volatile table for result registers and caching for the rest. Also mark write-only registers.
>
> So where did you explain dropping tag as I requested? Did you read the
> exact paragraph I asked you to read?
>
I dropped the tag in v6 and I was not sure if it was ok to modify the changelog
of v6 when sending v7. But I will keep this in mind.

Thanks,
Sirat