Re: [PATCH v2 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On 05/06/2026 05:22, joakim.zhang@cixtech.com wrote:
> +description: |
> +  Clock provider for the Cix Sky1 audio subsystem (AUDSS).
> +
> +  This node is a child of a cix,sky1-audss-system-control MFD/syscon node
> +  (see cix,sky1-system-control.yaml). It does not have a reg property; clock
> +  mux, divider and gate fields are accessed through the parent register block.
> +
> +  Software reset lines for AUDSS blocks are exposed on the parent syscon via
> +  #reset-cells. Reset indices are defined in
> +  include/dt-bindings/reset/cix,sky1-audss-system-control.h.
> +
> +  Six SoC-level reference clocks listed in clocks/clock-names feed the AUDSS
> +  clock tree. The provider exposes the internal AUDSS clocks to other devices
> +  via #clock-cells; indices are defined in cix,sky1-audss.h.
> +
> +properties:
> +  compatible:
> +    const: cix,sky1-audss-clock
> +
> +  '#clock-cells':
> +    const: 1
> +    description:
> +      Clock indices are defined in include/dt-bindings/clock/cix,sky1-audss.h.
> +
> +  clocks:
> +    minItems: 6

Drop

> +    maxItems: 6
> +    description:
> +      Six SoC-level audio reference clocks that feed the audio subsystem,
> +      in the same order as clock-names.
> +
> +  clock-names:
> +    items:
> +      - const: audio_clk0
> +      - const: audio_clk1
> +      - const: audio_clk2
> +      - const: audio_clk3
> +      - const: audio_clk4
> +      - const: audio_clk5

Pretty pointless names. Names matching indexes have no benefits, drop
all of them and instead list items in "clocks" with description.

> +
> +  resets:
> +    maxItems: 1
> +    description: Audio subsystem NoC (or bus) reset line.
> +
> +  power-domains:
> +    maxItems: 1
> +    description: Audio subsystem power domain.

So the clock part has power domain but reset part does not? This is odd.
Especially that parent is audss (right?) and here you describe that this
is audss poer domain.

Same question about resets.

> +
> +required:
> +  - compatible
> +  - '#clock-cells'
> +  - clocks
> +  - clock-names
> +  - resets
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/cix,sky1.h>
> +    #include <dt-bindings/reset/cix,sky1-audss-system-control.h>
> +    #include <dt-bindings/reset/cix,sky1-s5-system-control.h>
> +
> +    audss_syscon: system-controller@7110000 {
> +        compatible = "cix,sky1-audss-system-control", "simple-mfd", "syscon";
> +        reg = <0x7110000 0x10000>;
> +        #reset-cells = <1>;

Drop parent node.

> +
> +        audss_clk: clock-controller {
> +            compatible = "cix,sky1-audss-clock";
> +            power-domains = <&smc_devpd 0>;
> +            #clock-cells = <1>;
> +            clocks = <&scmi_clk CLK_TREE_AUDIO_CLK0>, <&scmi_clk CLK_TREE_AUDIO_CLK1>,
> +                     <&scmi_clk CLK_TREE_AUDIO_CLK2>, <&scmi_clk CLK_TREE_AUDIO_CLK3>,
> +                     <&scmi_clk CLK_TREE_AUDIO_CLK4>, <&scmi_clk CLK_TREE_AUDIO_CLK5>;
> +            clock-names = "audio_clk0", "audio_clk1", "audio_clk2",
> +                          "audio_clk3", "audio_clk4", "audio_clk5";
> +            resets = <&src SKY1_AUDIO_HIFI5_NOC_RESET_N>;
> +        };
> +    };



> +#define CLK_MCLK4		40
> +
> +#define AUDSS_MAX_CLKS		41

Drop


Best regards,
Krzysztof