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

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

[Svyatoslav Ryhel]

Define the '#sound-dai-cells' property only in the dai-common schema with
an enum of commonly used values and a description on how to use them. Drop
all trivial uses of '#sound-dai-cells' across devices with DAI support,
replacing them with a reference to the dai-common schema.

This should resolve any issues with codecs having multiple DAI cells, no
DAI cells at all, or incorrectly configured sound-dai-cells properties.
Simply include the common schema and switch additionalProperties to
unevaluatedProperties.

Non-trivial '#sound-dai-cells' usages can still be overridden in dedicated
device schemas.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
NOTE: This is currently just a concept. The actual patch will be very large,
as it touches nearly every codec binding. Therefore, I am sending this RFC
to ask if this change would be acceptable in principle, and if so, how it
should best be structured and split. Thanks.
---
 Documentation/devicetree/bindings/sound/dai-common.yaml   | 6 +++++-
 Documentation/devicetree/bindings/sound/linux,bt-sco.yaml | 7 -------
 Documentation/devicetree/bindings/sound/linux,spdif.yaml  | 5 +----
 3 files changed, 6 insertions(+), 12 deletions(-)

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]
 
 additionalProperties: true
diff --git a/Documentation/devicetree/bindings/sound/linux,bt-sco.yaml b/Documentation/devicetree/bindings/sound/linux,bt-sco.yaml
index a67b79cbe006b..f30aea4b545c9 100644
--- a/Documentation/devicetree/bindings/sound/linux,bt-sco.yaml
+++ b/Documentation/devicetree/bindings/sound/linux,bt-sco.yaml
@@ -13,13 +13,6 @@ allOf:
   - $ref: dai-common.yaml#
 
 properties:
-  '#sound-dai-cells':
-    enum:
-      - 0
-
-      # For Wideband PCM
-      - 1
-
   compatible:
     enum:
       - delta,dfbmcs320
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
-
   sound-name-prefix: true
 
   port:
@@ -30,7 +27,7 @@ required:
   - "#sound-dai-cells"
   - compatible
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.53.0

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

[sashiko-bot]

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