linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] dt-bindings: sound: Convert Allwinner SPDIF binding to YAML
@ 2019-05-07 18:07 Maxime Ripard
  2019-05-07 18:07 ` [PATCH v2 2/2] dt-bindings: sound: sun4i-spdif: Document that the RX channel can be missing Maxime Ripard
  0 siblings, 1 reply; 4+ messages in thread
From: Maxime Ripard @ 2019-05-07 18:07 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Mark Rutland, Rob Herring, Frank Rowand
  Cc: Maxime Ripard, devicetree, alsa-devel, Chen-Yu Tsai, linux-arm-kernel

The Allwinner SoCs feature an SPDIF controller across multiple SoC
generations.

However, earlier generations were a bit simpler than the subsequent ones,
and for example would always have RX and TX capabilities, and no reset
lines.

In order to express this, let's create two YAML schemas instead of the free
form text we had before.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

---

Changes from v1:
  - Merged the two schemas together and used the draft-7 conditionals
---
 Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt          | 42 +-------------------------------
 2 files changed, 98 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
 delete mode 100644 Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
new file mode 100644
index 000000000000..5698e5de5e31
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/allwinner,sun4i-a10-spdif.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner A10 S/PDIF Controller Device Tree Bindings
+
+maintainers:
+  - Chen-Yu Tsai <wens@csie.org>
+  - Liam Girdwood <lgirdwood@gmail.com>
+  - Mark Brown <broonie@kernel.org>
+  - Maxime Ripard <maxime.ripard@bootlin.com>
+
+properties:
+  "#sound-dai-cells":
+    const: 0
+
+  compatible:
+    oneOf:
+      - const: allwinner,sun4i-a10-spdif
+      - const: allwinner,sun6i-a31-spdif
+      - const: allwinner,sun8i-h3-spdif
+      - items:
+          - const: allwinner,sun8i-a83t-spdif
+          - const: allwinner,sun8i-h3-spdif
+      - items:
+          - const: allwinner,sun50i-a64-spdif
+          - const: allwinner,sun8i-h3-spdif
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Bus Clock
+      - description: Module Clock
+
+  clock-names:
+    items:
+      - const: apb
+      - const: spdif
+
+  dmas:
+    items:
+      - description: RX DMA Channel
+      - description: TX DMA Channel
+
+  dma-names:
+    items:
+      - const: rx
+      - const: tx
+
+  resets:
+    maxItems: 1
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - allwinner,sun6i-a31-spdif
+              - allwinner,sun8i-h3-spdif
+
+    then:
+      required:
+        - resets
+
+required:
+  - "#sound-dai-cells"
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - dmas
+  - dma-names
+
+additionalProperties: false
+
+examples:
+  - |
+    spdif: spdif@1c21000 {
+        #sound-dai-cells = <0>;
+        compatible = "allwinner,sun4i-a10-spdif";
+        reg = <0x01c21000 0x40>;
+        interrupts = <13>;
+        clocks = <&apb0_gates 1>, <&spdif_clk>;
+        clock-names = "apb", "spdif";
+        dmas = <&dma 0 2>, <&dma 0 2>;
+        dma-names = "rx", "tx";
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt b/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt
deleted file mode 100644
index 0c64a209c2e9..000000000000
--- a/Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt
+++ /dev/null
@@ -1,42 +0,0 @@
-Allwinner Sony/Philips Digital Interface Format (S/PDIF) Controller
-
-The Allwinner S/PDIF audio block is a transceiver that allows the
-processor to receive and transmit digital audio via an coaxial cable or
-a fibre cable.
-For now only playback is supported.
-
-Required properties:
-
-  - compatible		: should be one of the following:
-    - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
-    - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC
-    - "allwinner,sun8i-h3-spdif": for the Allwinner H3 SoC
-
-  - reg			: Offset and length of the register set for the device.
-
-  - interrupts		: Contains the spdif interrupt.
-
-  - dmas		: Generic dma devicetree binding as described in
-			  Documentation/devicetree/bindings/dma/dma.txt.
-
-  - dma-names		: Two dmas have to be defined, "tx" and "rx".
-
-  - clocks		: Contains an entry for each entry in clock-names.
-
-  - clock-names		: Includes the following entries:
-	"apb"		  clock for the spdif bus.
-	"spdif"		  clock for spdif controller.
-
-  - resets		: reset specifier for the ahb reset (A31 and newer only)
-
-Example:
-
-spdif: spdif@1c21000 {
-	compatible = "allwinner,sun4i-a10-spdif";
-	reg = <0x01c21000 0x40>;
-	interrupts = <13>;
-	clocks = <&apb0_gates 1>, <&spdif_clk>;
-	clock-names = "apb", "spdif";
-	dmas = <&dma 0 2>, <&dma 0 2>;
-	dma-names = "rx", "tx";
-};

base-commit: 00c3bc00f0cadbb48f2414370733892676511e79
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] dt-bindings: sound: sun4i-spdif: Document that the RX channel can be missing
  2019-05-07 18:07 [PATCH v2 1/2] dt-bindings: sound: Convert Allwinner SPDIF binding to YAML Maxime Ripard
@ 2019-05-07 18:07 ` Maxime Ripard
  2019-05-08 19:35   ` Rob Herring
  0 siblings, 1 reply; 4+ messages in thread
From: Maxime Ripard @ 2019-05-07 18:07 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Mark Rutland, Rob Herring, Frank Rowand
  Cc: Maxime Ripard, devicetree, alsa-devel, Chen-Yu Tsai, linux-arm-kernel

The H3 and compatibles controllers don't have any reception capabilities,
even though it was never documented as such in the binding before.

Therefore, on those controllers, we don't have the option to set an RX DMA
channel.

This was already done in the DTSI, but the binding itself was never
updated. Let's add a special case in the schemas.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

---

Changes from v1:
  - switch to a draft7 conditional
---
 Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
index 5698e5de5e31..8f1bc1a1af96 100644
--- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
@@ -44,15 +44,8 @@ properties:
       - const: apb
       - const: spdif
 
-  dmas:
-    items:
-      - description: RX DMA Channel
-      - description: TX DMA Channel
-
-  dma-names:
-    items:
-      - const: rx
-      - const: tx
+  dmas: true
+  dma-names: true
 
   resets:
     maxItems: 1
@@ -70,6 +63,40 @@ allOf:
       required:
         - resets
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun8i-h3-spdif
+
+    then:
+      properties:
+        dmas:
+          maxItems: 1
+          items:
+            - description: RX DMA Channel
+
+        dma-names:
+          maxItems: 1
+          items:
+            - const: tx
+
+    else:
+      properties:
+        dmas:
+          minItems: 2
+          maxItems: 2
+          items:
+            - description: RX DMA Channel
+            - description: TX DMA Channel
+
+        dma-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            - const: rx
+            - const: tx
+
 required:
   - "#sound-dai-cells"
   - compatible
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: sound: sun4i-spdif: Document that the RX channel can be missing
  2019-05-07 18:07 ` [PATCH v2 2/2] dt-bindings: sound: sun4i-spdif: Document that the RX channel can be missing Maxime Ripard
@ 2019-05-08 19:35   ` Rob Herring
  2019-05-10 10:26     ` Maxime Ripard
  0 siblings, 1 reply; 4+ messages in thread
From: Rob Herring @ 2019-05-08 19:35 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, Linux-ALSA, Liam Girdwood,
	Chen-Yu Tsai, Mark Brown, Frank Rowand,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Tue, May 7, 2019 at 1:07 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> The H3 and compatibles controllers don't have any reception capabilities,
> even though it was never documented as such in the binding before.
>
> Therefore, on those controllers, we don't have the option to set an RX DMA
> channel.
>
> This was already done in the DTSI, but the binding itself was never
> updated. Let's add a special case in the schemas.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> ---
>
> Changes from v1:
>   - switch to a draft7 conditional
> ---
>  Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml | 45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
> index 5698e5de5e31..8f1bc1a1af96 100644
> --- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
> +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml
> @@ -44,15 +44,8 @@ properties:
>        - const: apb
>        - const: spdif
>
> -  dmas:
> -    items:
> -      - description: RX DMA Channel
> -      - description: TX DMA Channel
> -
> -  dma-names:
> -    items:
> -      - const: rx
> -      - const: tx
> +  dmas: true
> +  dma-names: true
>
>    resets:
>      maxItems: 1
> @@ -70,6 +63,40 @@ allOf:
>        required:
>          - resets
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: allwinner,sun8i-h3-spdif
> +
> +    then:
> +      properties:
> +        dmas:
> +          maxItems: 1

In this and below, these should get added automatically by
fixup_schema. If not present, we set minItems/maxItems to the size of
the items list. It look like you added support for that, so left over
from before you addressed that for if/then/else?

> +          items:
> +            - description: RX DMA Channel

s/RX/TX/

> +
> +        dma-names:
> +          maxItems: 1
> +          items:
> +            - const: tx
> +
> +    else:
> +      properties:
> +        dmas:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - description: RX DMA Channel
> +            - description: TX DMA Channel
> +
> +        dma-names:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - const: rx
> +            - const: tx

I'm really on the fence whether it's worth it to add all this just add
the restrictions based on the compatible. I guess with copy-n-paste
this would be a common error.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: sound: sun4i-spdif: Document that the RX channel can be missing
  2019-05-08 19:35   ` Rob Herring
@ 2019-05-10 10:26     ` Maxime Ripard
  0 siblings, 0 replies; 4+ messages in thread
From: Maxime Ripard @ 2019-05-10 10:26 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Linux-ALSA, Liam Girdwood,
	Chen-Yu Tsai, Mark Brown, Frank Rowand,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

Hi Rob,

On Wed, May 08, 2019 at 02:35:10PM -0500, Rob Herring wrote:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: allwinner,sun8i-h3-spdif
> > +
> > +    then:
> > +      properties:
> > +        dmas:
> > +          maxItems: 1
>
> In this and below, these should get added automatically by
> fixup_schema. If not present, we set minItems/maxItems to the size of
> the items list. It look like you added support for that, so left over
> from before you addressed that for if/then/else?

Sorry, I should have brought that up in the pull request.

It seems that it's still necessary when using allOf, otherwise the
schema won't match

Maybe there's something more to fix when using allOf?

> > +          items:
> > +            - description: RX DMA Channel
>
> s/RX/TX/
>
> > +
> > +        dma-names:
> > +          maxItems: 1
> > +          items:
> > +            - const: tx
> > +
> > +    else:
> > +      properties:
> > +        dmas:
> > +          minItems: 2
> > +          maxItems: 2
> > +          items:
> > +            - description: RX DMA Channel
> > +            - description: TX DMA Channel
> > +
> > +        dma-names:
> > +          minItems: 2
> > +          maxItems: 2
> > +          items:
> > +            - const: rx
> > +            - const: tx
>
> I'm really on the fence whether it's worth it to add all this just add
> the restrictions based on the compatible. I guess with copy-n-paste
> this would be a common error.

Converting most of the bindings to the schemas has shown that (at
least in our case), we've been pretty bad at keeping the documentation
up to date with that kind of information.

Adding that kind of construct at least has the benefit to actively
enforce that the documentation is complete.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-05-10 10:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 18:07 [PATCH v2 1/2] dt-bindings: sound: Convert Allwinner SPDIF binding to YAML Maxime Ripard
2019-05-07 18:07 ` [PATCH v2 2/2] dt-bindings: sound: sun4i-spdif: Document that the RX channel can be missing Maxime Ripard
2019-05-08 19:35   ` Rob Herring
2019-05-10 10:26     ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).