Re: [PATCH v4 2/6] dt-bindings: audio-graph-card: Add plls and sysclks properties

From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: Rob Herring <robh@kernel.org>
Cc: <broonie@kernel.org>, <kuninori.morimoto.gx@renesas.com>,
	<nsaenzjulienne@suse.de>, <f.fainelli@gmail.com>,
	<linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<alsa-devel@alsa-project.org>, <patches@opensource.cirrus.com>,
	<bcm-kernel-feedback-list@broadcom.com>,
	<linux-rpi-kernel@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v4 2/6] dt-bindings: audio-graph-card: Add plls and sysclks properties
Date: Thu, 14 Jan 2021 10:31:10 +0000	[thread overview]
Message-ID: <c12a846f-9e79-4646-e7f4-397f074eb613@opensource.cirrus.com> (raw)
In-Reply-To: <20210113152225.GA2334778@robh.at.kernel.org>

On 13/01/2021 15:22, Rob Herring wrote:
> On Fri, Jan 08, 2021 at 04:04:57PM +0000, Richard Fitzgerald wrote:
>> The audio-graph-card driver has properties for configuring the clocking
>> for DAIs within a component, but is missing properties for setting
>> up the PLLs and sysclks of the component.
>>
>> This patch adds the two new properties 'plls' and 'sysclks' so that the
>> audio-graph-driver can fully configure the component clocking.
> 
> I'm not sure this makes sense to be generic, but if so, we already have
> the clock binding and should use (and possibly extend) that.
> 
> This appears to all be configuration of clocks within the codec, so
> these properties belong in the codec or cpu nodes.
>

audio-graph-card doesn't have codec or cpu nodes. Those were in
simple-card but are replaced in audio-graph-card by a simple phandle
array forming a graph.

I could assume that all clock settings apply to the codec and that there
is only ever one codec in an audio-graph-card configuration.

>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> ---
>>   .../bindings/sound/audio-graph.yaml           | 46 +++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/audio-graph.yaml b/Documentation/devicetree/bindings/sound/audio-graph.yaml
>> index 4b46794e5153..9e0819205a17 100644
>> --- a/Documentation/devicetree/bindings/sound/audio-graph.yaml
>> +++ b/Documentation/devicetree/bindings/sound/audio-graph.yaml
>> @@ -39,6 +39,52 @@ properties:
>>     mic-det-gpio:
>>       maxItems: 1
>>   
>> +  plls:
>> +    description: |
>> +      A list of component pll settings. There are 4 cells per PLL setting:
>> +        - phandle to the node of the codec or cpu component,
>> +        - component PLL id,
>> +        - component clock source id,
>> +        - frequency (in Hz) of the PLL output clock.
> 
> assigned-clocks binding can set frequencies and parent clocks.
> 
> 'pll' is too specific to the implementation. You may want to configure
> the freq and parent of something that's not a pll.
> 
>> +      The PLL id and clock source id are specific to the particular component
>> +      so see the relevant component driver for the ids. Typically the
>> +      clock source id indicates the pin the source clock is connected to.
>> +      The same phandle can appear in multiple entries so that several plls
>> +      can be set in the same component.
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +
>> +  plls-clocks:
>> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> +    description: |
>> +      A list of clock names giving the source clock for each setting
>> +      in the plls property.
>> +
>> +  sysclks:
>> +    description: |
>> +      A list of component sysclk settings. There are 4 cells per sysclk
>> +      setting:
>> +        - phandle to the node of the codec or cpu component,
>> +        - component sysclk id,
>> +        - component clock source id,
>> +        - direction of the clock: 0 if the clock is an input to the component,
>> +          1 if it is an output.
> 
> A clock provider and consumer would provide the direction.
> 
>> +      The sysclk id and clock source id are specific to the particular
>> +      component so see the relevant component driver for the ids. Typically
>> +      the clock source id indicates the pin the source clock is connected to.
>> +      The same phandle can appear in multiple entries so that several sysclks
>> +      can be set in the same component.
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +
>> +  sysclks-clocks:
>> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> +    description: |
>> +      A list of clock names giving the source clock for each setting
>> +      in the sysclks property.
>> +
>> +dependencies:
>> +  plls: [ plls-clocks ]
>> +  sysclks: [ sysclks-clocks ]
>> +
>>   required:
>>     - dais
>>   
>> -- 
>> 2.20.1
>>