lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6fcbd97b-4172-48a9-bcdb-3bdf35aba8f7@kernel.org>
Date: Sun, 23 Jun 2024 13:07:46 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Elinor Montmasson <elinor.montmasson@...oirfairelinux.com>,
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>, Fabio Estevam <festevam@...il.com>,
 Russell King <linux@...linux.org.uk>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
 Shengjiu Wang <shengjiu.wang@...il.com>, Xiubo Li <Xiubo.Lee@...il.com>,
 Nicolin Chen <nicoleotsuka@...il.com>
Cc: Pengutronix Kernel Team <kernel@...gutronix.de>,
 linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
 imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
 linuxppc-dev@...ts.ozlabs.org,
 Philip-Dylan <philip-dylan.gleonec@...oirfairelinux.com>
Subject: Re: [PATCHv5 6/9] ASoC: dt-bindings: fsl-asoc-card: add compatible
 string for spdif

On 20/06/2024 15:25, Elinor Montmasson wrote:
> The S/PDIF audio card support was merged from imx-spdif into the
> fsl-asoc-card driver, making it possible to use an S/PDIF with an ASRC.
> Add the new compatible and update properties.

Please use standard email subjects, so with the PATCH keyword in the
title. `git format-patch -v5` helps here to create proper versioned
patches. Another useful tool is b4.

> 
> Signed-off-by: Elinor Montmasson <elinor.montmasson@...oirfairelinux.com>
> ---
>  .../bindings/sound/fsl-asoc-card.yaml         | 30 ++++++++++++++++---
>  1 file changed, 26 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..f2e28b32808e 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> @@ -33,6 +33,7 @@ properties:
>        - items:
>            - enum:
>                - fsl,imx-sgtl5000
> +              - fsl,imx-sabreauto-spdif
>                - fsl,imx25-pdk-sgtl5000
>                - fsl,imx53-cpuvo-sgtl5000
>                - fsl,imx51-babbage-sgtl5000
> @@ -54,6 +55,7 @@ properties:
>                - fsl,imx6q-ventana-sgtl5000
>                - fsl,imx6sl-evk-wm8962
>                - fsl,imx6sx-sdb-mqs
> +              - fsl,imx6sx-sdb-spdif
>                - fsl,imx6sx-sdb-wm8962
>                - fsl,imx7d-evk-wm8960
>                - karo,tx53-audio-sgtl5000
> @@ -65,6 +67,7 @@ properties:
>                - fsl,imx-audio-sgtl5000
>                - fsl,imx-audio-wm8960
>                - fsl,imx-audio-wm8962
> +              - fsl,imx-audio-spdif

This does not look right. It's quite generic, so now you allow any
variant to be used with this fallback.

Please do not grow more this list of all possible combinations and
instead add specific lists. Otherwise, please explain why this is valid
hardware:
"fsl,imx7d-evk-wm8960", "fsl,imx-audio-spdif"


>        - items:
>            - enum:
>                - fsl,imx-audio-ac97
> @@ -81,6 +84,7 @@ properties:
>                - fsl,imx-audio-wm8960
>                - fsl,imx-audio-wm8962
>                - fsl,imx-audio-wm8958
> +              - fsl,imx-audio-spdif

Fallbacks should not be used alone. Why this is needed? The compatible
is already documented, so now you create duplicated binding.

This is very confusing.

>  
>    model:
>      $ref: /schemas/types.yaml#/definitions/string
> @@ -93,8 +97,15 @@ 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.
> +      With "fsl,imx-audio-spdif", either SPDIF audio codec spdif_transmitter,
> +      spdif_receiver or both.
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      maxItems: 1
>  
>    audio-cpu:
>      $ref: /schemas/types.yaml#/definitions/phandle
> @@ -150,8 +161,10 @@ 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
> @@ -195,3 +208,12 @@ examples:
>               "AIN2L", "Line In Jack",
>               "AIN2R", "Line In Jack";
>      };
> +
> +  - |
> +    sound-spdif-asrc {
> +      compatible = "fsl,imx-audio-spdif";
> +      model = "spdif-asrc-audio";
> +      audio-cpu = <&spdif>;
> +      audio-asrc = <&easrc>;
> +      audio-codec = <&spdifdit>, <&spdifdir>;
> +    };

Do not introduce another indentation style. Look what is above.


Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ