Re: [PATCH v3 RFC v3] dt-bindings: iio: adc: add ad7816/7/8 digital temperature sensor / ADC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Strictly disallowing `busy-gpios` for `adi,ad7816` breaks the existing driver probe.
- [Medium] `rdwr-gpios` and `convert-gpios` are unconditionally required for all variants, forcing dummy GPIOs for 8-pin devices.
- [Medium] The `vref-supply` property is incorrectly allowed for AD7816 and lacks a schema restriction.
- [Medium] The `interrupts` property is incorrectly permitted for the 8-pin AD7818.
--

commit c95479a0b55dc8599b178909579f060bf75eb282
Author: Taha Narimani <tahanarimani3443@gmail.com>

dt-bindings: iio: adc: add ad7816/7/8 digital temperature sensor / ADC

This commit introduces YAML device tree bindings for the Analog Devices
AD7816, AD7817, and AD7818. It formally documents the required properties
and uses conditionals to restrict busy-gpios to the AD7817 variant based
on hardware capabilities.

> 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 0000000000000..48563e4eab446
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml

[ ... ]

> +  vref-supply:
> +    description: |
> +      Optional external reference voltage supply. If not provided, the
> +      internal reference is used. Applicable for AD7816 and AD7817.

[Severity: Medium]
Does the AD7816 actually have a VREF pin?

The 8-pin AD7816 lacks an external reference voltage pin due to physical
pinout constraints (power, SPI, CONVST, OTI), and relies entirely on its
internal reference. Should the description limit this to only the AD7817?

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - rdwr-gpios
> +  - convert-gpios

[Severity: Medium]
Are rdwr-gpios and convert-gpios present on all these devices?

The AD7816 and AD7818 are 8-pin devices whose pins are fully consumed by
SPI bus communication, power, and their specific analog or interrupt
functions.

Since they physically lack dedicated RD/WR pins, does making these globally
required force device tree authors to invent dummy GPIOs to pass validation?
Should these be conditionally required instead?

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

[Severity: High]
Will this break the existing staging driver probe for the AD7816?

While the hardware lacks a BUSY pin, the driver unconditionally requires this
GPIO for both AD7816 and AD7817 during probe:

drivers/staging/iio/adc/ad7816.c:ad7816_probe() {
    ...
    chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy", GPIOD_IN);
    ...
}

If a device tree complies with this new binding by omitting busy-gpios for
the AD7816, devm_gpiod_get() will return -ENOENT and the driver will abort
probing.

Can we address the driver dependency before strictly enforcing this in the
binding?

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,ad7818
> +    then:
> +      properties:
> +        vref-supply: false

[Severity: Medium]
Should vref-supply: false also be enforced for the AD7816?

Since the AD7816 is an 8-pin device like the AD7818, it seems to rely
entirely on its internal reference without an external pin.

Additionally, does the AD7818 physically support the OTI interrupt?

The AD7818 sacrifices the OTI pin to accommodate its Analog Input (AIN) pin.
Should this conditional block also enforce interrupts: false for the
adi,ad7818 compatible string to prevent physically impossible device tree
configurations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530165917.55767-1-tahanarimani3443@gmail.com?part=1