| Message ID | 20201016173541.21180-3-rf@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Rpi4b + Cirrus Lochnagar2 and CS47L15 | expand |
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. > > 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 the > + codec or cpu component, followed by the four arguments id, source, > + frequency_in, frequency_out. Multiple entries can have the same phandle > + so that several plls can be set in the same component. Where do the values of id and source come from? > + > +- 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 of > + the codec or cpu component, followed by the four arguments id, source, > + frequency, direction. Direction is 0 if the clock is an input, 1 if it > + is an output. Multiple entries can have the same phandle so that several > + clocks 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). > + > ----------------------- > 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 >
On Mon, Oct 26, 2020 at 08:27:04AM -0500, Rob Herring wrote: > On Fri, Oct 16, 2020 at 06:35:36PM +0100, Richard Fitzgerald wrote: > > +- 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 the > > + codec or cpu component, followed by the four arguments id, source, > > + frequency_in, frequency_out. Multiple entries can have the same phandle > > + so that several plls can be set in the same component. > Where do the values of id and source come from? The device bindings will need to define them. > > +- 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 of > > + the codec or cpu component, followed by the four arguments id, source, > > + frequency, direction. Direction is 0 if the clock is an input, 1 if it > > + is an output. Multiple entries can have the same phandle so that several > > + clocks 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). It's very common for audio devices to have very flexible clocking, to the exetent this is Linux specific it's issues with the clock API not being able to handle clock controllers on buses that need clock control to access rather than conceptually.
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 the >> + codec or cpu component, followed by the four arguments id, source, >> + frequency_in, frequency_out. Multiple entries can have the same phandle >> + so 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 of >> + the codec or cpu component, followed by the four arguments id, source, >> + frequency, direction. Direction is 0 if the clock is an input, 1 if it >> + is an output. Multiple entries can have the same phandle so that several >> + clocks 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 >>
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 the + codec or cpu component, followed by the four arguments id, source, + frequency_in, frequency_out. Multiple entries can have the same phandle + so that several plls can be set in the same component. + +- 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 of + the codec or cpu component, followed by the four arguments id, source, + frequency, direction. Direction is 0 if the clock is an input, 1 if it + is an output. Multiple entries can have the same phandle so that several + clocks can be set in the same component. + ----------------------- 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>; + }; + }; + };
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(). Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- .../bindings/sound/audio-graph-card.txt | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+)