Re: [PATCH 2/2] dt-bindings: sound: sun6i-spdif: Document that the RX channel can be missing

Rob Herring
2019-04-16T21:50:57Z

On Tue, Apr 16, 2019 at 2:19 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi Rob,
>
> On Mon, Apr 15, 2019 at 08:36:28PM -0500, Rob Herring wrote:
> > On Mon, Apr 15, 2019 at 7:07 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > Not all controllers using the A31 SPDIF binding actually have some RX
> > > capabilities, and therefore on some 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.
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > > Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml | 16 +++++++++++++---
> > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml
> > > index 7329d9fcf34c..800f794fafe0 100644
> > > --- a/Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/allwinner,sun6i-a31-spdif.yaml
> > > @@ -44,14 +44,24 @@ properties:
> > > - const: spdif
> > >
> > > dmas:
> > > + minItems: 1
> > > + maxItems: 2
> > > items:
> > > - description: RX DMA Channel
> > > - description: TX DMA Channel
> > > + description:
> > > + Some controllers cannot receive but can only transmit data. In
> > > + such a case, the RX DMA channel is to be omitted.
> >
> > Really, the schema is saying rx is optional, but it doesn't really
> > matter here as the schema for each item is just 'description'.
>
> What should I do here then?
>
> Remove the global description and leave only the one under items?

I think the opposite. Just drop 'items' and leave 'description'.

> Also, it won't necessarily match the dma-names (since rx might be
> there or not), does it matter or is it obvious enough that we don't
> care?
>
> > > dma-names:
> > > - items:
> > > - - const: rx
> > > - - const: tx
> > > + minItems: 1
> > > + maxItems: 2
> > > + enum:
> > > + - rx
> > > + - tx
> > > + description:
> > > + Some controllers cannot receive but can only transmit data. In
> > > + such a case, the RX name is to be omitted.
> >
> > Here it matters though. This would allow just 'tx', '"tx", "tx"', or
> > either order.
> >
> > You need something like this:
> >
> > oneOf:
> > - items:
> > - const: rx
> > - const: tx
> > - const: tx
>
> Ok.
>
> > Ideally, we'd always put the required entry first and avoid this
> > problem. Not always possible if the first entry gets removed in later
> > h/w.
>
> One of the question I was wondering myself when I wrote those schemas
> is how are we supposed to deal with lists that need to have a
> particular set of values, but without any particular order?

'items' can be a list or dictionary. When it's a dictionary, the
schema for 'items' is applied to each item. For example:

items:
enum: [ rx, tx ]
uniqueItems: true

'uniqueItems' prevents the case of 'rx, rx' or 'tx, tx'.

> rx and tx here is a good example of that. We need both (let's leave
> the "missing RX" case aside for a minute), but since we reference them
> by name, '"rx", "tx"' is strictly equivalent to '"tx", "rx"'. Yet,
> items cares about the order, so the latter would fail to validate with
> that schemas.

Even when we reference things by name, the order should be defined
still. Using names allows for skipping entries.

If you have a mixture, I'd prefer to see dts files cleaned-up.

Rob