Re: [PATCH] dt-bindings: iio: adc: add documentation for adi,ad7816

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: iio: adc: adi,ad7816 omits required GPIOs, IRQ, and SPI props
--

commit 30fcb41a84f071fc52e91564f9cf17b3f641d76d
Author: gtl-m <admin@micronull.cn>

dt-bindings: iio: adc: add documentation for adi,ad7816

This commit adds a YAML device tree binding document for the Analog
Devices AD7816, AD7817, and AD7818 family of ADCs.

> 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..0bbc52829b777
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
> @@ -0,0 +1,43 @@

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7816
> +      - adi,ad7817
> +      - adi,ad7818
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad7816";
> +            reg = <0>;
> +        };
> +    };
> +...

[Severity: Medium]
Does this schema omit the required GPIO properties, optional interrupt,
and SPI peripheral schema reference needed by the driver?

Looking at ad7816_probe() in drivers/staging/iio/adc/ad7816.c, the
driver unconditionally requires rdwr-gpios and convert-gpios for all
variants, and requires busy-gpios when chip->id is ID_AD7816 or
ID_AD7817:

drivers/staging/iio/adc/ad7816.c:ad7816_probe() {
    ...
	chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_OUT_HIGH);
	if (IS_ERR(chip->rdwr_pin)) {
		ret = PTR_ERR(chip->rdwr_pin);
		dev_err(&spi_dev->dev, "Failed to request rdwr GPIO: %d\n",
			ret);
		return ret;
	}
	chip->convert_pin = devm_gpiod_get(&spi_dev->dev, "convert",
					   GPIOD_OUT_HIGH);
	if (IS_ERR(chip->convert_pin)) {
		ret = PTR_ERR(chip->convert_pin);
		dev_err(&spi_dev->dev, "Failed to request convert GPIO: %d\n",
			ret);
		return ret;
	}
	if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
		chip->busy_pin = devm_gpiod_get(&spi_dev->dev, "busy",
						GPIOD_IN);
		if (IS_ERR(chip->busy_pin)) {
			ret = PTR_ERR(chip->busy_pin);
			dev_err(&spi_dev->dev, "Failed to request busy GPIO: %d\n",
				ret);
			return ret;
		}
	}
    ...
}

ad7816_probe() also checks spi_dev->irq to register an optional
over-temperature indicator interrupt handler:

drivers/staging/iio/adc/ad7816.c:ad7816_probe() {
    ...
	if (spi_dev->irq) {
		/* Only low trigger is supported in ad7816/7/8 */
		ret = devm_request_threaded_irq(&spi_dev->dev, spi_dev->irq,
						NULL,
						&ad7816_event_handler,
						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
						indio_dev->name,
						indio_dev);
		if (ret)
			return ret;
	}
    ...
}

Because the schema only defines compatible and reg with
additionalProperties: false, any device tree node that conforms to this
binding (such as the adc@0 example node) will fail ad7816_probe() with
-ENOENT, while any working device tree node supplying rdwr-gpios,
convert-gpios, busy-gpios, interrupts, or standard SPI properties will
fail dtbs_check.

Should Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
reference /schemas/spi/spi-peripheral-props.yaml# with
unevaluatedProperties: false, document rdwr-gpios, convert-gpios,
busy-gpios (required for adi,ad7816 and adi,ad7817), and interrupts, and
include the required GPIOs in the example?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018101921.36424-1-wrmfwv@163.com?part=1