Re: [PATCH 4/6] dt-bindings: allwinner: add H616 sun4i audio codec binding

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On Sun, Sep 29, 2024 at 11:06:05PM +1300, Ryan Walklin wrote:
> The H616 has an audio codec compatible with the sun4i-a10 driver.
> 
> The codec is relatively cut down compared to some of the other Allwinner
> SoCs and only has a single line-out route (relying on a separate digital
> microphone IP block for input). HDMI and SPDIF audio are handled
> separately by an audio hub IP block, which is not currently implemented
> in mainline kernels. This and the use of SDM requires some additional
> flexibility to the DMA and clock bindings.
> 
> Add compatible string and routing for the H616 audio codec, and update
> the required clock and DMA descriptions.
> 
> Signed-off-by: Ryan Walklin <ryan@xxxxxxxxxxxxx>
> ---
>  .../sound/allwinner,sun4i-a10-codec.yaml      | 55 +++++++++++++++----
>  1 file changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> index 78273647f7665..5838600dbc730 100644
> --- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> @@ -22,6 +22,7 @@ properties:
>        - allwinner,sun8i-a23-codec
>        - allwinner,sun8i-h3-codec
>        - allwinner,sun8i-v3s-codec
> +      - allwinner,sun50i-h616-codec
>  
>    reg:
>      maxItems: 1
> @@ -30,24 +31,40 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    items:
> -      - description: Bus Clock
> -      - description: Module Clock
> +    oneOf:
> +      - items:
> +          - description: Bus Clock
> +          - description: Module Clock
> +      - items:
> +          - description: Bus Clock
> +          - description: Module Clock
> +          - description: Module Clock (4X)

No, grow the list and add minItems instead.

>  
>    clock-names:
> -    items:
> -      - const: apb
> -      - const: codec
> +    oneOf:
> +      - items:
> +          - const: apb
> +          - const: codec
> +      - items:
> +          - const: apb
> +          - const: codec
> +          - const: audio-codec-4x

Same comment.

>  
>    dmas:
> -    items:
> -      - description: RX DMA Channel
> -      - description: TX DMA Channel
> +    oneOf:
> +      - items:
> +          - description: RX DMA Channel
> +          - description: TX DMA Channel
> +      - items:
> +          - description: TX DMA Channel
>  
>    dma-names:
> -    items:
> -      - const: rx
> -      - const: tx
> +    oneOf:
> +      - items:
> +          - const: rx
> +          - const: tx
> +      - items:
> +          - const: tx

These two properties are fine.

>  
>    resets:
>      maxItems: 1
> @@ -229,6 +246,20 @@ allOf:
>                - Mic
>                - Speaker
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun50i-h616-codec
> +
> +    then:
> +      properties:
> +        allwinner,audio-routing:
> +          items:
> +            enum:
> +              - LINEOUT
> +              - Line Out

That's odd, why two same names?

You must restrict the properties you just changed per each variant.

Best regards,
Krzysztof