Re: [PATCH v5 05/13] dt-bindings: iio: frequency: add ad9910

[Rodrigo Alencar]

On 26/05/18 09:52AM, Krzysztof Kozlowski wrote:
> On Sun, May 17, 2026 at 07:37:49PM +0100, Rodrigo Alencar wrote:
> > +maintainers:
> > +  - Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> > +
> > +description:
> > +  The AD9910 is a 1 GSPS direct digital synthesizer (DDS) with an integrated
> > +  14-bit DAC. It features single tone mode with 8 configurable profiles,
> > +  a digital ramp generator, RAM control, OSK, and a parallel data port for
> > +  high-speed streaming.
> > +
> > +  https://www.analog.com/en/products/ad9910.html
> > +
> > +properties:
> > +  compatible:
> > +    const: adi,ad9910
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 70000000
> > +
> > +  clocks:
> > +    minItems: 1
> > +    items:
> > +      - description: Reference clock (REF_CLK).
> > +      - description: Optional synchronization clock (SYNC_IN).
> > +
> > +  clock-names:
> > +    oneOf:
> > +      - items:
> > +          - const: ref_clk
> > +      - items:
> > +          - const: ref_clk
> > +          - const: sync_in
> 
> So that's just items with two items and minItems: 1. Like you have in
> "clocks:".
> 
> You got this comment already at v2.

You're right! will adjust. For some reason I thought I had problems with
the dt-binding check without this.

> 
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  clock-output-names:
> > +    minItems: 1
> > +    maxItems: 3
> > +    items:
> > +      enum: [ sync_clk, pdclk, sync_out ]
> 
> Why are the names fixed? And why is the order random?

All of those would be optional. Having it in a specific order we would need
to register all the clocks even if only one (or none) is used?

> > +
> > +  interrupts:
> > +    minItems: 1
> > +    items:
> > +      - description:
> > +          Signal that indicates that Digital Ramp Generator has reached a limit.
> > +      - description:
> > +          Signal that indicates the end of a RAM Sweep.
> > +
> > +  interrupt-names:
> > +    minItems: 1
> > +    maxItems: 2
> > +    items:
> > +      enum: [ drover, ram_swp_ovr ]
> 
> Your "interrupts:" do not allow flexibility. Are you sure interrupts are
> optional in the hardware?

Good point, They are optional. Should I drop the items and descriptions in "interrupts"?

> > +
> > +  dvdd-io33-supply:
> > +    description: 3.3V Digital I/O supply.
> > +
> > +  avdd33-supply:
> > +    description: 3.3V Analog DAC supply.
> 
> Best regards,
> Krzysztof
> 

-- 
Kind regards,

Rodrigo Alencar