| Message ID | 20241219024631.3145377-2-paulha@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Cirrus Logic CS2600 clock device | expand |
On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: > + clock-output-names: > + maxItems: 3 > + description: Names for CLK_OUT, BCLK_OUT and FSYNC_OUT clocks. > + > + cirrus,aux-output-source: > + description: > + Specifies the function of the auxiliary 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 still do not understand how "clk_in", which is required, could be not available. To me it contradicts itself, but maybe just description is a bit incomplete. Anyway, why this cannot be simple string? > + default: 0 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - '#clock-cells' > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/cirrus,cs2600.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + clock-controller@2c { > + #clock-cells = <1>; Use order from DTS coding style. > + compatible = "cirrus,cs2600"; > + reg = <0x2c>; > + clocks = <&xtl_clk>, <&sync_clock>; > + clock-names = "xti", "clk_in"; > + clock-output-names = "audio_clk_out", "audio_bclk", "audio_lrclk"; > + vdd-supply = <&vreg>; > + }; > + }; > diff --git a/include/dt-bindings/clock/cirrus,cs2600.h b/include/dt-bindings/clock/cirrus,cs2600.h > new file mode 100644 > index 000000000000..86065f94a2b2 > --- /dev/null > +++ b/include/dt-bindings/clock/cirrus,cs2600.h > @@ -0,0 +1,24 @@ > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ > +/* > + * Copyright (c) 2024 Cirrus Logic, Inc. and > + * Cirrus Logic International Simiconductor Ltd. > + * > + * Author: Paul Handrigan <paulha@opensource.cirrus.com> > + * > + */ > + > +#ifndef _DT_BINDINGS_CLK_CIRRUS_CS2600_H > +#define _DT_BINDINGS_CLK_CIRRUS_CS2600_H > + > +/* CS2600 Clocks */ > +#define CS2600_PLL 0 /* Internal clock */ > +#define CS2600_CLK 1 > +#define CS2600_BCLK 2 > +#define CS2600_FSYNC 3 > + > +/* CS2600 Auxiliary Output */ > +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 > +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 > +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 I still don't see why these three are supposed to be bindings. Drop them. Best regards, Krzysztof
On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: > On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: > > + clock-output-names: > > + maxItems: 3 > > + description: Names for CLK_OUT, BCLK_OUT and FSYNC_OUT clocks. > > + > > + cirrus,aux-output-source: > > + description: > > + Specifies the function of the auxiliary 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 still do not understand how "clk_in", which is required, could be not > available. To me it contradicts itself, but maybe just description is a > bit incomplete. I think yeah the description perhaps needs to make this more clear this is setting an error output pin, ie. the pin goes high for the linked error condition. > > +/* CS2600 Auxiliary Output */ > > +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 > > +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 > > +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 > > I still don't see why these three are supposed to be bindings. Drop > them. In a binding one would presumably do: cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; Apologies but I don't quite understand what you mean by the values are not used in the binding? The driver reads the property and sets the pin to have the appropriate function. Admittedly one could drop the defines and then DTS would just have to do: cirrus,aux-output-source = <0>; But that feels a bit less helpful when reading the binding. Thanks, Charles
On 19/12/2024 12:02, Charles Keepax wrote: > On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: >> On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: >>> + clock-output-names: >>> + maxItems: 3 >>> + description: Names for CLK_OUT, BCLK_OUT and FSYNC_OUT clocks. >>> + >>> + cirrus,aux-output-source: >>> + description: >>> + Specifies the function of the auxiliary 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 still do not understand how "clk_in", which is required, could be not >> available. To me it contradicts itself, but maybe just description is a >> bit incomplete. > > I think yeah the description perhaps needs to make this more > clear this is setting an error output pin, ie. the pin goes high > for the linked error condition. > >>> +/* CS2600 Auxiliary Output */ >>> +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>> +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 >>> +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 >> >> I still don't see why these three are supposed to be bindings. Drop >> them. > > In a binding one would presumably do: > > cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; > > Apologies but I don't quite understand what you mean by the values > are not used in the binding? The driver reads the property and sets There is no user of these defines, so not a binding. > the pin to have the appropriate function. Admittedly one could drop It's not a proof that this is a binding. > the defines and then DTS would just have to do: > > cirrus,aux-output-source = <0>; > > But that feels a bit less helpful when reading the binding. Binding and being helpful are two different things. This to be the binding, it has to be used as a binding, so some translation layer between driver and DTS. It must have an user in DTS. I keep repeating this over and over... Best regards, Krzysztof
On Thu, Dec 19, 2024 at 12:39:38PM +0100, Krzysztof Kozlowski wrote: > On 19/12/2024 12:02, Charles Keepax wrote: > > On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: > >> On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: > >>> +/* CS2600 Auxiliary Output */ > >>> +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 > >>> +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 > >>> +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 > >> > >> I still don't see why these three are supposed to be bindings. Drop > >> them. > > > > In a binding one would presumably do: > > > > cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; > > > > Apologies but I don't quite understand what you mean by the values > > are not used in the binding? The driver reads the property and sets > > There is no user of these defines, so not a binding. > > > the pin to have the appropriate function. Admittedly one could drop > > It's not a proof that this is a binding. > > > the defines and then DTS would just have to do: > > > > cirrus,aux-output-source = <0>; > > > > But that feels a bit less helpful when reading the binding. > > Binding and being helpful are two different things. This to be the > binding, it has to be used as a binding, so some translation layer > between driver and DTS. It must have an user in DTS. I keep repeating > this over and over... > Apologies, but I not sure I totally follow this, and apologies if you have already explained this are there some docs I can look at? I think you are saying because these defines merely represent the valid values for a device tree property and are not translated into different values you can't put defines for them in the binding header? So this would not be allowed: #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; device_property_read_u32(dev, "cirrus,aux-output-source", &val); regmap_write(regmap, CS2600_OUTPUT_CFG2, val); But this would be fine: #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 1 cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; device_property_read_u32(dev, "cirrus,aux-output-source", &val); switch (val) { case CS2600_AUX_OUTPUT_FREQ_UNLOCK: regmap_write(regmap, CS2600_OUTPUT_CFG2, 0); } And this would also be fine? cirrus,aux-output-source = <0>; device_property_read_u32(dev, "cirrus,aux-output-source", &val); regmap_write(regmap, CS2600_OUTPUT_CFG2, val); Thanks, Charles
On 19/12/2024 13:59, Charles Keepax wrote: > On Thu, Dec 19, 2024 at 12:39:38PM +0100, Krzysztof Kozlowski wrote: >> On 19/12/2024 12:02, Charles Keepax wrote: >>> On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: >>>> On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: >>>>> +/* CS2600 Auxiliary Output */ >>>>> +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>>>> +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 >>>>> +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 >>>> >>>> I still don't see why these three are supposed to be bindings. Drop >>>> them. >>> >>> In a binding one would presumably do: >>> >>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>> >>> Apologies but I don't quite understand what you mean by the values >>> are not used in the binding? The driver reads the property and sets >> >> There is no user of these defines, so not a binding. >> >>> the pin to have the appropriate function. Admittedly one could drop >> >> It's not a proof that this is a binding. >> >>> the defines and then DTS would just have to do: >>> >>> cirrus,aux-output-source = <0>; >>> >>> But that feels a bit less helpful when reading the binding. >> >> Binding and being helpful are two different things. This to be the >> binding, it has to be used as a binding, so some translation layer >> between driver and DTS. It must have an user in DTS. I keep repeating >> this over and over... >> > > Apologies, but I not sure I totally follow this, and apologies if > you have already explained this are there some docs I can look > at? > > I think you are saying because these defines merely represent the > valid values for a device tree property and are not translated > into different values you can't put defines for them in the binding > header? > > So this would not be allowed: > > #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 > > cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; > > device_property_read_u32(dev, "cirrus,aux-output-source", &val); > regmap_write(regmap, CS2600_OUTPUT_CFG2, val); > > But this would be fine: > > #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 1 > > cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; > > device_property_read_u32(dev, "cirrus,aux-output-source", &val); > switch (val) { > case CS2600_AUX_OUTPUT_FREQ_UNLOCK: > regmap_write(regmap, CS2600_OUTPUT_CFG2, 0); > } > > And this would also be fine? > > cirrus,aux-output-source = <0>; > > device_property_read_u32(dev, "cirrus,aux-output-source", &val); > regmap_write(regmap, CS2600_OUTPUT_CFG2, val); > Yes. If you want to use in DTS user-readable values, then use string. Best regards, Krzysztof
On 19/12/24 13:16, Krzysztof Kozlowski wrote: > On 19/12/2024 13:59, Charles Keepax wrote: >> On Thu, Dec 19, 2024 at 12:39:38PM +0100, Krzysztof Kozlowski wrote: >>> On 19/12/2024 12:02, Charles Keepax wrote: >>>> On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: >>>>> On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: >>>>>> +/* CS2600 Auxiliary Output */ >>>>>> +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>>>>> +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 >>>>>> +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 >>>>> >>>>> I still don't see why these three are supposed to be bindings. Drop >>>>> them. >>>> >>>> In a binding one would presumably do: >>>> >>>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>>> >>>> Apologies but I don't quite understand what you mean by the values >>>> are not used in the binding? The driver reads the property and sets >>> >>> There is no user of these defines, so not a binding. >>> >>>> the pin to have the appropriate function. Admittedly one could drop >>> >>> It's not a proof that this is a binding. >>> >>>> the defines and then DTS would just have to do: >>>> >>>> cirrus,aux-output-source = <0>; >>>> >>>> But that feels a bit less helpful when reading the binding. >>> >>> Binding and being helpful are two different things. This to be the >>> binding, it has to be used as a binding, so some translation layer >>> between driver and DTS. It must have an user in DTS. I keep repeating >>> this over and over... >>> >> >> Apologies, but I not sure I totally follow this, and apologies if >> you have already explained this are there some docs I can look >> at? >> >> I think you are saying because these defines merely represent the >> valid values for a device tree property and are not translated >> into different values you can't put defines for them in the binding >> header? >> >> So this would not be allowed: >> >> #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >> >> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >> >> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >> regmap_write(regmap, CS2600_OUTPUT_CFG2, val); >> >> But this would be fine: >> >> #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 1 >> >> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >> >> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >> switch (val) { >> case CS2600_AUX_OUTPUT_FREQ_UNLOCK: >> regmap_write(regmap, CS2600_OUTPUT_CFG2, 0); >> } >> >> And this would also be fine? >> >> cirrus,aux-output-source = <0>; >> >> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >> regmap_write(regmap, CS2600_OUTPUT_CFG2, val); >> > Yes. If you want to use in DTS user-readable values, then use string. > I don't understand this. Why should we have to use a string value for something that only needs a simple integer value? Why can't we define constants with meaningful names? > Best regards, > Krzysztof
On 19/12/2024 14:36, Richard Fitzgerald wrote: > On 19/12/24 13:16, Krzysztof Kozlowski wrote: >> On 19/12/2024 13:59, Charles Keepax wrote: >>> On Thu, Dec 19, 2024 at 12:39:38PM +0100, Krzysztof Kozlowski wrote: >>>> On 19/12/2024 12:02, Charles Keepax wrote: >>>>> On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: >>>>>> On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: >>>>>>> +/* CS2600 Auxiliary Output */ >>>>>>> +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>>>>>> +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 >>>>>>> +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 >>>>>> >>>>>> I still don't see why these three are supposed to be bindings. Drop >>>>>> them. >>>>> >>>>> In a binding one would presumably do: >>>>> >>>>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>>>> >>>>> Apologies but I don't quite understand what you mean by the values >>>>> are not used in the binding? The driver reads the property and sets >>>> >>>> There is no user of these defines, so not a binding. >>>> >>>>> the pin to have the appropriate function. Admittedly one could drop >>>> >>>> It's not a proof that this is a binding. >>>> >>>>> the defines and then DTS would just have to do: >>>>> >>>>> cirrus,aux-output-source = <0>; >>>>> >>>>> But that feels a bit less helpful when reading the binding. >>>> >>>> Binding and being helpful are two different things. This to be the >>>> binding, it has to be used as a binding, so some translation layer >>>> between driver and DTS. It must have an user in DTS. I keep repeating >>>> this over and over... >>>> >>> >>> Apologies, but I not sure I totally follow this, and apologies if >>> you have already explained this are there some docs I can look >>> at? >>> >>> I think you are saying because these defines merely represent the >>> valid values for a device tree property and are not translated >>> into different values you can't put defines for them in the binding >>> header? >>> >>> So this would not be allowed: >>> >>> #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>> >>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>> >>> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >>> regmap_write(regmap, CS2600_OUTPUT_CFG2, val); >>> >>> But this would be fine: >>> >>> #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 1 >>> >>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>> >>> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >>> switch (val) { >>> case CS2600_AUX_OUTPUT_FREQ_UNLOCK: >>> regmap_write(regmap, CS2600_OUTPUT_CFG2, 0); >>> } >>> >>> And this would also be fine? >>> >>> cirrus,aux-output-source = <0>; >>> >>> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >>> regmap_write(regmap, CS2600_OUTPUT_CFG2, val); >>> >> Yes. If you want to use in DTS user-readable values, then use string. >> > > I don't understand this. Why should we have to use a string value for > something that only needs a simple integer value? Why can't we define > constants with meaningful names? You can and you will find plenty examples of this, but as I explained earlier - this is not a binding. We avoid defining as a binding something which is not a binding. Best regards, Krzysztof
On 19/12/24 15:57, Krzysztof Kozlowski wrote: > On 19/12/2024 14:36, Richard Fitzgerald wrote: >> On 19/12/24 13:16, Krzysztof Kozlowski wrote: >>> On 19/12/2024 13:59, Charles Keepax wrote: >>>> On Thu, Dec 19, 2024 at 12:39:38PM +0100, Krzysztof Kozlowski wrote: >>>>> On 19/12/2024 12:02, Charles Keepax wrote: >>>>>> On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote: >>>>>>> On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote: >>>>>>>> +/* CS2600 Auxiliary Output */ >>>>>>>> +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>>>>>>> +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 >>>>>>>> +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 >>>>>>> >>>>>>> I still don't see why these three are supposed to be bindings. Drop >>>>>>> them. >>>>>> >>>>>> In a binding one would presumably do: >>>>>> >>>>>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>>>>> >>>>>> Apologies but I don't quite understand what you mean by the values >>>>>> are not used in the binding? The driver reads the property and sets >>>>> >>>>> There is no user of these defines, so not a binding. >>>>> >>>>>> the pin to have the appropriate function. Admittedly one could drop >>>>> >>>>> It's not a proof that this is a binding. >>>>> >>>>>> the defines and then DTS would just have to do: >>>>>> >>>>>> cirrus,aux-output-source = <0>; >>>>>> >>>>>> But that feels a bit less helpful when reading the binding. >>>>> >>>>> Binding and being helpful are two different things. This to be the >>>>> binding, it has to be used as a binding, so some translation layer >>>>> between driver and DTS. It must have an user in DTS. I keep repeating >>>>> this over and over... >>>>> >>>> >>>> Apologies, but I not sure I totally follow this, and apologies if >>>> you have already explained this are there some docs I can look >>>> at? >>>> >>>> I think you are saying because these defines merely represent the >>>> valid values for a device tree property and are not translated >>>> into different values you can't put defines for them in the binding >>>> header? >>>> >>>> So this would not be allowed: >>>> >>>> #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 >>>> >>>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>>> >>>> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >>>> regmap_write(regmap, CS2600_OUTPUT_CFG2, val); >>>> >>>> But this would be fine: >>>> >>>> #define CS2600_AUX_OUTPUT_FREQ_UNLOCK 1 >>>> >>>> cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>; >>>> >>>> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >>>> switch (val) { >>>> case CS2600_AUX_OUTPUT_FREQ_UNLOCK: >>>> regmap_write(regmap, CS2600_OUTPUT_CFG2, 0); >>>> } >>>> >>>> And this would also be fine? >>>> >>>> cirrus,aux-output-source = <0>; >>>> >>>> device_property_read_u32(dev, "cirrus,aux-output-source", &val); >>>> regmap_write(regmap, CS2600_OUTPUT_CFG2, val); >>>> >>> Yes. If you want to use in DTS user-readable values, then use string. >>> >> >> I don't understand this. Why should we have to use a string value for >> something that only needs a simple integer value? Why can't we define >> constants with meaningful names? > > You can and you will find plenty examples of this, but as I explained > earlier - this is not a binding. We avoid defining as a binding > something which is not a binding. > What does that mean? Perhaps if you clearly explained what the problem is and what you want us to change instead of making cryptic statements like "this is not a binding" we wouldn't have to waste all this time exchanging emails that aren't getting anywhere. You didn't explain earlier. You typed some words earlier, but they failed to explain, and you are continuing to fail to explain. > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml b/Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml new file mode 100644 index 000000000000..f49f720addf1 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/cirrus,cs2600.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Cirrus Logic Fractional-N Clock Synthesizer & Clock Multiplier + +maintainers: + - Paul Handrigan <paulha@opensource.cirrus.com> + - patches@opensource.cirrus.com> + +description: + 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. + +properties: + compatible: + enum: + - cirrus,cs2600 + + clocks: + maxItems: 2 + + clock-names: + items: + - const: xti + - const: clk_in + + '#clock-cells': + const: 1 + + reg: + maxItems: 1 + + vdd-supply: + description: Power Supply + + clock-output-names: + maxItems: 3 + description: Names for CLK_OUT, BCLK_OUT and FSYNC_OUT clocks. + + cirrus,aux-output-source: + description: + Specifies the function of the auxiliary 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 + default: 0 + +required: + - compatible + - reg + - clocks + - clock-names + - '#clock-cells' + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/cirrus,cs2600.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + clock-controller@2c { + #clock-cells = <1>; + compatible = "cirrus,cs2600"; + reg = <0x2c>; + clocks = <&xtl_clk>, <&sync_clock>; + clock-names = "xti", "clk_in"; + clock-output-names = "audio_clk_out", "audio_bclk", "audio_lrclk"; + vdd-supply = <&vreg>; + }; + }; diff --git a/include/dt-bindings/clock/cirrus,cs2600.h b/include/dt-bindings/clock/cirrus,cs2600.h new file mode 100644 index 000000000000..86065f94a2b2 --- /dev/null +++ b/include/dt-bindings/clock/cirrus,cs2600.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ +/* + * Copyright (c) 2024 Cirrus Logic, Inc. and + * Cirrus Logic International Simiconductor Ltd. + * + * Author: Paul Handrigan <paulha@opensource.cirrus.com> + * + */ + +#ifndef _DT_BINDINGS_CLK_CIRRUS_CS2600_H +#define _DT_BINDINGS_CLK_CIRRUS_CS2600_H + +/* CS2600 Clocks */ +#define CS2600_PLL 0 /* Internal clock */ +#define CS2600_CLK 1 +#define CS2600_BCLK 2 +#define CS2600_FSYNC 3 + +/* CS2600 Auxiliary Output */ +#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0 +#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1 +#define CS2600_AUX_OUTPUT_NO_CLKIN 2 + +#endif
Add device tree support for the Cirrus Logic CS2600 clock device. Signed-off-by: Paul Handrigan <paulha@opensource.cirrus.com> --- .../bindings/clock/cirrus,cs2600.yaml | 81 +++++++++++++++++++ include/dt-bindings/clock/cirrus,cs2600.h | 24 ++++++ 2 files changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml create mode 100644 include/dt-bindings/clock/cirrus,cs2600.h