On 26/10/2020 13:27, Rob Herring wrote:
On Fri, Oct 16, 2020 at 06:35:36PM +0100, Richard Fitzgerald wrote:
This adds the two new properties 'plls' and 'sysclks' to the dt bindings. These add the ability to set values that will be passed to snd_soc_component_set_sysclk() and snd_soc_component_set_pll().
I worry this looks like Linux implementation details leaking into the binding.
I guess what you mean is referring to a function to explain the cells. I thought it would simplify the description but it yes, it does mean that the binding is tied to details of the kernel APIs. I can rewrite this description to be explicit about the cells instead of being in terms of kernel APIs.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com
.../bindings/sound/audio-graph-card.txt | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt b/Documentation/devicetree/bindings/sound/audio-graph-card.txt index d5f6919a2d69..59bbd5b55b59 100644 --- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt +++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt @@ -32,6 +32,19 @@ Required properties: Optional properties:
- pa-gpios: GPIO used to control external amplifier.
+- plls: A list of component pll settings that will be applied with
snd_soc_component_set_pll. Each entry is a phandle to the node of thecodec or cpu component, followed by the four arguments id, source,frequency_in, frequency_out. Multiple entries can have the same phandleso that several plls can be set in the same component.Where do the values of id and source come from?
They are specific to each codec driver, and ultimately depend on the hardware inside the codec. Compare with for example GPIO numbers being specific to hardware. I didn't say that because the description refers to the underlying kernel API, but if I update to not be in terms of an API I'll also add some more info about the fields.
+- sysclks: A list of component sysclk settings that will be applied with
snd_soc_component_set_sysclk. Each entry is a phandle to the node ofthe codec or cpu component, followed by the four arguments id, source,frequency, direction. Direction is 0 if the clock is an input, 1 if itis an output. Multiple entries can have the same phandle so that severalclocks can be set in the same component.Are these really common properties? They seem kind of Cirrus specific and perhaps should be located in the codec node(s).
I'm not sure what about this description makes you think it is Cirrus specific. They are standard ALSA ASoC subsystem APIs. I can find them used in drivers for Analog Devices, Dialog, Realtek and others, and this binding could be used for an audio-graph-card driver using those codecs.
It is the ASoC machine driver (in this case audio-graph-card) that handles this stuff so makes sense for them to be in its node, not the codec driver. The ASoC structure is somewhat complex but in short the codec driver provides an implementation for setting the hardware registers but doesn't know about use-cases or other audio components, so can't decide clocking. The "machine driver" sits above all the audio drivers and has a view of the whole audio subsystem so can decide on use-cases and clocking.
Having said that, we wouldn't need to do this if the kernel clock framework could support clock controllers on I2C/SPI buses. But years have gone by and nobody has managed to fix that yet.
Example: Single DAI case
@@ -335,3 +348,34 @@ Example: Multi DAI with DPCM }; }; };
+----------------------- +Example: Set component sysclks and PLLs +-----------------------
- sound {
compatible = "audio-graph-card";sysclks = <&cs47l15 1 4 98304000 0&cs47l15 8 4 147456000 0>;plls = <&cs47l15 1 0 24576000 98304000>;dais = <&cpu_i2s_port>;- };
- cs47l15: codec@0 {
...ports {#address-cells = <1>;#size-cells = <0>;cs47l15_aif1_port: port@0 {reg = <0>;cs47l15_aif1: endpoint {remote-endpoint = <&cpu_i2s_endpoint>;};};- };
-- 2.20.1