Re: [PATCH] dt-bindings: iio: adc: Add schema for AD7816/7/8 digital temperature sensor

[Jonathan Cameron]

On Sat, 30 May 2026 13:53:00 +0000
Taha Narimani <tahanarimani3443@xxxxxxxxx> wrote:

> Document the Analog Devices AD7816, AD7817, and AD7818 digital
> temperature sensor and ADC bindings in YAML format.
> 
> This resolves the checkpatch.pl warnings regarding undocumented DT
> compatible strings for 'adi,ad7816', 'adi,ad7817', and 'adi,ad7818'.
> 
> Signed-off-by: Taha Narimani <tahanarimani3443@xxxxxxxxx>
Hi Taha,

Please read the documentation for submitting patches. Also look at similar
dt-binding patches for title style

[PATCH v2] dt-bindings: iio: adc: add ad7816/7/8 digital temperature sensor / ADC.

The ADC bit is to make the point this is a general purpose ADC rather than simply
temp monitoring.

Also, never send a new version in with reply-to set to the previous version.
That just makes for very confusing emails. Even more so when they both have
the same email title!  New thread for every version.

The driver is in staging currently.  Normally we only submit a dt-binding as part
of the patch series that includes the patch moving the driver out of staging.

I don't mind reviewing it anyway, but for now I'm not considering this for
merging.  Various comments inline.  If you do send a new version make sure
the patch description calls out that the driver is in staging and not yet
ready to move out.  Maybe better to keep this as an RFC given that constraint.
The only exception we might make is if another project (i.e. not the kernel)
needs the dt-binding.

Jonathan

> 
> ---
> v2: Added conditional schema constraints for busy-gpios as suggested by Sashiko AI.
> ---
>  .../bindings/iio/adc/adi,ad7816.yaml          | 87 +++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
> new file mode 100644
> index 0000000..74008c1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7816.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7816/7/8 digital temperature sensor and ADC
> +
> +maintainers:
> +  - Taha Narimani <tahanarimani3443@xxxxxxxxx>
> +
> +description: |
> +  Analog Devices AD7816, AD7817, and AD7818 10-Bit temperature sensor and ADC.
> +  The device communicates via an SPI interface.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7816
> +      - adi,ad7817
> +      - adi,ad7818
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency: true
We normally provide a maximum for this based on what this chip supports.
Obviously the board or the spi controller might have lower limits but
we can at least ensure we don't set it too high for the device.

> +
> +  rdwr-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the RD/WR pin.
> +
> +  convert-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the CONVST (Convert Start) pin.
> +
> +  busy-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the BUSY pin.

Interestingly the datasheet for the ad7817 suggests using this as an interrupt.
The driver seems to currently be busy looping on this which is a rather ugly
solution and likely something we would want to look at before moving the
driver out of staging.  This is an example of why we don't tend to do bindings
ahead of that move!

> +
> +  interrupts:
> +    maxItems: 1
> +    description: OTI (Over Temperature Indicator) interrupt.
> +

Power supplies are missing.  They should be required if they need to be
there for the device to function. It doesn't matter if the driver actually
uses them or not as the binding should still be complete.  Note that some
of the parts definitely have an internal reference option, so vref-supply
will be optional.  The lack of it being provided means use the internal
reference instead.

Also looks like at least some parts have an !OTI pin which should be an
interrupt.


> +required:
> +  - compatible
> +  - reg
> +  - rdwr-gpios
> +  - convert-gpios
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7816
> +              - adi,ad7817
> +    then:
> +      required:
> +        - busy-gpios
> +    else:
> +      properties:
> +        busy-gpios: false

Looks like you'll need to handle vref-supply in a similar fashion.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad7816";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            rdwr-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
> +            convert-gpios = <&gpio 6 GPIO_ACTIVE_HIGH>;
> +            busy-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>;
> +            interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> +            interrupt-parent = <&gpio>;

You don't have any interrupts in the binding. So this shouldn't
pass testing.

> +        };
> +    };