| Message ID | 20231215144005.934728-11-elinor.montmasson@savoirfairelinux.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller | expand |
On 15/12/2023 15:40, Elinor Montmasson wrote: > Add documentation about new dts bindings following new support > for compatible "fsl,imx-audio-generic". Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time, thus I will skip this patch entirely till you follow the process allowing the patch to be tested. Please kindly resend and include all necessary To/Cc entries. > > Some CPU DAI don't require a real audio codec. The new compatible > "fsl,imx-audio-generic" allows using the driver with codec drivers > SPDIF DIT and SPDIF DIR as dummy codecs. > It also allows using not pre-configured audio codecs which > don't require specific control through a codec driver. > > The new dts properties give the possibility to set some parameters > about the CPU DAI usually set through the codec configuration. > > Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com> > Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com> > --- > .../bindings/sound/fsl-asoc-card.txt | 28 ++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > index 4e8dbc5abfd1..f137ef2154e3 100644 > --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > @@ -17,6 +17,9 @@ Note: The card is initially designed for those sound cards who use AC'97, I2S > and PCM DAI formats. However, it'll be also possible to support those non > AC'97/I2S/PCM type sound cards, such as S/PDIF audio and HDMI audio, as > long as the driver has been properly upgraded. > + To use CPU DAIs that do not require a codec such as an S/PDIF controller, > + or to use a DAI to output or capture raw I2S/TDM data, you can > + use the compatible "fsl,imx-audio-generic". > > > The compatible list for this generic sound card currently: > @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently: > > "fsl,imx-audio-nau8822" > > + "fsl,imx-audio-generic" Generic does not look like hardware specific. Best regards, Krzysztof
Hello, > > Add documentation about new dts bindings following new support > > for compatible "fsl,imx-audio-generic". > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. I saw that most of the commits use "ASoC: dt-bindings:" prefix, but commits related to "fsl-asoc-card.txt" use "ASoC: bindings:" prefix. Should I follow the general style or the file style ? > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > You missed at least devicetree list (maybe more), so this won't be > tested by automated tooling. Performing review on untested code might be > a waste of time, thus I will skip this patch entirely till you follow > the process allowing the patch to be tested. > > Please kindly resend and include all necessary To/Cc entries. Sorry for the mistake, I will resend patches with the correct list from the get_maintainers.pl script. > > Some CPU DAI don't require a real audio codec. The new compatible > > "fsl,imx-audio-generic" allows using the driver with codec drivers > > SPDIF DIT and SPDIF DIR as dummy codecs. > > It also allows using not pre-configured audio codecs which > > don't require specific control through a codec driver. > > > > The new dts properties give the possibility to set some parameters > > about the CPU DAI usually set through the codec configuration. > > > > Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com> > > Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com> > > --- > > .../bindings/sound/fsl-asoc-card.txt | 28 ++++++++++++++++++- > > 1 file changed, 27 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > > index 4e8dbc5abfd1..f137ef2154e3 100644 > > --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > > +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt > > @@ -17,6 +17,9 @@ Note: The card is initially designed for those sound cards who use AC'97, I2S > > and PCM DAI formats. However, it'll be also possible to support those non > > AC'97/I2S/PCM type sound cards, such as S/PDIF audio and HDMI audio, as > > long as the driver has been properly upgraded. > > + To use CPU DAIs that do not require a codec such as an S/PDIF controller, > > + or to use a DAI to output or capture raw I2S/TDM data, you can > > + use the compatible "fsl,imx-audio-generic". > > > > > > The compatible list for this generic sound card currently: > > @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently: > > > > "fsl,imx-audio-nau8822" > > > > + "fsl,imx-audio-generic" > > Generic does not look like hardware specific. Even if our end goal is to use it with the S/PDIF controller, this new support can be used with different hardware that doesn't require a codec. Thus, we don't really want to specify "spdif" in it. Is this compatible string not suitable ? Should we rename it to something else, like "fsl,imx-audio-no-codec" ? Best regards, Elinor Montmasson
On 18/12/2023 10:49, Elinor Montmasson wrote: > Hello, > >>> Add documentation about new dts bindings following new support >>> for compatible "fsl,imx-audio-generic". >> >> Please use subject prefixes matching the subsystem. You can get them for >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >> your patch is touching. > > I saw that most of the commits use "ASoC: dt-bindings:" prefix, but This one, plEASE. > commits related to "fsl-asoc-card.txt" use "ASoC: bindings:" prefix. > Should I follow the general style or the file style ? General style. ... >>> The compatible list for this generic sound card currently: >>> @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently: >>> >>> "fsl,imx-audio-nau8822" >>> >>> + "fsl,imx-audio-generic" >> >> Generic does not look like hardware specific. > > Even if our end goal is to use it with the S/PDIF controller, this new > support can be used with different hardware that doesn't > require a codec. Thus, we don't really want to specify "spdif" in it. > > Is this compatible string not suitable ? > Should we rename it to something else, like "fsl,imx-audio-no-codec" ? Maybe Mark or Rob will help here, but for me "imx-audio" is just way too generic. Also, you add several new properties, so I really expect either converting old binding to DT schema first or adding new device in DT schema format. Best regards, Krzysztof
On Mon, Dec 18, 2023 at 10:51:44AM +0100, Krzysztof Kozlowski wrote: > On 18/12/2023 10:49, Elinor Montmasson wrote: > > Is this compatible string not suitable ? > > Should we rename it to something else, like "fsl,imx-audio-no-codec" ? > Maybe Mark or Rob will help here, but for me "imx-audio" is just way too > generic. I think it's fine. > Also, you add several new properties, so I really expect either > converting old binding to DT schema first or adding new device in DT > schema format. So long as the binding conversion can go through quickly...
On Monday, 18 December, 2023 15:04:42, Mark Brown wrote: > On Mon, Dec 18, 2023 at 10:51:44AM +0100, Krzysztof Kozlowski wrote: > > On 18/12/2023 10:49, Elinor Montmasson wrote: > > Also, you add several new properties, so I really expect either > > converting old binding to DT schema first or adding new device in DT > > schema format. > > So long as the binding conversion can go through quickly... I can take some time next week to do the conversion, then I'll send this additional commit in a v4 patch series. > > > Is this compatible string not suitable ? > > > Should we rename it to something else, like "fsl,imx-audio-no-codec" ? > > > Maybe Mark or Rob will help here, but for me "imx-audio" is just way too > > generic. > > I think it's fine. I will keep the compatible name to "fsl,imx-audio-generic" for now, but if it is needed to change it, tell me and I will do it in the v4 patch series. Thank you for the review. Best regards, Elinor Montmasson
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt index 4e8dbc5abfd1..f137ef2154e3 100644 --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt @@ -17,6 +17,9 @@ Note: The card is initially designed for those sound cards who use AC'97, I2S and PCM DAI formats. However, it'll be also possible to support those non AC'97/I2S/PCM type sound cards, such as S/PDIF audio and HDMI audio, as long as the driver has been properly upgraded. + To use CPU DAIs that do not require a codec such as an S/PDIF controller, + or to use a DAI to output or capture raw I2S/TDM data, you can + use the compatible "fsl,imx-audio-generic". The compatible list for this generic sound card currently: @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently: "fsl,imx-audio-nau8822" + "fsl,imx-audio-generic" + Required properties: - compatible : Contains one of entries in the compatible list. @@ -56,7 +61,11 @@ Required properties: - audio-cpu : The phandle of an CPU DAI controller - - audio-codec : The phandle of an audio codec + - audio-codec : The phandle of an audio codec. + If using the "fsl,imx-audio-generic" compatible, + give instead a pair of phandles with the + spdif_transmitter first (driver SPDIF DIT) and the + spdif_receiver second (driver SPDIF DIR). Optional properties: @@ -87,6 +96,23 @@ Optional properties: - frame-inversion : dai-link uses frame clock inversion, for details see simple-card.yaml. - bitclock-inversion : dai-link uses bit clock inversion, for details see simple-card.yaml. - mclk-id : main clock id, specific for each card configuration. + For multi-codec configurations, an array of ids can be + given, one for each codec. + +Optional, relevant only with the "fsl,imx-audio-generic" compatible: + + - cpu-slot-width : Indicates a specific TDM slot width in bits. + - cpu-slot-num : Indicates a specific number of TDM slots per frame. + + - cpu-sysclk-freq-rx : Frequency of the CPU DAI sys clock for Rx. + - cpu-sysclk-freq-tx : Frequency of the CPU DAI sys clock for Tx. + + - cpu-sysclk-dir-rx-out : Boolean property. Specifies sys clock direction + as 'out' on initialization for Rx. + If not set, default direction is 'in'. + - cpu-sysclk-dir-tx-out : Boolean property. Specifies sys clock direction + as 'out' on initialization for Tx. + If not set, default direction is 'in'. Optional unless SSI is selected as a CPU DAI:
Add documentation about new dts bindings following new support for compatible "fsl,imx-audio-generic". Some CPU DAI don't require a real audio codec. The new compatible "fsl,imx-audio-generic" allows using the driver with codec drivers SPDIF DIT and SPDIF DIR as dummy codecs. It also allows using not pre-configured audio codecs which don't require specific control through a codec driver. The new dts properties give the possibility to set some parameters about the CPU DAI usually set through the codec configuration. Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com> Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com> --- .../bindings/sound/fsl-asoc-card.txt | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)