On Fri, Dec 13, 2024 at 11:55:22AM +0100, Krzysztof Kozlowski wrote: > > +description: | > > Do not need '|' unless you need to preserve formatting. Ack > > > + The CS2600 is a system-clocking device that enables frequency synthesis and > > + clock generation from a stable timing reference clock. The device can generate > > + low-jitter clocks from a noisy clock reference at frequencies as low as 50 Hz. > > Be sure these are wrapped at 80 (looks like 81). Ack > > > + > > +properties: > > + compatible: > > + enum: > > + - cirrus,cs2600 > > + > > + clocks: > > + description: > > + Common clock binding for XTI/REF_CLK, CLK_IN > > Just drop description, kind of obvious from the clock-names. If you want > to keep it, then rather list the items with description per each item. Ack > > > + maxItems: 2 > > + > > + clock-names: > > + items: > > + - const: ref_clk > > s/ref_clk/ref/ > (or xti) > > "clk" is just obvious. Ack > > > + cirrus,aux-output-source: > > + description: > > + Specifies the function of the auxiliary clock output pin > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: > > + - 0 # CS2600_AUX_OUTPUT_FREQ_UNLOCK: Frequency unlock notification > > + - 1 # CS2600_AUX_OUTPUT_PHASE_UNLOCK: Phase unlock notification > > + - 2 # CS2600_AUX_OUTPUT_NO_CLKIN: CLK_IN is not available > > I don't get the meanings - how clock output could be clock is not > available? clk_in is a required property, so it cannot be unavailable. These are the values for the enum. I see how this is confusing. I will change it. > > + reg = <0x0 0x100>; > > Drop reg Ack > > > +#define CS2600_CLK_OUTPUT 0 > > Drop "OUTPUT". These are names of the clocks. Ack > > > +#define CS2600_BCLK_OUTPUT 1 > > +#define CS2600_FSYNC_OUTPUT 2 > > + > > +/* CS2600 Auxiliary Output */ > > +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 > > +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 > > +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 > > Not sure what are these, but feel like some register values. Otherwise, > why these are exactly bindings? What part of driver do they bind? They are different outputs for the aux_out_source pin. I think it may be best just to remove these defines. Regards, Paul
