Re: [PATCH v2 05/21] ASoC: dt-bindings: Add binding for Apple Silicon Mac audio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] dt-bindings: conflicting additionalProperties alongside $ref
- [Low] dt-bindings: dynamic index-suffixed property names for TDM configs
- [Low] dt-bindings: missing #address-cells and #size-cells in required list
--

commit e0246be35f768471da65ec0d3bc4f036e14c8598
Author: Martin Povišer <povik+lin@cutebit.org>
ASoC: dt-bindings: Add binding for Apple Silicon Mac audio

This commit adds a binding for the primary audio subsystem found on Apple
Silicon Macs. It integrates the codecs and SoC I2S peripheral used for
driving the headset jack and speaker arrays.

> diff --git a/Documentation/devicetree/bindings/sound/apple,macaudio.yaml b/Documentation/devicetree/bindings/sound/apple,macaudio.yaml
> new file mode 100644
> index 0000000000000..c0a04fab82d31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/apple,macaudio.yaml
[ ... ]
> +patternProperties:
> +  "^dai-link(@[0-9a-f]+)?$":
> +    description: |
> +      Node for each sound peripheral such as the speaker array, headphones jack,
> +      or microphone.
> +    type: object
> +    $ref: tdm-slot.yaml#
> +
> +    additionalProperties: false

[Severity: Low]
Does the use of additionalProperties: false alongside a top-level $ref import
block standard properties defined in the referenced schema from being validated?

In JSON Schema, additionalProperties only evaluates properties explicitly
defined in the local schema. If standard properties from tdm-slot.yaml were
intended to be allowed, they will be erroneously rejected, requiring
unevaluatedProperties: false instead. 

If they were meant to be rejected, is the top-level $ref dead code that
should be removed?

[ ... ]
> +    patternProperties:
> +      "^dai-tdm-idle-mode-[0-9]$":
> +        $ref: tdm-slot.yaml#/properties/dai-tdm-idle-mode
> +
> +      "^dai-tdm-slot-tx-idle-mask-[0-9]$":
> +        $ref: /schemas/types.yaml#/definitions/uint32

[Severity: Low]
Does appending dynamic integer indices to property names fragment standard
Devicetree design and circumvent generic TDM parsing utilities?

Property names are typically static. Could configuration values intended
for multiple codecs on a single DAI link either be defined as an array
property or configured directly inside discrete child nodes representing
each codec context?

[ ... ]
> +    required:
> +      - reg
> +      - codec
> +      - cpu
> +
> +required:
> +  - compatible
> +  - model
> +
> +additionalProperties: false

[Severity: Low]
Because the dai-link child nodes unconditionally require a reg property, do
#address-cells and #size-cells need to be mandated in the required list?

Omitting them from the parent's required list allows an incomplete Device
Tree node to silently pass dt_binding_check schema validation while
remaining invalid per the core DT specification.

[ ... ]
> +examples:
> +  - |
> +    sound {
> +      compatible = "apple,j416-macaudio", "apple,j316-macaudio";
> +      model = "MacBook Pro J416";
> +
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      dai-link@0 {
> +        reg = <0>;
> +        link-name = "Speakers";
> +
> +        dai-tdm-idle-mode-0 = "zero";
> +        dai-tdm-idle-mode-3 = "zero";
> +        dai-tdm-slot-tx-idle-mask-0 = <0xf0f0f0>;
> +        dai-tdm-slot-tx-idle-mask-3 = <0x0f0f0f>;

[Severity: Low]
This isn't a functional bug, but the examples showcase the usage of
hardcoded suffix indices on property keys mentioned above. Should these be
refactored to use static arrays or discrete child nodes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-macaudio-v2-0-7b47ed6335a8@gmail.com?part=5