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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/06/2026 08:27, Joakim Zhang wrote:
> 
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Friday, June 5, 2026 5:24 PM
>> To: Joakim Zhang <joakim.zhang@cixtech.com>; mturquette@baylibre.com;
>> sboyd@kernel.org; bmasney@redhat.com; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de; Gary Yang
>> <gary.yang@cixtech.com>
>> Cc: cix-kernel-upstream <cix-kernel-upstream@cixtech.com>; linux-
>> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>> linux-arm-kernel@lists.infradead.org
>> Subject: Re: [PATCH v2 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss
>> clock controller
>>
>> EXTERNAL EMAIL
>>
>> 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
> OK
> 
>>> +    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.
> Yes, you are right, I will describe these more meaningful.
> 
>>> +
>>> +  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.
> 
> The reset and power domain takes effect on the entire subsystem, i.e., audss can be accessed only after powered on and reset released, including the CRU registers which contains clock/reset/control bits for all device within the audss.
> 
> Because the reset controller probe does not access the hardware, while the clock controller does, so at that time, the power domain and reset were placed in the clock driver. At present, it does not seem very reasonable either. 
> 
> Linking the "reset" and "power domain" to the parent node requires us to ensure the order of the probes. We need to perform deferred probes within the child nodes until the parent node has been probed.
> 

Please wrap your replies.

You refer here to probe, so driver design, but I did not ask about that.
I asked about hardware design.

Best regards,
Krzysztof