Re: JSON schema and conditions

[Maxime Ripard <maxime.ripard@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 3794 bytes --]

Hi,

On Tue, Jan 29, 2019 at 04:25:52PM -0600, Rob Herring wrote:
> On Tue, Jan 29, 2019 at 3:19 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Fri, Jan 18, 2019 at 11:57:12AM -0600, Rob Herring wrote:
> > > > > Including/inheriting another schema can be done with "allOf: [ {$ref:
> > > > > path/to/base/schema} ]". I'm currently using this for providers such
> > > > > as clock or reset providers and for buses. This works well for
> > > > > inheriting schemas which are collections of properties. See the GIC
> > > > > conversions to json-schema I posted for an example. The main issue
> > > > > with this approach that I've found is you have to list all the
> > > > > inherited properties to make them required or if you have
> > > > > 'addtionalProperties: false' (which is desirable IMO).
> > > > >
> > > > > If there's a lot of conditionals, there may not be much left common to
> > > > > inherit and we may just want to split each compatible into a separate
> > > > > doc. I'm also fine with leaving those constraints as comments or
> > > > > description for now. That's no worse than what we have today.
> > > > >
> > > > > Note that so far, all the $ref values pointing to other files get
> > > > > resolved to files in the yaml-bindings repo schemas. I don't think a
> > > > > ref from the kernel tree to the kernel tree works currently. I need to
> > > > > sort that out.
> > > >
> > > > Yeah, I've tried that already, and it indeed looks like it always try
> > > > to look them up on your github repo (or the local cache), but will not
> > > > try to locate it in the kernel tree.
> > >
> > > Actually, this should already kind of work, but only if all schema
> > > files are used. IOW, it doesn't work if you limit the schema files
> > > setting DT_SCHEMA_FILES. A reference to '/schemas/foo/bar.yaml' will
> > > match 'Documentation/devicetree/bindings/foo/bar.yaml'.
> > >
> > > I looked at trying to fix this, but at the point I need to load the
> > > reference I don't have the kernel source path, only the build path.
> > > The easiest fix would be to ignore (or only warn on) unresolved
> > > schemas, so we don't crash at least. Then you'd have to set
> > > DT_SCHEMA_FILES to the inherited schema if you wanted to validate with
> > > that.
> >
> > So I gave it a try with the following patch:
> > http://code.bulix.org/k8yxtm-566934?raw
> >
> > The output is:
> >
> > $DIR/arch/arm/boot/dts/sun4i-a10-cubieboard.dt.yaml: Additional properties are not allowed ('target-supply', 'clocks' were unexpected)
> > $DIR/arch/arm/boot/dts/sun4i-a10-cubieboard.dt.yaml: compatible: ['allwinner,sun4i-a10-ahci'] is not valid under any of the given schemas
> >
> > So it looks like it doesn't pick everything up, but yet if the number
> > of clocks is wrong, it's reported, which looks kind of weird since
> > compatible apparently doesn't match. Do you see any flaw with this?
> 
> Essentially, each schema file here is evaluated separately not as a
> union of the 2 files. You could just drop 'additionalProperties', but
> then either ahci-platform.yaml should not have compatible defined (you
> could have a 3rd file with that) or it needs *all* the possible
> compatibles. Probably the former would be better.

I'll give it a try then, thanks for the suggestion

> However, if the common one just has reg and interrupts, then there's
> probably not much point in trying to share with
> sun4i-a10-cubieboard.dt.yaml. But I'm assuming there's more
> properties you just haven't put in?

It's pretty much it, but I wanted to have a rather basic example
before trying to go into more complex bindings :)

Maxime

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

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