Re: [PATCH 1/2] dt-bindings: sound: Convert Allwinner SPDIF binding to YAML

Maxime Ripard
2019-04-17T18:22:49Z

[-- Attachment #1.1: Type: text/plain, Size: 2527 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]