diff mbox series

[PATCHv3,10/10] ASoC: bindings: fsl-asoc-card: add compatible for generic codec

Message ID 20231215144005.934728-11-elinor.montmasson@savoirfairelinux.com
State Superseded
Headers show
Series ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller | expand

Commit Message

Elinor Montmasson Dec. 15, 2023, 2:40 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski Dec. 18, 2023, 8:34 a.m. UTC | #1
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
Krzysztof Kozlowski Dec. 18, 2023, 9:51 a.m. UTC | #2
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
Mark Brown Dec. 18, 2023, 2:04 p.m. UTC | #3
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...
Elinor Montmasson Dec. 19, 2023, 12:33 p.m. UTC | #4
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 mbox series

Patch

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: