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

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 13/01/2021 16:09, Mark Brown wrote:
> On Wed, Jan 13, 2021 at 09:22:25AM -0600, Rob Herring wrote:
> 
>> 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.
> 
> Right, I think this should just be the clock binding.
> 

Ok, so if the idea is to do this:

sound {
	clocks = <&audio_mclk>, <&pll>;
	clock-names = "mclk", "pll";
}

some_codec {
	pll: pll {
		compatible = "fixed-clock";
		clocks = <&audio_mclk>;
		clock-frequency = <98304000>;
	}
};

For this to work the clock binding must be a real clock object (so needs
a valid compatible=). But I need to somehow specify the PLL ID and
source pin for the PLL configuration. The schema for "fixed-clock" has
"additionalProperties: false" so I can't add extra custom properties to
the clock node.

Of course if we were able to use the clock framework to provide real
clock drivers for the plls and sysclks, the ID would be inherent in
the binding, and it can define a custom property for the source pin.

Some options:

1) Remove "additionalProperties: false" from the "fixed-clock" binding.

2) Add new core clock properties. Well, source-pin might legitimately be
meaningful, but for a real clock provider the clock ID is implicit.

3) Use 'reg' as fixed-clock doesn't use it. This works, but I suspect it
will be seen as an abuse of reg.

4) Put some extra properties in the sound node to define the <id,source>
pair for each clock. But that's clumsy to have some of the config in a
clock binding and a couple of extra elsewhere.

5) Use a bare clock binding that isn't a real clock provider, like:

sound {
	plls = <&pll>;
}

some_codec {
	pll: pll {
		reg = <1>; /* PLL ID */
	 	audio-graph-card,source-pin = <4>;
		clocks = <&audio_mclk>;
		clock-frequency = <98304000>;
		
	}
};


>>> +      The PLL id and clock source id are specific to the particular component
>>> +      so see the relevant component driver for the ids. Typically the
> 
> This should refer to the bindings for components, not to their drivers.
> 
>>> +      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
>>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel