| Message ID | 4060ec46117ccf3cd643b8f1b9759ed44d38c5e7.1555330005.git-series.maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] dt-bindings: sound: Convert Allwinner SPDIF binding to YAML | expand |
On Mon, Apr 15, 2019 at 7:07 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > 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. The only difference is 'reset' is required in one? Perhaps better to just not make 'reset' required til we figure out how to handle a conditional like this. Seems like we discussed this and I probably said to split things? I guess it's a judgement call depending on how different things are. Possibly, we could handle this case like this: allOf: - $ref: allwinner,sun4i-a10-spdif.yaml properties: resets: maxItems: 1 required: - resets Plus we'd need the 'allwinner,sun6i-a31-spdif' and other compatibles in both files. Note that you can't use 'additionalProperties: false' in either file in this case. I don't really love this solution though. Rob
Hi Rob, On Tue, Apr 16, 2019 at 04:36:18PM -0500, Rob Herring wrote: > On Mon, Apr 15, 2019 at 7:07 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > 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. > > The only difference is 'reset' is required in one? Perhaps better to > just not make 'reset' required til we figure out how to handle a > conditional like this. Seems like we discussed this and I probably > said to split things? We indeed discussed this, and this was one of the solutions to support this. I wanted to get the discussion started as of how exactly we want to support this kind of construct. I looked it up a bit, and it looks like json schema 7 introduces conditionals that would allow us to deal with this nicely. https://json-schema.org/understanding-json-schema/reference/conditionals.html Is that something we can switch to while we have not a lot of schemas, or would that require some significant work? > I guess it's a judgement call depending on how different things are. > > Possibly, we could handle this case like this: > > allOf: > - $ref: allwinner,sun4i-a10-spdif.yaml > > properties: > resets: > maxItems: 1 > > required: > - resets > > > Plus we'd need the 'allwinner,sun6i-a31-spdif' and other compatibles > in both files. Note that you can't use 'additionalProperties: false' > in either file in this case. > > I don't really love this solution though. Yeah, I'm not a big fan of it either. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, Apr 17, 2019 at 1:43 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi Rob, > > On Tue, Apr 16, 2019 at 04:36:18PM -0500, Rob Herring wrote: > > On Mon, Apr 15, 2019 at 7:07 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > 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. > > > > The only difference is 'reset' is required in one? Perhaps better to > > just not make 'reset' required til we figure out how to handle a > > conditional like this. Seems like we discussed this and I probably > > said to split things? > > We indeed discussed this, and this was one of the solutions to support > this. I wanted to get the discussion started as of how exactly we want > to support this kind of construct. > > I looked it up a bit, and it looks like json schema 7 introduces > conditionals that would allow us to deal with this nicely. > > https://json-schema.org/understanding-json-schema/reference/conditionals.html > > Is that something we can switch to while we have not a lot of schemas, > or would that require some significant work? We need check if the json-schema library supports this yet. I think it should as there was an issue for it which is closed now. We'll then need to update the meta-schema and maybe the schema fixups to handle this case. We should have a test case in the library too. Test cases are important given that if you get schemas wrong, the result is silence. Rob
Hi, On Wed, Apr 17, 2019 at 11:04:22AM -0500, Rob Herring wrote: > On Wed, Apr 17, 2019 at 1:43 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Tue, Apr 16, 2019 at 04:36:18PM -0500, Rob Herring wrote: > > > On Mon, Apr 15, 2019 at 7:07 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > 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. > > > > > > The only difference is 'reset' is required in one? Perhaps better to > > > just not make 'reset' required til we figure out how to handle a > > > conditional like this. Seems like we discussed this and I probably > > > said to split things? > > > > We indeed discussed this, and this was one of the solutions to support > > this. I wanted to get the discussion started as of how exactly we want > > to support this kind of construct. > > > > I looked it up a bit, and it looks like json schema 7 introduces > > conditionals that would allow us to deal with this nicely. > > > > https://json-schema.org/understanding-json-schema/reference/conditionals.html > > > > Is that something we can switch to while we have not a lot of schemas, > > or would that require some significant work? > > We need check if the json-schema library supports this yet. I think it > should as there was an issue for it which is closed now. Apparently, it's supported since 3.0.0, that got released on the 24th of february. I mass converted the yaml-bindings code to use the draft 7, and the tests are passing (and a dtbs_check run seems legit too). So that went smoothly... > We'll then need to update the meta-schema and maybe the schema fixups > to handle this case. ... except that after adding if, then and else to the meta-schemas, a schema using that construct doesn't do anything (well, anything more than what it is doing without if / then / else). Apart from the fact that it doesn't work as expected (yet), the syntax is pretty elegant, so I guess we should go for that. > We should have a test case in the library too. > Test cases are important given that if you get schemas wrong, the > result is silence. ACK Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
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..f2e97f250519 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml @@ -0,0 +1,73 @@ +# 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: + const: allwinner,sun4i-a10-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 + +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/allwinner,sun6i-a31-spdif.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml new file mode 100644 index 000000000000..7329d9fcf34c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml @@ -0,0 +1,86 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/allwinner,sun6i-a31-spdif.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A31 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,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 + +required: + - "#sound-dai-cells" + - compatible + - reg + - interrupts + - clocks + - clock-names + - dmas + - dma-names + - resets + +additionalProperties: false + +examples: + - | + spdif: spdif@1c21000 { + #sound-dai-cells = <0>; + compatible = "allwinner,sun6i-a31-spdif"; + reg = <0x01c21000 0x400>; + interrupts = <0 12 4>; + clocks = <&ccu 62>, <&ccu 99>; + clock-names = "apb", "spdif"; + resets = <&ccu 43>; + dmas = <&dma 2>, <&dma 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"; -};
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> --- Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt | 42 +----------------------------------- 3 files changed, 159 insertions(+), 42 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-spdif.yaml create mode 100644 Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml delete mode 100644 Documentation/devicetree/bindings/sound/sunxi,sun4i-spdif.txt base-commit: f9221a7a1014d8a047b277a73289678646ddc110