Message-ID: <20250802121238.108ebfad@jic23-huawei>
Date: Sat, 2 Aug 2025 12:12:38 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, dlechner@...libre.com,
 nuno.sa@...log.com, andy@...nel.org, conor+dt@...nel.org,
 mranostay@...il.com, ~lkcamp/patches@...ts.sr.ht,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, William Breathitt Gray <wbg@...nel.org>
Subject: Re: [PATCH v2] dt-bindings: iio: proximity: Add Lidar-lite-v2

On Fri,  1 Aug 2025 19:39:15 -0300
Rodrigo Gobbi <rodrigo.gobbi.7@...il.com> wrote:

> Move existing ABI documentation from trivial to
> a dedicated binding file since Lidar is not a trivial
> device considering power-enable and mode control pin.
> 
> Also, add a fallback compatible for v3, which has the
> same pinout and is already supported by the driver.
> 
> Fixes: b257c1a45e99 ("iio: pulsedlight-lidar-lite-v2: add lidar-lite-v3 property")
> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
> ---
> I was unsure about sending a new v0 patch for this or send a v2. To avoid losing
> the lore about this topic, I`m sending a v2. If this is not correct, I can send a
> new patch later.
> 
> On 7/3/25 18:26, David Lechner wrote:
> > On 7/1/25 5:30 PM, Rodrigo Gobbi wrote:  
> >> The compatible grmn,lidar-lite-v3 is managed by the same
> >> driver of pulsedlight,lidar-lite-v2, which is a trivial device.  
> > 
> > As a general rule of thumb, using the driver as justification for
> > dt-bindings is never a good reason. The bindings describe the hardware,
> > not the driver.
> > 
> > Assuming I found the correct datasheet [1], I see a power enable pin
> > and a mode control pin, so I would say that this isn't a trivial device.
> > Therefore this will need it's own new file. We could at least add
> > power-gpios and power-supply properties. How to handle the mode pin
> > isn't so clear to me though, so might omit that for now.  
> About the mode control pin and the data being returned within PWM, it`s also
> unclear to me how to describe that here. Looking other kind of existing iio
> devices, couldn`t find a reference for it so I`ve not described that.

So far we've never supported a sensor with a PWM output.  Needs some capture logic
and whilst there is some supported in the kernel, I don't think we have the
infrastructure to describe the sensor beyond it. It relies on an odd combination
of triggering via a light pull low that the device then drives high.  To make
that work with a standard capture unit is probably a case of wiring multiple pins
or some external components.

+CC counters subsystem maintainer William.

https://static.garmin.com/pumac/LIDAR_Lite_v3_Operation_Manual_and_Technical_Specifications.pdf
for reference

However, I'm also in agreement with others that this is an unusual case where
we are very likely to missdesign a DT-binding without having explored what the
driver stack looks like and so are best just leaving a gap for now.

Even if we did describe the mode stuff it would be optional so not describing it
for now should be fine.


> 
> Also, I`m quoting the driver author about this binding due the maintainer ref for it.
> 
> Dear @Matt Ranostay, I`ve noticed you were the original driver author.
> During the discussion about adding lidar-v3 as trivial [1], we noticed that
> this HW is not actually a trivial due other pins like power-enable
> and mode control. We are considering moving v2 and v3 (which was not documented)
> out of trivial and this is what this patch is trying to do. 
> Also, we need a maintainer for the binding file and I`ve quoted you there.
> I would appreciate your comments or suggestions over this topic.
> 
> Tks and regards to all.
> 
> Changelog:
> v2: creating an initial binding for lidar v2 and v3 (fallback to v2)
>     also, moving v2 out of trivial
> v1: https://lore.kernel.org/all/20250701223341.36835-1-rodrigo.gobbi.7@gmail.com/#t
> ---
>  .../proximity/pulsedlight,lidar-lite-v2.yaml  | 54 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  2 -
>  2 files changed, 54 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/proximity/pulsedlight,lidar-lite-v2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/proximity/pulsedlight,lidar-lite-v2.yaml b/Documentation/devicetree/bindings/iio/proximity/pulsedlight,lidar-lite-v2.yaml
> new file mode 100644
> index 000000000000..f49a1c365f3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/pulsedlight,lidar-lite-v2.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/proximity/pulsedlight,lidar-lite-v2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Pulsedlight LIDAR-Lite v2 range-finding sensor
> +
> +maintainers:
> +  - Matt Ranostay <mranostay@...il.com>
> +
> +description: |
> +  Support for LIDAR_Lite v2 and v3 laser rangefinders. These devices
> +  can use a simple I2C communication bus or can operate in a PWM mode using a
> +  mode control pin to trigger acquisitions and return the measured distance.
> +  It also have a power enable pin, which can be used to shut off the device.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - grmn,lidar-lite-v3
> +          - const: pulsedlight,lidar-lite-v2
> +      - const: pulsedlight,lidar-lite-v2
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-gpios:
> +    description: GPIO that can be driven low to shut off power to the device.
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        proximity@62 {
> +            compatible = "grmn,lidar-lite-v3", "pulsedlight,lidar-lite-v2";
> +            reg = <0x62>;
> +            vdd-supply = <&vdd_5v0>;
> +        };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 8da408107e55..347897b999c9 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -313,8 +313,6 @@ properties:
>            - onnn,adt7462
>              # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
>            - plx,pex8648
> -            # Pulsedlight LIDAR range-finding sensor
> -          - pulsedlight,lidar-lite-v2
>              # Renesas HS3001 Temperature and Relative Humidity Sensors
>            - renesas,hs3001
>              # Renesas ISL29501 time-of-flight sensor

Powered by blists - more mailing lists