Messages in this thread Patch in this message | | | Date | Thu, 28 Mar 2024 10:01:45 +0100 | Subject | Re: [RFC PATCH v2 3/5] dt-bindings: clock: meson: document A1 SoC audio clock controller driver | From | Krzysztof Kozlowski <> |
| |
On 28/03/2024 02:08, Jan Dakinevich wrote: > Add device tree bindings for A1 SoC audio clock and reset controllers. > > Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com> > ---
> +title: Amlogic A1 Audio Clock Control Unit and Reset Controller > + > +maintainers: > + - Neil Armstrong <neil.armstrong@linaro.org> > + - Jerome Brunet <jbrunet@baylibre.com> > + - Jan Dakinevich <jan.dakinevich@salutedevices.com> > + > +properties: > + compatible: > + enum: > + - amlogic,a1-audio-clkc > + - amlogic,a1-audio2-clkc
What is "2"?
> + > + '#clock-cells': > + const: 1 > + > + '#reset-cells': > + const: 1 > + > + reg: > + maxItems: 1 > + > + clocks: > + minItems: 6 > + maxItems: 7 > + > + clock-names: > + minItems: 6 > + maxItems: 7 > + > +required: > + - compatible > + - '#clock-cells' > + - reg > + - clocks > + - clock-names > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - amlogic,a1-audio-clkc > + then: > + properties: > + clocks: > + items: > + - description: input core clock > + - description: input main peripheral bus clock > + - description: input dds_in > + - description: input fixed pll div2 > + - description: input fixed pll div3 > + - description: input hifi_pll > + - description: input oscillator (usually at 24MHz) > + clocks-names: > + items: > + - const: core > + - const: pclk > + - const: dds_in > + - const: fclk_div2 > + - const: fclk_div3 > + - const: hifi_pll > + - const: xtal > + required: > + - '#reset-cells' > + else: > + properties: > + clocks: > + items: > + - description: input main peripheral bus clock > + - description: input dds_in > + - description: input fixed pll div2 > + - description: input fixed pll div3 > + - description: input hifi_pll > + - description: input oscillator (usually at 24MHz) > + clock-names: > + items: > + - const: pclk > + - const: dds_in > + - const: fclk_div2 > + - const: fclk_div3 > + - const: hifi_pll > + - const: xtal
#reset-cells: false
> + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/amlogic,a1-pll-clkc.h> > + #include <dt-bindings/clock/amlogic,a1-peripherals-clkc.h> > + #include <dt-bindings/clock/amlogic,a1-audio-clkc.h> > + audio {
If there is going to be any new version/resend: soc {
> + #address-cells = <2>; > + #size-cells = <2>; > + > + clkc_audio: audio-clock-controller@fe050000 {
Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation So: clock-controller
> + compatible = "amlogic,a1-audio-clkc"; > + reg = <0x0 0xfe050000 0x0 0xb0>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + clocks = <&clkc_audio2 AUD2_CLKID_AUDIOTOP>, > + <&clkc_periphs CLKID_AUDIO>, > + <&clkc_periphs CLKID_DDS_IN>, > + <&clkc_pll CLKID_FCLK_DIV2>, > + <&clkc_pll CLKID_FCLK_DIV3>, > + <&clkc_pll CLKID_HIFI_PLL>, > + <&xtal>; > + clock-names = "core", > + "pclk", > + "dds_in", > + "fclk_div2", > + "fclk_div3", > + "hifi_pll", > + "xtal"; > + }; > + > + clkc_audio2: audio-clock-controller@fe054800 {
clock-controller (so I expect resend)
> + compatible = "amlogic,a1-audio2-clkc"; > + reg = <0x0 0xfe054800 0x0 0x20>; > + #clock-cells = <1>; > + clocks = <&clkc_periphs CLKID_AUDIO>, > + <&clkc_periphs CLKID_DDS_IN>, > + <&clkc_pll CLKID_FCLK_DIV2>, > + <&clkc_pll CLKID_FCLK_DIV3>, > + <&clkc_pll CLKID_HIFI_PLL>, > + <&xtal>; > + clock-names = "pclk", > + "dds_in", > + "fclk_div2", > + "fclk_div3", > + "hifi_pll", > + "xtal"; > + }; > + }; > diff --git a/include/dt-bindings/clock/amlogic,a1-audio-clkc.h b/include/dt-bindings/clock/amlogic,a1-audio-clkc.h > new file mode 100644 > index 000000000000..b30df3b1ae08 > --- /dev/null > +++ b/include/dt-bindings/clock/amlogic,a1-audio-clkc.h > @@ -0,0 +1,122 @@ > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ > +/* > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved. > + * > + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com> > + */ > + > +#ifndef __A1_AUDIO_CLKC_BINDINGS_H > +#define __A1_AUDIO_CLKC_BINDINGS_H > + > +#define AUD_CLKID_DDR_ARB 1 > +#define AUD_CLKID_TDMIN_A 2 > +#define AUD_CLKID_TDMIN_B 3 > +#define AUD_CLKID_TDMIN_LB 4 Why both clock controllers have the same clocks? This is confusing. It seems you split same block into two!
Best regards, Krzysztof
| |