2024-05-15 13:56:18

by Elinor Montmasson

[permalink] [raw]
Subject: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

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 <[email protected]>
---
.../bindings/sound/fsl-asoc-card.yaml | 96 ++++++++++++++++++-
1 file changed, 92 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
index 9922664d5ccc..332d8bf96e06 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
@@ -23,6 +23,16 @@ description:
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".
+
+definitions:
+ imx-audio-generic-dependency:
+ properties:
+ compatible:
+ contains:
+ const: fsl,imx-audio-generic

maintainers:
- Shengjiu Wang <[email protected]>
@@ -81,6 +91,7 @@ properties:
- fsl,imx-audio-wm8960
- fsl,imx-audio-wm8962
- fsl,imx-audio-wm8958
+ - fsl,imx-audio-generic

model:
$ref: /schemas/types.yaml#/definitions/string
@@ -93,8 +104,14 @@ properties:
need to add ASRC support via DPCM.

audio-codec:
- $ref: /schemas/types.yaml#/definitions/phandle
- description: The phandle of an audio codec
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ 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).
+ items:
+ maxItems: 1

audio-cpu:
$ref: /schemas/types.yaml#/definitions/phandle
@@ -150,8 +167,8 @@ properties:
description: dai-link uses bit clock inversion.

mclk-id:
- $ref: /schemas/types.yaml#/definitions/uint32
- description: main clock id, specific for each card configuration.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: Main clock id for each codec, specific for each card configuration.

mux-int-port:
$ref: /schemas/types.yaml#/definitions/uint32
@@ -167,10 +184,68 @@ properties:
$ref: /schemas/types.yaml#/definitions/phandle
description: The phandle of an CPU DAI controller

+ # Properties relevant only with "fsl,imx-audio-generic" compatible
+ dai-tdm-slot-width:
+ description: See tdm-slot.txt.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ dai-tdm-slot-num:
+ description: See tdm-slot.txt.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ clocks:
+ description: |
+ The CPU DAI system clock, used to retrieve
+ the CPU DAI system clock frequency with the generic codec.
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: cpu_sysclk
+
+ cpu-system-clock-direction-out:
+ description: |
+ Specifies cpu system clock direction as 'out' on initialization.
+ If not set, direction is 'in'.
+ $ref: /schemas/types.yaml#/definitions/flag
+
+dependencies:
+ dai-tdm-slot-width:
+ $ref: "#/definitions/imx-audio-generic-dependency"
+ dai-tdm-slot-num:
+ $ref: "#/definitions/imx-audio-generic-dependency"
+ clocks:
+ $ref: "#/definitions/imx-audio-generic-dependency"
+ cpu-system-clock-direction-out:
+ $ref: "#/definitions/imx-audio-generic-dependency"
+
required:
- compatible
- model

+allOf:
+ - if:
+ $ref: "#/definitions/imx-audio-generic-dependency"
+ then:
+ properties:
+ audio-codec:
+ items:
+ - description: SPDIF DIT phandle
+ - description: SPDIF DIR phandle
+ mclk-id:
+ maxItems: 1
+ items:
+ minItems: 1
+ maxItems: 2
+ else:
+ properties:
+ audio-codec:
+ maxItems: 1
+ mclk-id:
+ maxItems: 1
+ items:
+ maxItems: 1
+
unevaluatedProperties: false

examples:
@@ -195,3 +270,16 @@ examples:
"AIN2L", "Line In Jack",
"AIN2R", "Line In Jack";
};
+
+ - |
+ #include <dt-bindings/clock/imx8mn-clock.h>
+ sound-spdif-asrc {
+ compatible = "fsl,imx-audio-generic";
+ model = "spdif-asrc-audio";
+ audio-cpu = <&spdif>;
+ audio-asrc = <&easrc>;
+ audio-codec = <&spdifdit>, <&spdifdir>;
+ clocks = <&clk IMX8MN_CLK_SAI5_ROOT>;
+ clock-names = "cpu_sysclk";
+ cpu-system-clock-direction-out;
+ };
--
2.34.1



2024-05-16 12:11:34

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:

> Add documentation about new dts bindings following new support
> for compatible "fsl,imx-audio-generic".

> audio-codec:
> - $ref: /schemas/types.yaml#/definitions/phandle
> - description: The phandle of an audio codec
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + 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).
> + items:
> + maxItems: 1

This description (and the code) don't feel like they're actually generic
- they're clearly specific to the bidrectional S/PDIF case. I'd expect
something called -generic to cope with single CODECs as well as double,
and not to have any constraints on what those are.


Attachments:
(No filename) (955.00 B)
signature.asc (499.00 B)
Download all attachments

2024-05-17 09:06:24

by Elinor Montmasson

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

From: "Mark Brown" <[email protected]>
Sent: Thursday, 16 May, 2024 14:11:11
> On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:
>
>> Add documentation about new dts bindings following new support
>> for compatible "fsl,imx-audio-generic".
>
>> audio-codec:
>> - $ref: /schemas/types.yaml#/definitions/phandle
>> - description: The phandle of an audio codec
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + 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).
>> + items:
>> + maxItems: 1
>
> This description (and the code) don't feel like they're actually generic
> - they're clearly specific to the bidrectional S/PDIF case. I'd expect
> something called -generic to cope with single CODECs as well as double,
> and not to have any constraints on what those are.

I proposed, in an reply of the v3 patch series to Krzysztof Kozlowski,
the compatible "fsl,imx-audio-no-codec" instead of "generic".
Krzysztof thought it was too generic, but it would convey more clearly
that it is for cases without codec driver.
Would this other compatible string be more appropriate ?

Best regards,
Elinor Montmasson

2024-05-17 11:11:56

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

On Fri, May 17, 2024 at 05:05:41AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <[email protected]>

> > This description (and the code) don't feel like they're actually generic
> > - they're clearly specific to the bidrectional S/PDIF case. I'd expect
> > something called -generic to cope with single CODECs as well as double,
> > and not to have any constraints on what those are.

> I proposed, in an reply of the v3 patch series to Krzysztof Kozlowski,
> the compatible "fsl,imx-audio-no-codec" instead of "generic".
> Krzysztof thought it was too generic, but it would convey more clearly
> that it is for cases without codec driver.
> Would this other compatible string be more appropriate ?

No. There is very clearly a CODEC here, it physically exists, we can
point at it on the board and it has a software representation. Your
code is also very specific to the two CODEC case.


Attachments:
(No filename) (916.00 B)
signature.asc (499.00 B)
Download all attachments

2024-05-20 18:31:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:
> 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 <[email protected]>
> ---
> .../bindings/sound/fsl-asoc-card.yaml | 96 ++++++++++++++++++-
> 1 file changed, 92 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> index 9922664d5ccc..332d8bf96e06 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> @@ -23,6 +23,16 @@ description:
> 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".
> +
> +definitions:
> + imx-audio-generic-dependency:
> + properties:
> + compatible:
> + contains:
> + const: fsl,imx-audio-generic
>
> maintainers:
> - Shengjiu Wang <[email protected]>
> @@ -81,6 +91,7 @@ properties:
> - fsl,imx-audio-wm8960
> - fsl,imx-audio-wm8962
> - fsl,imx-audio-wm8958
> + - fsl,imx-audio-generic
>
> model:
> $ref: /schemas/types.yaml#/definitions/string
> @@ -93,8 +104,14 @@ properties:
> need to add ASRC support via DPCM.
>
> audio-codec:
> - $ref: /schemas/types.yaml#/definitions/phandle
> - description: The phandle of an audio codec
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + 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).

minItems: 1
maxItems: 2

> + items:
> + maxItems: 1
>
> audio-cpu:
> $ref: /schemas/types.yaml#/definitions/phandle
> @@ -150,8 +167,8 @@ properties:
> description: dai-link uses bit clock inversion.
>
> mclk-id:
> - $ref: /schemas/types.yaml#/definitions/uint32
> - description: main clock id, specific for each card configuration.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: Main clock id for each codec, specific for each card configuration.

minItems: 1
maxItems: 2
>
> mux-int-port:
> $ref: /schemas/types.yaml#/definitions/uint32
> @@ -167,10 +184,68 @@ properties:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: The phandle of an CPU DAI controller
>
> + # Properties relevant only with "fsl,imx-audio-generic" compatible
> + dai-tdm-slot-width:
> + description: See tdm-slot.txt.
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + dai-tdm-slot-num:
> + description: See tdm-slot.txt.
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + clocks:
> + description: |
> + The CPU DAI system clock, used to retrieve
> + the CPU DAI system clock frequency with the generic codec.
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: cpu_sysclk
> +
> + cpu-system-clock-direction-out:
> + description: |
> + Specifies cpu system clock direction as 'out' on initialization.
> + If not set, direction is 'in'.
> + $ref: /schemas/types.yaml#/definitions/flag
> +
> +dependencies:
> + dai-tdm-slot-width:
> + $ref: "#/definitions/imx-audio-generic-dependency"
> + dai-tdm-slot-num:
> + $ref: "#/definitions/imx-audio-generic-dependency"
> + clocks:
> + $ref: "#/definitions/imx-audio-generic-dependency"
> + cpu-system-clock-direction-out:
> + $ref: "#/definitions/imx-audio-generic-dependency"

This works, but is an unusual pattern...

> +
> required:
> - compatible
> - model
>
> +allOf:
> + - if:
> + $ref: "#/definitions/imx-audio-generic-dependency"
> + then:
> + properties:
> + audio-codec:
> + items:
> + - description: SPDIF DIT phandle
> + - description: SPDIF DIR phandle
> + mclk-id:
> + maxItems: 1
> + items:
> + minItems: 1
> + maxItems: 2
> + else:
> + properties:
> + audio-codec:
> + maxItems: 1
> + mclk-id:
> + maxItems: 1
> + items:
> + maxItems: 1


You can handle the dependency like this instead:

dai-tdm-slot-width: false
dai-tdm-slot-num: false


And then you don't need the definitions.

> +
> unevaluatedProperties: false
>
> examples:
> @@ -195,3 +270,16 @@ examples:
> "AIN2L", "Line In Jack",
> "AIN2R", "Line In Jack";
> };
> +
> + - |
> + #include <dt-bindings/clock/imx8mn-clock.h>
> + sound-spdif-asrc {
> + compatible = "fsl,imx-audio-generic";
> + model = "spdif-asrc-audio";
> + audio-cpu = <&spdif>;
> + audio-asrc = <&easrc>;
> + audio-codec = <&spdifdit>, <&spdifdir>;
> + clocks = <&clk IMX8MN_CLK_SAI5_ROOT>;
> + clock-names = "cpu_sysclk";
> + cpu-system-clock-direction-out;
> + };
> --
> 2.34.1
>

2024-05-31 12:48:15

by Elinor Montmasson

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

From: "Mark Brown" <[email protected]>
Sent: Friday, 17 May, 2024 13:11:43
> On Fri, May 17, 2024 at 05:05:41AM -0400, Elinor Montmasson wrote:
>> From: "Mark Brown" <[email protected]>
>
>> > This description (and the code) don't feel like they're actually generic
>> > - they're clearly specific to the bidrectional S/PDIF case. I'd expect
>> > something called -generic to cope with single CODECs as well as double,
>> > and not to have any constraints on what those are.
>
>> I proposed, in an reply of the v3 patch series to Krzysztof Kozlowski,
>> the compatible "fsl,imx-audio-no-codec" instead of "generic".
>> Krzysztof thought it was too generic, but it would convey more clearly
>> that it is for cases without codec driver.
>> Would this other compatible string be more appropriate ?
>
> No. There is very clearly a CODEC here, it physically exists, we can
> point at it on the board and it has a software representation. Your
> code is also very specific to the two CODEC case.

Then maybe it's not be a good idea to make this compatible generic
for this contribution.
The original intention is to bring support for the S/PDIF,
so maybe the contribution should focus on this use case?
In that case, would changing the compatible for "fsl,imx-audio-spdif-card"
be acceptable?
"fsl,imx-audio-spdif" is already used for the `imx-spdif.c`
which does not use the ASRC.

2024-05-31 12:48:30

by Elinor Montmasson

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

From: "Rob Herring" <[email protected]>
Sent: Monday, 20 May, 2024 20:31:48
> On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:
>> 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 <[email protected]>
>> ---
>> .../bindings/sound/fsl-asoc-card.yaml | 96 ++++++++++++++++++-
>> 1 file changed, 92 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> index 9922664d5ccc..332d8bf96e06 100644
>> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> @@ -23,6 +23,16 @@ description:
>> 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".
>> +
>> +definitions:
>> + imx-audio-generic-dependency:
>> + properties:
>> + compatible:
>> + contains:
>> + const: fsl,imx-audio-generic
>>
>> maintainers:
>> - Shengjiu Wang <[email protected]>
>> @@ -81,6 +91,7 @@ properties:
>> - fsl,imx-audio-wm8960
>> - fsl,imx-audio-wm8962
>> - fsl,imx-audio-wm8958
>> + - fsl,imx-audio-generic
>>
>> model:
>> $ref: /schemas/types.yaml#/definitions/string
>> @@ -93,8 +104,14 @@ properties:
>> need to add ASRC support via DPCM.
>>
>> audio-codec:
>> - $ref: /schemas/types.yaml#/definitions/phandle
>> - description: The phandle of an audio codec
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + 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).
>
> minItems: 1
> maxItems: 2
>
>> + items:
>> + maxItems: 1
>>
>> audio-cpu:
>> $ref: /schemas/types.yaml#/definitions/phandle
>> @@ -150,8 +167,8 @@ properties:
>> description: dai-link uses bit clock inversion.
>>
>> mclk-id:
>> - $ref: /schemas/types.yaml#/definitions/uint32
>> - description: main clock id, specific for each card configuration.
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description: Main clock id for each codec, specific for each card
>> configuration.
>
> minItems: 1
> maxItems: 2
>>
>> mux-int-port:
>> $ref: /schemas/types.yaml#/definitions/uint32
>> @@ -167,10 +184,68 @@ properties:
>> $ref: /schemas/types.yaml#/definitions/phandle
>> description: The phandle of an CPU DAI controller
>>
>> + # Properties relevant only with "fsl,imx-audio-generic" compatible
>> + dai-tdm-slot-width:
>> + description: See tdm-slot.txt.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + dai-tdm-slot-num:
>> + description: See tdm-slot.txt.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> + clocks:
>> + description: |
>> + The CPU DAI system clock, used to retrieve
>> + the CPU DAI system clock frequency with the generic codec.
>> + maxItems: 1
>> +
>> + clock-names:
>> + items:
>> + - const: cpu_sysclk
>> +
>> + cpu-system-clock-direction-out:
>> + description: |
>> + Specifies cpu system clock direction as 'out' on initialization.
>> + If not set, direction is 'in'.
>> + $ref: /schemas/types.yaml#/definitions/flag
>> +
>> +dependencies:
>> + dai-tdm-slot-width:
>> + $ref: "#/definitions/imx-audio-generic-dependency"
>> + dai-tdm-slot-num:
>> + $ref: "#/definitions/imx-audio-generic-dependency"
>> + clocks:
>> + $ref: "#/definitions/imx-audio-generic-dependency"
>> + cpu-system-clock-direction-out:
>> + $ref: "#/definitions/imx-audio-generic-dependency"
>
> This works, but is an unusual pattern...
>
>> +
>> required:
>> - compatible
>> - model
>>
>> +allOf:
>> + - if:
>> + $ref: "#/definitions/imx-audio-generic-dependency"
>> + then:
>> + properties:
>> + audio-codec:
>> + items:
>> + - description: SPDIF DIT phandle
>> + - description: SPDIF DIR phandle
>> + mclk-id:
>> + maxItems: 1
>> + items:
>> + minItems: 1
>> + maxItems: 2
>> + else:
>> + properties:
>> + audio-codec:
>> + maxItems: 1
>> + mclk-id:
>> + maxItems: 1
>> + items:
>> + maxItems: 1
>
>
> You can handle the dependency like this instead:
>
> dai-tdm-slot-width: false
> dai-tdm-slot-num: false
>
>
> And then you don't need the definitions.
>
>> +
>> unevaluatedProperties: false
>>
>> examples:
>> @@ -195,3 +270,16 @@ examples:
>> "AIN2L", "Line In Jack",
>> "AIN2R", "Line In Jack";
>> };
>> +
>> + - |
>> + #include <dt-bindings/clock/imx8mn-clock.h>
>> + sound-spdif-asrc {
>> + compatible = "fsl,imx-audio-generic";
>> + model = "spdif-asrc-audio";
>> + audio-cpu = <&spdif>;
>> + audio-asrc = <&easrc>;
>> + audio-codec = <&spdifdit>, <&spdifdir>;
>> + clocks = <&clk IMX8MN_CLK_SAI5_ROOT>;
>> + clock-names = "cpu_sysclk";
>> + cpu-system-clock-direction-out;
>> + };
>> --
>> 2.34.1

Ok, thank you for the review, I'll make these modifications in v5.

2024-05-31 13:17:37

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

On Fri, May 31, 2024 at 08:48:04AM -0400, Elinor Montmasson wrote:

> Then maybe it's not be a good idea to make this compatible generic
> for this contribution.
> The original intention is to bring support for the S/PDIF,
> so maybe the contribution should focus on this use case?
> In that case, would changing the compatible for "fsl,imx-audio-spdif-card"
> be acceptable?
> "fsl,imx-audio-spdif" is already used for the `imx-spdif.c`
> which does not use the ASRC.

Why not just use the existing compatible - why would someone not want to
be able to use the ASRC if it's available in their system?


Attachments:
(No filename) (615.00 B)
signature.asc (499.00 B)
Download all attachments

2024-05-31 14:48:40

by Elinor Montmasson

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

From: "Mark Brown" <[email protected]>
Sent: Friday, 31 May, 2024 15:14:13

> On Fri, May 31, 2024 at 08:48:04AM -0400, Elinor Montmasson wrote:
>
>> Then maybe it's not be a good idea to make this compatible generic
>> for this contribution.
>> The original intention is to bring support for the S/PDIF,
>> so maybe the contribution should focus on this use case?
>> In that case, would changing the compatible for "fsl,imx-audio-spdif-card"
>> be acceptable?
>> "fsl,imx-audio-spdif" is already used for the `imx-spdif.c`
>> which does not use the ASRC.
>
> Why not just use the existing compatible - why would someone not want to
> be able to use the ASRC if it's available in their system?

That's true but it will be a problem if both `fsl-asoc-card.c` and
`imx-spdif.c` drivers have the same compatible, and they don't
have the same DT properties.

2024-05-31 16:08:23

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

On Fri, May 31, 2024 at 10:48:14AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <[email protected]>

> > Why not just use the existing compatible - why would someone not want to
> > be able to use the ASRC if it's available in their system?

> That's true but it will be a problem if both `fsl-asoc-card.c` and
> `imx-spdif.c` drivers have the same compatible, and they don't
> have the same DT properties.

So merge the two then?


Attachments:
(No filename) (451.00 B)
signature.asc (499.00 B)
Download all attachments

2024-06-06 15:39:47

by Elinor Montmasson

[permalink] [raw]
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

> From: "Mark Brown" <[email protected]>
> Sent: Friday, 31 May, 2024 18:06:30

> On Fri, May 31, 2024 at 10:48:14AM -0400, Elinor Montmasson wrote:
>> From: "Mark Brown" <[email protected]>
>
>> > Why not just use the existing compatible - why would someone not want to
>> > be able to use the ASRC if it's available in their system?
>
>> That's true but it will be a problem if both `fsl-asoc-card.c` and
>> `imx-spdif.c` drivers have the same compatible, and they don't
>> have the same DT properties.
>
> So merge the two then?

It would avoid having duplicate drivers yes, I will do this for the v5 of this contribution.
Thank you for the review.