Re: [PATCH v6 08/16] dt-bindings: iio: frequency: add ad9910

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `#clock-cells` property is missing from the `required` list and the `examples` section, violating Device Tree subsystem guidelines.
- [Medium] The commit message claims to include 'DAC current' configurations, but no such property exists in the schema.
- [Low] The `reset-gpios` property is missing the `maxItems: 1` constraint.
--

commit 14de29fc570f7c4ee9b2e39250151ecb7c9edb56
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

dt-bindings: iio: frequency: add ad9910

DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
configurations for clocks, DAC current, reset and basic GPIO control.

> DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
> configurations for clocks, DAC current, reset and basic GPIO control.

[Severity: Medium]
The commit message states the binding includes configurations for
"DAC current". Does the schema actually define this property?

Looking at the schema, there is adi,charge-pump-current-microamp for the
PLL charge pump, but no property appears to configure the DAC current.

Could the commit message be updated to match the schema, or is the DAC current
property missing from the YAML?

> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml
> new file mode 100644
> index 0000000000000..a78fe33ba21f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,ad9910.yaml

[ ... ]

> +  avdd18-supply:
> +    description: 1.8V Analog Core supply.
> +
> +  reset-gpios:
> +    description:
> +      GPIOs controlling the Main Device reset.

[Severity: Low]
Should reset-gpios have a maxItems: 1 constraint?

Unlike io-reset-gpios and powerdown-gpios below, this property lacks a
maxItems constraint. In dt-schema, properties ending in -gpios allow an
array of GPIOs unless explicitly constrained.

This omission allows invalid device tree configurations with multiple reset
GPIOs to pass schema validation.

> +  io-reset-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO controlling the I/O_RESET pin.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - dvdd-io33-supply
> +  - avdd33-supply
> +  - dvdd18-supply
> +  - avdd18-supply

[Severity: High]
Should #clock-cells be included in this required list?

According to the Device Tree Bindings subsystem guidelines, when hardware
defines provider capabilities (acting as a clock provider as indicated by
the #clock-cells: const: 1 property defined earlier in this schema), the
corresponding standard properties must be explicitly included in the
required list.

> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        dds@0 {
> +            compatible = "adi,ad9910";
> +            reg = <0>;

[Severity: High]
Does this example need the #clock-cells property?

The example device tree node is missing the #clock-cells property.

Subsystem guidelines dictate that the examples section must demonstrate
all required properties to pass dt_binding_check validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-ad9910-iio-driver-v6-0-79125ffbe430@analog.com?part=8