Received-SPF: pass (google.com: domain of linux-kernel+bounces-196821-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 31 May 2024 08:48:15 -0400 (EDT)
From: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
To: Rob Herring <robh@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, 
	Conor Dooley <conor+dt@kernel.org>, 
	shengjiu wang <shengjiu.wang@gmail.com>, 
	Xiubo Lee <Xiubo.Lee@gmail.com>, Fabio Estevam <festevam@gmail.com>, 
	Nicolin Chen <nicoleotsuka@gmail.com>, 
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, 
	linux-sound <linux-sound@vger.kernel.org>, 
	devicetree <devicetree@vger.kernel.org>, 
	linux-kernel <linux-kernel@vger.kernel.org>, 
	alsa-devel <alsa-devel@alsa-project.org>, 
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Message-ID: <1712008549.701259.1717159695483.JavaMail.zimbra@savoirfairelinux.com>
In-Reply-To: <20240520183148.GA1250699-robh@kernel.org>
References: <20240515135411.343333-1-elinor.montmasson@savoirfairelinux.com> <20240515135411.343333-10-elinor.montmasson@savoirfairelinux.com> <20240520183148.GA1250699-robh@kernel.org>
Subject: Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible
 for generic codec
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec
Thread-Index: 9W7FNkO/DHfWsLN9ZmR/Fd4sK0HmGw==

From: "Rob Herring" <robh@kernel.org>
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 <elinor.montmasson@savoirfairelinux.com>
>> ---
>>  .../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 <shengjiu.wang@nxp.com>
>> @@ -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.