Re: [PATCH] dt-bindings: sound: nvidia,tegra30-ahub: Convert to DT schema

[Krzysztof Kozlowski]

On Sat, Jun 13, 2026 at 08:24:47AM +0000, Charan Pedumuru wrote:
> Convert NVIDIA Tegra Audio Hub (AHUB) binding to DT schema.

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

ASoC: dt-bindings:

...

> +description:
> +  The NVIDIA Tegra AHUB (Audio Hub) is an audio interconnect block used to
> +  route data between various audio clients such as I2S, DAM, SPDIF, and
> +  APBIF. It exposes multiple register regions and supports different
> +  configurations depending on the Tegra SoC generation. The AHUB also
> +  provides a configlink bus for child audio components, which use CIF
> +  (Client Interface) IDs to identify their data paths. The number of DMA
> +  channels, reset lines, and additional modules varies across Tegra30,
> +  Tegra114, and Tegra124 platforms.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: nvidia,tegra30-ahub
> +      - const: nvidia,tegra114-ahub
> +      - const: nvidia,tegra124-ahub

These three are enum and please sort alphanumerically, not by natural
sorting (so 30 > 124)

> +      - items:
> +          - const: nvidia,tegra132-ahub
> +          - const: nvidia,tegra124-ahub
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 3
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Clock for the D_AUDIO domain
> +      - description: Clock for the APBIF interface
> +
> +  clock-names:
> +    items:
> +      - const: d_audio
> +      - const: apbif
> +
> +  resets:
> +    minItems: 11
> +    maxItems: 21
> +
> +  reset-names:
> +    oneOf:
> +      # Tegra30: d_audio apbif i2s0-4 dam0-2 spdif
> +      - items:
> +          - const: d_audio
> +          - const: apbif
> +          - const: i2s0
> +          - const: i2s1
> +          - const: i2s2
> +          - const: i2s3
> +          - const: i2s4
> +          - const: dam0
> +          - const: dam1
> +          - const: dam2
> +          - const: spdif
> +
> +      # Tegra114: adds amx adx
> +      - items:
> +          - const: d_audio
> +          - const: apbif
> +          - const: i2s0
> +          - const: i2s1
> +          - const: i2s2
> +          - const: i2s3
> +          - const: i2s4
> +          - const: dam0
> +          - const: dam1
> +          - const: dam2
> +          - const: spdif
> +          - const: amx
> +          - const: adx
> +
> +      # Tegra124: adds amx1 adx1 afc0-5
> +      - items:
> +          - const: d_audio
> +          - const: apbif
> +          - const: i2s0
> +          - const: i2s1
> +          - const: i2s2
> +          - const: i2s3
> +          - const: i2s4
> +          - const: dam0
> +          - const: dam1
> +          - const: dam2
> +          - const: spdif
> +          - const: amx
> +          - const: amx1
> +          - const: adx
> +          - const: adx1
> +          - const: afc0
> +          - const: afc1
> +          - const: afc2
> +          - const: afc3
> +          - const: afc4
> +          - const: afc5

These should be placed in allOf: section. Here define only min/maxItems.

> +
> +  dmas:
> +    minItems: 8
> +    maxItems: 20
> +
> +  dma-names:
> +    oneOf:
> +      # Tegra30: rx0..rx3 tx0..tx3 (n=3)
> +      - items:
> +          - const: rx0
> +          - const: tx0
> +          - const: rx1
> +          - const: tx1
> +          - const: rx2
> +          - const: tx2
> +          - const: rx3
> +          - const: tx3
> +
> +      # Tegra114/124: rx0..rx9 tx0..tx9 (n=9)
> +      - items:
> +          - const: rx0
> +          - const: tx0
> +          - const: rx1
> +          - const: tx1
> +          - const: rx2
> +          - const: tx2
> +          - const: rx3
> +          - const: tx3
> +          - const: rx4
> +          - const: tx4
> +          - const: rx5
> +          - const: tx5
> +          - const: rx6
> +          - const: tx6
> +          - const: rx7
> +          - const: tx7
> +          - const: rx8
> +          - const: tx8
> +          - const: rx9
> +          - const: tx9

Keep this list (so drop the "oneOf") and add minItems. Then in
allOf:if:then: block you customize min/maxItems.

> +
> +  "#address-cells":
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  "#size-cells":
> +    $ref: /schemas/types.yaml#/definitions/uint32

You do not need types. You need values (const). See also other bindings
for examples. Even if this is MMIO bus, I doubt children need 64-bit
addressing, so this should be really 32-bit (thus const:1 , not even
enum). If you need 64-bit addressing (e.g. for DMA), this would have to
be explained.

> +
> +  ranges: true
> +
> +patternProperties:
> +  "^i2s@[0-9a-f]+$":

I think this was not in the old binding. so you need to explain in
commit msg why you made this change to the binding.

> +    type: object
> +
> +    allOf:
> +      - $ref: /schemas/sound/nvidia,tegra30-i2s.yaml#

$ref directly, no need for allOf

> +
> +    properties:
> +      nvidia,ahub-cif-ids:
> +        description: Pair of AHUB CIF IDs for the RX and TX data paths.
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 2
> +        maxItems: 2
> +
> +    required:
> +      - nvidia,ahub-cif-ids

Why do you need the properties and required here? The child (i2s) schema
should define that.

> +
> +    unevaluatedProperties: false
> +

Best regards,
Krzysztof