| Message ID | d9afb19c32f8b9b2c40c8d4c0c3df74bff0ccf35.1557252411.git-series.maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: sound: Convert Allwinner SPDIF binding to YAML | expand |
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
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
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
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(-)