diff mbox series

[3/7] ASoC: dt-bindings: add schema for rockchip SAI controllers

Message ID 20250305-rk3576-sai-v1-3-64e6cf863e9a@collabora.com (mailing list archive)
State New
Headers show
Series Add RK3576 SAI Audio Controller Support | expand

Commit Message

Nicolas Frattaroli March 5, 2025, 9:24 p.m. UTC
Rockchip introduced a new audio controller called the "Serial Audio
Interface", or "SAI" for short, on some of their newer SoCs. In
particular, this controller is used several times on the RK3576 SoC.

Add a schema for it, with only an RK3576 compatible for now. Other SoCs
may follow as mainline support for them lands.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/sound/rockchip,sai.yaml    | 151 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 157 insertions(+)

Comments

Krzysztof Kozlowski March 6, 2025, 7:42 a.m. UTC | #1
On 05/03/2025 22:24, Nicolas Frattaroli wrote:
> Rockchip introduced a new audio controller called the "Serial Audio
> Interface", or "SAI" for short, on some of their newer SoCs. In
> particular, this controller is used several times on the RK3576 SoC.
> 
> Add a schema for it, with only an RK3576 compatible for now. Other SoCs
> may follow as mainline support for them lands.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../devicetree/bindings/sound/rockchip,sai.yaml    | 151 +++++++++++++++++++++

Filename based on compatible.

>  MAINTAINERS                                        |   6 +
>  2 files changed, 157 insertions(+)
> 

...

> +
> +  dma-names:
> +    minItems: 1
> +    maxItems: 2
> +    oneOf:
> +      - const: tx
> +      - const: rx
> +      - items:
> +          - const: tx
> +          - const: rx

Why all combinations are possible?

> +
> +  clocks:
> +    items:
> +      - description: master audio clock
> +      - description: AHB clock driving the interface
> +
> +  clock-names:
> +    items:
> +      - const: mclk
> +      - const: hclk
> +
> +  resets:
> +    minItems: 1
> +    maxItems: 2
> +    description: resets for the mclk domain and ahb domain

List the items instead with description and minItems: 1.

> +
> +  reset-names:
> +    minItems: 1
> +    items:
> +      - const: m
> +      - const: h
> +
> +  port:
> +    $ref: audio-graph-port.yaml#
> +    unevaluatedProperties: false
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  "#sound-dai-cells":
> +    const: 0
> +
> +  rockchip,sai-rx-route:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Defines the mapping of the controller's SDI ports to actual input lanes,
> +      as well as the number of input lanes.
> +      rockchip,sai-rx-route = <3> would mean sdi3 is receiving from data0, and
> +      that there is only one receiving lane.
> +      This property's absence is to be understood as only one receiving lane
> +      being used if the controller has capture capabilities.
> +    maxItems: 4
> +    items:
> +      enum: [0, 1, 2, 3]
> +
> +  rockchip,sai-tx-route:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Defines the mapping of the controller's SDO ports to actual output lanes,
> +      as well as the number of output lanes.
> +      rockchip,sai-tx-route = <3> would mean sdo3 is sending to data0, and

I understand this is only example because = <3> would not be allowed
(test it).

> +      that there is only one transmitting lane.
> +      This property's absence is to be understood as only one transmitting lane
> +      being used if the controller has playback capabilities.
> +    maxItems: 4
> +    items:
> +      enum: [0, 1, 2, 3]
> +
> +  rockchip,always-on:
> +    type: boolean
> +    description:
> +      The hardware requires this controller to remain turned on.

How hardware requires this? You rather miss proper PM domain handling or
some other resources.

> +
> +

Just one blank line.

> +required:
> +  - compatible
> +  - reg
> +  - dmas
> +  - dma-names
> +  - clocks
> +  - clock-names
> +  - "#sound-dai-cells"
> +
> +unevaluatedProperties: false



Best regards,
Krzysztof
Nicolas Frattaroli March 6, 2025, 1:13 p.m. UTC | #2
On Thursday, 6 March 2025 08:42:54 Central European Standard Time Krzysztof 
Kozlowski wrote:
> On 05/03/2025 22:24, Nicolas Frattaroli wrote:
> > Rockchip introduced a new audio controller called the "Serial Audio
> > Interface", or "SAI" for short, on some of their newer SoCs. In
> > particular, this controller is used several times on the RK3576 SoC.
> > 
> > Add a schema for it, with only an RK3576 compatible for now. Other SoCs
> > may follow as mainline support for them lands.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> > 
> >  .../devicetree/bindings/sound/rockchip,sai.yaml    | 151
> >  +++++++++++++++++++++
> Filename based on compatible.

Sure, but more compatibles will follow. Are you certain you want a file named 
rockchip,rk3576-sai.yaml to then contain rockchip,rk3528-sai? If so then I do 
not understand the reason behind this policy.

> 
> >  MAINTAINERS                                        |   6 +
> >  2 files changed, 157 insertions(+)
> 
> ...
> 
> > +
> > +  dma-names:
> > +    minItems: 1
> > +    maxItems: 2
> > +    oneOf:
> > +      - const: tx
> > +      - const: rx
> > +      - items:
> > +          - const: tx
> > +          - const: rx
> 
> Why all combinations are possible?

Because they are. sai5 in rk3576 is rx only. sai7 is tx only. Others are both 
tx and rx. Do you want me to enforce that those with both are always tx 
followed by rx?

> 
> > +
> > +  clocks:
> > +    items:
> > +      - description: master audio clock
> > +      - description: AHB clock driving the interface
> > +
> > +  clock-names:
> > +    items:
> > +      - const: mclk
> > +      - const: hclk
> > +
> > +  resets:
> > +    minItems: 1
> > +    maxItems: 2
> > +    description: resets for the mclk domain and ahb domain
> 
> List the items instead with description and minItems: 1.

Will do

> 
> > +
> > +  reset-names:
> > +    minItems: 1
> > +    items:
> > +      - const: m
> > +      - const: h
> > +
> > +  port:
> > +    $ref: audio-graph-port.yaml#
> > +    unevaluatedProperties: false
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  "#sound-dai-cells":
> > +    const: 0
> > +
> > +  rockchip,sai-rx-route:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description:
> > +      Defines the mapping of the controller's SDI ports to actual input
> > lanes, +      as well as the number of input lanes.
> > +      rockchip,sai-rx-route = <3> would mean sdi3 is receiving from
> > data0, and +      that there is only one receiving lane.
> > +      This property's absence is to be understood as only one receiving
> > lane +      being used if the controller has capture capabilities.
> > +    maxItems: 4
> > +    items:
> > +      enum: [0, 1, 2, 3]
> > +
> > +  rockchip,sai-tx-route:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description:
> > +      Defines the mapping of the controller's SDO ports to actual output
> > lanes, +      as well as the number of output lanes.
> > +      rockchip,sai-tx-route = <3> would mean sdo3 is sending to data0,
> > and
> 
> I understand this is only example because = <3> would not be allowed
> (test it).

I'll have to look into that, I was fairly certain I tested it, but maybe I 
forgot to run a CHECK_DTBS with it in.

> 
> > +      that there is only one transmitting lane.
> > +      This property's absence is to be understood as only one
> > transmitting lane +      being used if the controller has playback
> > capabilities.
> > +    maxItems: 4
> > +    items:
> > +      enum: [0, 1, 2, 3]
> > +
> > +  rockchip,always-on:
> > +    type: boolean
> > +    description:
> > +      The hardware requires this controller to remain turned on.
> 
> How hardware requires this? You rather miss proper PM domain handling or
> some other resources.

This isn't about power domains. It's about the FS/SCLK generator inside the 
IP. I'll remove it in the next revision since downstream only uses it for a 
different IP on the RK3588, and I'd rather not get bogged down by discussions 
as to whether SAI should be modelled as a clock provider.

> 
> > +
> > +
> 
> Just one blank line.

Will do

> 
> > +required:
> > +  - compatible
> > +  - reg
> > +  - dmas
> > +  - dma-names
> > +  - clocks
> > +  - clock-names
> > +  - "#sound-dai-cells"
> > +
> > +unevaluatedProperties: false
> 
> Best regards,
> Krzysztof

Cheers,
Nicolas Frattaroli
Krzysztof Kozlowski March 6, 2025, 1:43 p.m. UTC | #3
On 06/03/2025 14:13, Nicolas Frattaroli wrote:
> On Thursday, 6 March 2025 08:42:54 Central European Standard Time Krzysztof 
> Kozlowski wrote:
>> On 05/03/2025 22:24, Nicolas Frattaroli wrote:
>>> Rockchip introduced a new audio controller called the "Serial Audio
>>> Interface", or "SAI" for short, on some of their newer SoCs. In
>>> particular, this controller is used several times on the RK3576 SoC.
>>>
>>> Add a schema for it, with only an RK3576 compatible for now. Other SoCs
>>> may follow as mainline support for them lands.
>>>
>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>>> ---
>>>
>>>  .../devicetree/bindings/sound/rockchip,sai.yaml    | 151
>>>  +++++++++++++++++++++
>> Filename based on compatible.
> 
> Sure, but more compatibles will follow. Are you certain you want a file named 
> rockchip,rk3576-sai.yaml to then contain rockchip,rk3528-sai? If so then I do 

Yes (or the other way around if 3528 is older)

> not understand the reason behind this policy.

So the name will match hardware, instead of sai.yaml, sai2.yaml,
sai3.yaml and sai-green-frog.yaml.

> 
>>
>>>  MAINTAINERS                                        |   6 +
>>>  2 files changed, 157 insertions(+)
>>
>> ...
>>
>>> +
>>> +  dma-names:
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +    oneOf:
>>> +      - const: tx
>>> +      - const: rx
>>> +      - items:
>>> +          - const: tx
>>> +          - const: rx
>>
>> Why all combinations are possible?
> 
> Because they are. sai5 in rk3576 is rx only. sai7 is tx only. Others are both 

What is sai5 and sai7? Instances on the same chip? Does it mean some of
instances can only receive and some only transmit?

> tx and rx. Do you want me to enforce that those with both are always tx 
> followed by rx?
> 
Best regards,
Krzysztof
Nicolas Frattaroli March 6, 2025, 2:38 p.m. UTC | #4
On Thursday, 6 March 2025 14:43:21 Central European Standard Time Krzysztof 
Kozlowski wrote:
> On 06/03/2025 14:13, Nicolas Frattaroli wrote:
> > On Thursday, 6 March 2025 08:42:54 Central European Standard Time
> > Krzysztof
> > 
> > Kozlowski wrote:
> >> On 05/03/2025 22:24, Nicolas Frattaroli wrote:
> >>> Rockchip introduced a new audio controller called the "Serial Audio
> >>> Interface", or "SAI" for short, on some of their newer SoCs. In
> >>> particular, this controller is used several times on the RK3576 SoC.
> >>> 
> >>> Add a schema for it, with only an RK3576 compatible for now. Other SoCs
> >>> may follow as mainline support for them lands.
> >>> 
> >>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> >>> ---
> >>> 
> >>>  .../devicetree/bindings/sound/rockchip,sai.yaml    | 151
> >>>  +++++++++++++++++++++
> >> 
> >> Filename based on compatible.
> > 
> > Sure, but more compatibles will follow. Are you certain you want a file
> > named rockchip,rk3576-sai.yaml to then contain rockchip,rk3528-sai? If so
> > then I do
> Yes (or the other way around if 3528 is older)

They were released at about the same time from what I know. I'll pick the 
RK3576 though since it was the first to get upstream support.

> 
> > not understand the reason behind this policy.
> 
> So the name will match hardware, instead of sai.yaml, sai2.yaml,
> sai3.yaml and sai-green-frog.yaml.

Downstream called the compatible "sai-v1" so you're onto something here I'm 
afraid. :)

> 
> >>>  MAINTAINERS                                        |   6 +
> >>>  2 files changed, 157 insertions(+)
> >> 
> >> ...
> >> 
> >>> +
> >>> +  dma-names:
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>> +    oneOf:
> >>> +      - const: tx
> >>> +      - const: rx
> >>> +      - items:
> >>> +          - const: tx
> >>> +          - const: rx
> >> 
> >> Why all combinations are possible?
> > 
> > Because they are. sai5 in rk3576 is rx only. sai7 is tx only. Others are
> > both
> What is sai5 and sai7? Instances on the same chip? Does it mean some of
> instances can only receive and some only transmit?

sai5 and sai7 are instances on the same chip, yes, see rk3576.dtsi changes in 
this series. Some instances can only receive, some can only transmit. Some can 
do both. I think the way I wrote the binding already allows for just "tx", 
just "rx", and "tx, rx" but not "rx, tx", which is I think what we want here.

> 
> > tx and rx. Do you want me to enforce that those with both are always tx
> > followed by rx?
> 
> Best regards,
> Krzysztof

Cheers,
Nicolas Frattaroli
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/rockchip,sai.yaml b/Documentation/devicetree/bindings/sound/rockchip,sai.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..8f5a292a7f2a6c737d983d00cbe40ec45bfa6249
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip,sai.yaml
@@ -0,0 +1,151 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/rockchip,sai.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Serial Audio Interface Controller
+
+description:
+  The Rockchip Serial Audio Interface (SAI) controller is a flexible audio
+  controller that implements the I2S, I2S/TDM and the PDM standards.
+
+maintainers:
+  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    const: rockchip,rk3576-sai
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  dmas:
+    minItems: 1
+    maxItems: 2
+
+  dma-names:
+    minItems: 1
+    maxItems: 2
+    oneOf:
+      - const: tx
+      - const: rx
+      - items:
+          - const: tx
+          - const: rx
+
+  clocks:
+    items:
+      - description: master audio clock
+      - description: AHB clock driving the interface
+
+  clock-names:
+    items:
+      - const: mclk
+      - const: hclk
+
+  resets:
+    minItems: 1
+    maxItems: 2
+    description: resets for the mclk domain and ahb domain
+
+  reset-names:
+    minItems: 1
+    items:
+      - const: m
+      - const: h
+
+  port:
+    $ref: audio-graph-port.yaml#
+    unevaluatedProperties: false
+
+  power-domains:
+    maxItems: 1
+
+  "#sound-dai-cells":
+    const: 0
+
+  rockchip,sai-rx-route:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Defines the mapping of the controller's SDI ports to actual input lanes,
+      as well as the number of input lanes.
+      rockchip,sai-rx-route = <3> would mean sdi3 is receiving from data0, and
+      that there is only one receiving lane.
+      This property's absence is to be understood as only one receiving lane
+      being used if the controller has capture capabilities.
+    maxItems: 4
+    items:
+      enum: [0, 1, 2, 3]
+
+  rockchip,sai-tx-route:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Defines the mapping of the controller's SDO ports to actual output lanes,
+      as well as the number of output lanes.
+      rockchip,sai-tx-route = <3> would mean sdo3 is sending to data0, and
+      that there is only one transmitting lane.
+      This property's absence is to be understood as only one transmitting lane
+      being used if the controller has playback capabilities.
+    maxItems: 4
+    items:
+      enum: [0, 1, 2, 3]
+
+  rockchip,always-on:
+    type: boolean
+    description:
+      The hardware requires this controller to remain turned on.
+
+
+required:
+  - compatible
+  - reg
+  - dmas
+  - dma-names
+  - clocks
+  - clock-names
+  - "#sound-dai-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rockchip,rk3576-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/pinctrl/rockchip.h>
+    #include <dt-bindings/power/rockchip,rk3576-power.h>
+    #include <dt-bindings/reset/rockchip,rk3576-cru.h>
+
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        sai1: sai@2a610000 {
+            compatible = "rockchip,rk3576-sai";
+            reg = <0x0 0x2a610000 0x0 0x1000>;
+            interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cru MCLK_SAI1_8CH>, <&cru HCLK_SAI1_8CH>;
+            clock-names = "mclk", "hclk";
+            dmas = <&dmac0 2>, <&dmac0 3>;
+            dma-names = "tx", "rx";
+            power-domains = <&power RK3576_PD_AUDIO>;
+            resets = <&cru SRST_M_SAI1_8CH>, <&cru SRST_H_SAI1_8CH>;
+            reset-names = "m", "h";
+            pinctrl-names = "default";
+            pinctrl-0 = <&sai1m0_lrck
+                         &sai1m0_sclk
+                         &sai1m0_sdi0
+                         &sai1m0_sdo0
+                         &sai1m0_sdo1
+                         &sai1m0_sdo2
+                         &sai1m0_sdo3>;
+            rockchip,sai-tx-route = <3 1 2 0>;
+            #sound-dai-cells = <0>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e9e0e52f92e3c649835ddfb65b2fde0a486198b..a68bf5391df6d3785f376c68ae61669d310cfb17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20661,6 +20661,12 @@  F:	Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml
 F:	sound/soc/codecs/rk3308_codec.c
 F:	sound/soc/codecs/rk3308_codec.h
 
+ROCKCHIP SAI DRIVER
+M:	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+L:	linux-rockchip@lists.infradead.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/sound/rockchip,sai.yaml
+
 ROCKCHIP VIDEO DECODER DRIVER
 M:	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
 L:	linux-media@vger.kernel.org