Re: [PATCH v6 01/16] ASoC: dt-bindings: sound: Add DT binding for RZ/G3E sound

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The binding omits provider cell properties (`#clock-cells` and `#sound-dai-cells`) from the `required` list despite exposing these capabilities.
- [High] The schema allows and the example uses repeated strings (e.g., multiple `"tx"`) in the `dma-names` arrays, which breaks standard DMA API lookup.
- [Medium] The `ssi` subnodes define `dmas: true` and `dma-names: true` which bypasses all validation and contradicts the hardware's operational mode.
- [Low] The device tree example contains multiple unresolved phandle references that will cause `dt_binding_check` to fail.
--

commit 2000fc3ecea1872cd8c0d174d7a5e3c5c89282e9
Author: John Madieu <john.madieu.xa@bp.renesas.com>

ASoC: dt-bindings: sound: Add DT binding for RZ/G3E sound

Add a standalone device tree binding for the Renesas RZ/G3E (R9A09G047)
sound controller. The RZ/G3E sound IP is based on R-Car Sound but adds
extended DMA channel support, additional clock domains, and resets.

> diff --git a/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml b/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml
> new file mode 100644
> index 0000000000000..0b651214bd613
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml
[ ... ]
> +  dvc:
> +    type: object
> +    additionalProperties: false
> +    patternProperties:
> +      "^dvc-[0-1]$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          dmas:
> +            maxItems: 5
> +          dma-names:
> +            maxItems: 5
> +            allOf:
> +              - items:
> +                  enum:
> +                    - tx

The commit message explicitly notes allowing multiple DMA entries with
repeated channel names in SSIU, SRC and DVC sub-nodes. However, will this
regression break the standard DMA API lookup?

The of_dma_request_slave_channel() function relies on
of_property_match_string() to resolve DMA channels by name, which strictly
returns the index of the first match.

If multiple entries use the same name, any DMA channels listed after the first
tx or rx will be inaccessible. To support fallback or alternative
channels, shouldn't they have unique names like tx0 and tx1?

[ ... ]
> +  ssi:
> +    type: object
> +    additionalProperties: false
> +    patternProperties:
> +      "^ssi-[0-9]$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          interrupts:
> +            maxItems: 1
> +          dmas: true
> +          dma-names: true

Does defining dmas and dma-names as true bypass schema validation for
these properties?

This seems inconsistent with the strict constraints applied to src and
dvc.

Additionally, the commit message states that SSI operates exclusively in BUSIF
mode, which usually implies DMA is handled by the SSIU, not the SSI directly
(as reflected by the omission of DMA properties in the ssi example nodes).
Should these properties be constrained or removed entirely from the ssi
subnodes?

[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names

Should #clock-cells and #sound-dai-cells be included in this list?

The schema defines #clock-cells: const: 0 and #sound-dai-cells: const: 1
in the properties block, indicating the hardware acts as a clock and DAI
provider.

According to device tree binding guidelines, when a hardware binding supports
provider capabilities, the corresponding cell-count properties must be
mandated in the required list to prevent incomplete device tree nodes from
passing schema validation.

[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    sound@13c00000 {
> +      #sound-dai-cells = <1>;
> +      #clock-cells = <0>;
> +      compatible = "renesas,r9a09g047-sound";
> +      reg = <0x13c00000 0x10000>,
> +            <0x13c20000 0x10000>,
> +            <0x13c30000 0x1000>,
> +            <0x13c31000 0x1f000>,
> +            <0x13c50000 0x10000>;
> +      reg-names = "scu", "adg", "ssiu", "ssi", "audmapp";
> +      clocks = <&cpg 245>,

Are there definitions for external labels like <&cpg 245>, <&dmac0>, and
<&codec_endpoint> in the example snippet?

When dt_binding_check compiles the example using dtc, unresolved phandle
references will cause syntax errors for referencing non-existent nodes or
labels.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512182631.3842065-1-john.madieu.xa@bp.renesas.com?part=1