diff mbox series

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

Message ID 20240929100750.860329-5-ryan@testtoast.com (mailing list archive)
State Not Applicable, archived
Headers show
Series ASoC: add Allwinner H616 audio codec support | expand

Commit Message

Ryan Walklin Sept. 29, 2024, 10:06 a.m. UTC
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@testtoast.com>
---
 .../sound/allwinner,sun4i-a10-codec.yaml      | 55 +++++++++++++++----
 1 file changed, 43 insertions(+), 12 deletions(-)

Comments

Krzysztof Kozlowski Sept. 29, 2024, 7:56 p.m. UTC | #1
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@testtoast.com>
> ---
>  .../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
Ryan Walklin Oct. 20, 2024, 6:58 a.m. UTC | #2
On Mon, 30 Sep 2024, at 8:56 AM, Krzysztof Kozlowski wrote:
> On Sun, Sep 29, 2024 at 11:06:05PM +1300, Ryan Walklin wrote:

>>  
>>    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.

Thanks, turns out the 4x clock is not actually required by the driver so will remove this and the clock-names change for v2.

>> +    then:
>> +      properties:
>> +        allwinner,audio-routing:
>> +          items:
>> +            enum:
>> +              - LINEOUT
>> +              - Line Out
>
> That's odd, why two same names?

These are the input and output sides respectively, the LINEOUT is the SoC pinout, the Line Out is the board connector as per the routing description above. Just looks odd because the H616 codec has only this single route.

>
> You must restrict the properties you just changed per each variant.
>
Thanks, will do .

> Best regards,
> Krzysztof

Regards,

Ryan
Andre Przywara Oct. 20, 2024, 10:27 a.m. UTC | #3
On Sun, 20 Oct 2024 19:58:21 +1300
"Ryan Walklin" <ryan@testtoast.com> wrote:

Hi,

> On Mon, 30 Sep 2024, at 8:56 AM, Krzysztof Kozlowski wrote:
> > On Sun, Sep 29, 2024 at 11:06:05PM +1300, Ryan Walklin wrote:  
> 
> >>  
> >>    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.  
> 
> Thanks, turns out the 4x clock is not actually required by the driver so will remove this and the clock-names change for v2.

Please note that "...not required by *the* driver ..." is not a valid
rationale in the context of a DT binding: it's supposed to describe the
hardware, not a particular implementation.
But I see that the *hardware* manual states that the codec only uses
PLL_AUDIO(1X) as its input clock, so it indeed seems to be not needed.

Cheers,
Andre

> 
> >> +    then:
> >> +      properties:
> >> +        allwinner,audio-routing:
> >> +          items:
> >> +            enum:
> >> +              - LINEOUT
> >> +              - Line Out  
> >
> > That's odd, why two same names?  
> 
> These are the input and output sides respectively, the LINEOUT is the SoC pinout, the Line Out is the board connector as per the routing description above. Just looks odd because the H616 codec has only this single route.
> 
> >
> > You must restrict the properties you just changed per each variant.
> >  
> Thanks, will do .
> 
> > Best regards,
> > Krzysztof  
> 
> Regards,
> 
> Ryan
>
diff mbox series

Patch

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)
 
   clock-names:
-    items:
-      - const: apb
-      - const: codec
+    oneOf:
+      - items:
+          - const: apb
+          - const: codec
+      - items:
+          - const: apb
+          - const: codec
+          - const: audio-codec-4x
 
   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
 
   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
+
 unevaluatedProperties: false
 
 examples: