Re: [PATCH v1 RFC] ASoC: dt-bindings: Define '#sound-dai-cells' only in the dai-common schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] ASoC: dt-bindings: dai-common.yaml enum [0, 1] breaks 2-cell DAI schemas
- [Medium] ASoC: dt-bindings: dropping const #sound-dai-cells weakens DT validation
--

commit 26a69ea0b2bf774fcda9427ca7712dbec4492c12
Author: Svyatoslav Ryhel <clamor95@gmail.com>

ASoC: dt-bindings: Define '#sound-dai-cells' only in the dai-common schema

This commit constrains #sound-dai-cells in dai-common.yaml to an enum of
[0, 1] and drops the explicit #sound-dai-cells property definitions from
linux,bt-sco.yaml and linux,spdif.yaml in favor of referencing
dai-common.yaml.

> diff --git a/Documentation/devicetree/bindings/sound/dai-common.yaml b/Documentation/devicetree/bindings/sound/dai-common.yaml
> index 1aed2f0f17756..152f319d8f382 100644
> --- a/Documentation/devicetree/bindings/sound/dai-common.yaml
> +++ b/Documentation/devicetree/bindings/sound/dai-common.yaml
> @@ -13,6 +13,10 @@ allOf:
>    - $ref: component-common.yaml#
>  
>  properties:
> -  '#sound-dai-cells': true
> +  '#sound-dai-cells':
> +    description:
> +      Use 0 if only a single DAI cell is available, or 1 if there are multiple
> +      DAI cells. For special cases, check the device schema.
> +    enum: [0, 1]

[Severity: Medium]
Can device schemas that require more than one cell actually override
enum: [0, 1] when referencing dai-common.yaml?

In devicetree schemas, device bindings include dai-common.yaml via allOf,
which requires every subschema to be satisfied simultaneously. A referencing
schema can narrow the allowed values in dai-common.yaml, but it cannot widen
enum: [0, 1] to allow 2.

For example, Documentation/devicetree/bindings/sound/amlogic,aiu.yaml
includes dai-common.yaml and requires 2 cells:

    allOf:
      - $ref: dai-common.yaml#

    properties:
      $nodename:
        pattern: "^audio-controller@.*"

      "#sound-dai-cells":
        const: 2

This matches sound/soc/meson/aiu.c:aiu_of_xlate_dai_name():

    if (args->args_count != 2)
            return -EINVAL;

With enum: [0, 1] in dai-common.yaml, the intersection of enum: [0, 1] and
const: 2 is empty, causing dt_binding_check on amlogic,aiu.yaml (and
dtbs_check on arch/arm/boot/dts/amlogic/meson.dtsi and
arch/arm64/boot/dts/amlogic/meson-gx.dtsi) to fail with a validation
regression because 2 is not one of [0, 1].

Commit 3fda85324b8d ("ASoC: dt-bindings: Extend name-prefix.yaml into common
DAI properties") originally left '#sound-dai-cells': true in dai-common.yaml
for this reason. Should dai-common.yaml keep '#sound-dai-cells': true (or
include 2 in the allowed range) so bindings with 2 cells remain valid?

[ ... ]

> diff --git a/Documentation/devicetree/bindings/sound/linux,spdif.yaml b/Documentation/devicetree/bindings/sound/linux,spdif.yaml
> index aea6230db54c8..62c035b739d38 100644
> --- a/Documentation/devicetree/bindings/sound/linux,spdif.yaml
> +++ b/Documentation/devicetree/bindings/sound/linux,spdif.yaml
> @@ -18,9 +18,6 @@ properties:
>        - linux,spdif-dit
>        - linux,spdif-dir
>  
> -  "#sound-dai-cells":
> -    const: 0
> -

[Severity: Medium]
Does dropping const: 0 here weaken schema validation for fixed single-dai
and multi-dai devices?

Both linux,spdif-dit and linux,spdif-dir only ever register a single stub
dai in sound/soc/codecs/spdif_transmitter.c:spdif_dit_probe():

    static int spdif_dit_probe(struct platform_device *pdev)
    {
            return devm_snd_soc_register_component(&pdev->dev,
                            &soc_codec_spdif_dit,
                            &dit_stub_dai, 1);
    }

and sound/soc/codecs/spdif_receiver.c:spdif_dir_probe():

    static int spdif_dir_probe(struct platform_device *pdev)
    {
            return devm_snd_soc_register_component(&pdev->dev,
                            &soc_codec_spdif_dir,
                            &dir_stub_dai, 1);
    }

Per the description in dai-common.yaml, #sound-dai-cells must be 0 when only
a single dai is available. Replacing const: 0 in linux,spdif.yaml with the
common enum: [0, 1] allows a devicetree node with #sound-dai-cells = <1> to
pass dtbs_check silently.

Similarly, if bindings for multi-dai codecs drop const: 1 in favor of
dai-common.yaml, nodes erroneously setting #sound-dai-cells = <0> will pass
schema validation and then fail at runtime during phandle translation in
functions such as snd_soc_of_xlate_dai_name(), qmc_audio_of_xlate_dai_name(),
or q6dsp_audio_ports_of_xlate_dai_name().

Should bindings for devices with a fixed number of dais keep their explicit
const: 0 or const: 1 constraint in the device schema?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024084307.116715-1-clamor95@gmail.com?part=1