[PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Rob Herring]

Convert the fixed-clock binding to DT schema format using json-schema.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/clock/fixed-clock.txt | 23 ----------
 .../bindings/clock/fixed-clock.yaml           | 44 +++++++++++++++++++
 2 files changed, 44 insertions(+), 23 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.txt b/Documentation/devicetree/bindings/clock/fixed-clock.txt
deleted file mode 100644
index 0641a663ad69..000000000000
--- a/Documentation/devicetree/bindings/clock/fixed-clock.txt
+++ /dev/null
@@ -1,23 +0,0 @@
-Binding for simple fixed-rate clock sources.
-
-This binding uses the common clock binding[1].
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Required properties:
-- compatible : shall be "fixed-clock".
-- #clock-cells : from common clock binding; shall be set to 0.
-- clock-frequency : frequency of clock in Hz. Should be a single cell.
-
-Optional properties:
-- clock-accuracy : accuracy of clock in ppb (parts per billion).
-		   Should be a single cell.
-- clock-output-names : From common clock binding.
-
-Example:
-	clock {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1000000000>;
-		clock-accuracy = <100>;
-	};
diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
new file mode 100644
index 000000000000..8b5628463b90
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/fixed-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Binding for simple fixed-rate clock sources.
+
+maintainers:
+  - Michael Turquette <mturquette@baylibre.com>
+  - Stephen Boyd <sboyd@kernel.org>
+
+properties:
+  compatible:
+    const: fixed-clock
+
+  "#clock-cells":
+    const: 0
+
+  clock-frequency: true
+
+  clock-accuracy:
+    description: accuracy of clock in ppb (parts per billion).
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  clock-output-names:
+    maxItems: 1
+
+required:
+  - compatible
+  - "#clock-cells"
+  - clock-frequency
+
+additionalProperties: false
+
+examples:
+  - |
+    clock {
+      compatible = "fixed-clock";
+      #clock-cells = <0>;
+      clock-frequency = <1000000000>;
+      clock-accuracy = <100>;
+    };
+...
-- 
2.19.1

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Stephen Boyd]

Quoting Rob Herring (2019-01-10 14:19:01)
> Convert the fixed-clock binding to DT schema format using json-schema.
> 

Any pointer to the full schema?

> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
[...]
> diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> new file mode 100644
> index 000000000000..8b5628463b90
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/fixed-clock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binding for simple fixed-rate clock sources.

Why does title have a full stop?

> +
> +maintainers:
> +  - Michael Turquette <mturquette@baylibre.com>
> +  - Stephen Boyd <sboyd@kernel.org>
> +
> +properties:
> +  compatible:
> +    const: fixed-clock
> +
> +  "#clock-cells":
> +    const: 0
> +
> +  clock-frequency: true

Why doesn't this need the $ref: /schemas/types.yaml#/... thing?

> +  clock-accuracy:
> +    description: accuracy of clock in ppb (parts per billion).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  clock-output-names:
> +    maxItems: 1

Is there a schema for strings?

> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - clock-frequency
> +
> +additionalProperties: false

Does this always have to be specified even if it's false?

> +
> +examples:
> +  - |
> +    clock {
> +      compatible = "fixed-clock";
> +      #clock-cells = <0>;
> +      clock-frequency = <1000000000>;
> +      clock-accuracy = <100>;
> +    };
> +...
> 

Is the triple dot part of the schema?

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Rob Herring]

On Fri, Jan 11, 2019 at 11:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Rob Herring (2019-01-10 14:19:01)
> > Convert the fixed-clock binding to DT schema format using json-schema.
> >
>
> Any pointer to the full schema?

https://github.com/robherring/yaml-bindings/blob/master/schemas/

And the clock schema in particular:
https://github.com/robherring/yaml-bindings/blob/master/schemas/clock.yaml

> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Cc: linux-clk@vger.kernel.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> [...]
> > diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> > new file mode 100644
> > index 000000000000..8b5628463b90
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/fixed-clock.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Binding for simple fixed-rate clock sources.
>
> Why does title have a full stop?

Because it was there in the original. My script to extract just takes
the first line of alphanumeric text.

> > +
> > +maintainers:
> > +  - Michael Turquette <mturquette@baylibre.com>
> > +  - Stephen Boyd <sboyd@kernel.org>
> > +
> > +properties:
> > +  compatible:
> > +    const: fixed-clock
> > +
> > +  "#clock-cells":
> > +    const: 0
> > +
> > +  clock-frequency: true
>
> Why doesn't this need the $ref: /schemas/types.yaml#/... thing?

You might want to read bindings/example-schema.yaml which tries to
explain some of this.

Standard properties are already defined in the core schemas. So we
only have to say "we use this property" and any binding specific
constraints. In this case, there aren't any other constraints. It is
also needed to be listed here to mark it required and to satisfy
'additionalProperties: false'.

> > +  clock-accuracy:
> > +    description: accuracy of clock in ppb (parts per billion).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  clock-output-names:
> > +    maxItems: 1
>
> Is there a schema for strings?

Yes. The core already covers that '*-names' properties are a list of
strings. So no need to do that again here and we just need to define
how many strings are valid.

> > +
> > +required:
> > +  - compatible
> > +  - "#clock-cells"
> > +  - clock-frequency
> > +
> > +additionalProperties: false
>
> Does this always have to be specified even if it's false?

Yes, the default defined as true by the json-schema spec. In some
cases we don't want to specify it.

> > +
> > +examples:
> > +  - |
> > +    clock {
> > +      compatible = "fixed-clock";
> > +      #clock-cells = <0>;
> > +      clock-frequency = <1000000000>;
> > +      clock-accuracy = <100>;
> > +    };
> > +...
> >
>
> Is the triple dot part of the schema?

Yes. Well, it is part of YAML. It's optional really as are the 2 lines
at the start. It's just good hygiene.

Rob

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Stephen Boyd]

Quoting Rob Herring (2019-01-11 10:27:48)
> On Fri, Jan 11, 2019 at 11:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Rob Herring (2019-01-10 14:19:01)
> > > Convert the fixed-clock binding to DT schema format using json-schema.
> > >
> >
> > Any pointer to the full schema?
> 
> https://github.com/robherring/yaml-bindings/blob/master/schemas/
> 
> And the clock schema in particular:
> https://github.com/robherring/yaml-bindings/blob/master/schemas/clock.yaml

Awesome. Thanks for the pointers! Is the clock schema posted to the list
somewhere?

> 
> > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > Cc: linux-clk@vger.kernel.org
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > [...]
> > > diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> > > new file mode 100644
> > > index 000000000000..8b5628463b90
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> > > @@ -0,0 +1,44 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/clock/fixed-clock.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Binding for simple fixed-rate clock sources.
> >
> > Why does title have a full stop?
> 
> Because it was there in the original. My script to extract just takes
> the first line of alphanumeric text.

Ok. I think it would be good to treat them like commit subjects that
don't have the full stop either, so if the script is able to drop the
full stop it would be great.

> 
> > > +
> > > +maintainers:
> > > +  - Michael Turquette <mturquette@baylibre.com>
> > > +  - Stephen Boyd <sboyd@kernel.org>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: fixed-clock
> > > +
> > > +  "#clock-cells":
> > > +    const: 0
> > > +
> > > +  clock-frequency: true
> >
> > Why doesn't this need the $ref: /schemas/types.yaml#/... thing?
> 
> You might want to read bindings/example-schema.yaml which tries to
> explain some of this.
> 
> Standard properties are already defined in the core schemas. So we
> only have to say "we use this property" and any binding specific
> constraints. In this case, there aren't any other constraints. It is
> also needed to be listed here to mark it required and to satisfy
> 'additionalProperties: false'.

Hmm ok. I suppose I'll have to hold that information in my mind when
reviewing bindings. Is there any tooling or some script that I can run
on json-schema bindings to verify they're correct? Or to find something
redundant like this where a standard property is redefined? Grep would
work for the redundant problem I suppose.

> 
> > > +  clock-accuracy:
> > > +    description: accuracy of clock in ppb (parts per billion).
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +  clock-output-names:
> > > +    maxItems: 1
> >
> > Is there a schema for strings?
> 
> Yes. The core already covers that '*-names' properties are a list of
> strings. So no need to do that again here and we just need to define
> how many strings are valid.

Alright. Thanks for the explanations.

> 
> > > +
> > > +required:
> > > +  - compatible
> > > +  - "#clock-cells"
> > > +  - clock-frequency
> > > +
> > > +additionalProperties: false
> >
> > Does this always have to be specified even if it's false?
> 
> Yes, the default defined as true by the json-schema spec. In some
> cases we don't want to specify it.
> 
> > > +
> > > +examples:
> > > +  - |
> > > +    clock {
> > > +      compatible = "fixed-clock";
> > > +      #clock-cells = <0>;
> > > +      clock-frequency = <1000000000>;
> > > +      clock-accuracy = <100>;
> > > +    };
> > > +...
> > >
> >
> > Is the triple dot part of the schema?
> 
> Yes. Well, it is part of YAML. It's optional really as are the 2 lines
> at the start. It's just good hygiene.
> 

Sounds good.

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Rob Herring]

On Fri, Jan 11, 2019 at 12:54 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Rob Herring (2019-01-11 10:27:48)
> > On Fri, Jan 11, 2019 at 11:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Rob Herring (2019-01-10 14:19:01)
> > > > Convert the fixed-clock binding to DT schema format using json-schema.
> > > >
> > >
> > > Any pointer to the full schema?
> >
> > https://github.com/robherring/yaml-bindings/blob/master/schemas/
> >
> > And the clock schema in particular:
> > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock.yaml
>
> Awesome. Thanks for the pointers! Is the clock schema posted to the list
> somewhere?

No. Happy to post things, but I'm struggling to find anyone that cares.

> > > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > > Cc: linux-clk@vger.kernel.org
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > [...]
> > > > diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> > > > new file mode 100644
> > > > index 000000000000..8b5628463b90
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
> > > > @@ -0,0 +1,44 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/clock/fixed-clock.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Binding for simple fixed-rate clock sources.
> > >
> > > Why does title have a full stop?
> >
> > Because it was there in the original. My script to extract just takes
> > the first line of alphanumeric text.
>
> Ok. I think it would be good to treat them like commit subjects that
> don't have the full stop either, so if the script is able to drop the
> full stop it would be great.

I can just write a meta-schema to enforce that. :)

> > > > +
> > > > +maintainers:
> > > > +  - Michael Turquette <mturquette@baylibre.com>
> > > > +  - Stephen Boyd <sboyd@kernel.org>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: fixed-clock
> > > > +
> > > > +  "#clock-cells":
> > > > +    const: 0
> > > > +
> > > > +  clock-frequency: true
> > >
> > > Why doesn't this need the $ref: /schemas/types.yaml#/... thing?
> >
> > You might want to read bindings/example-schema.yaml which tries to
> > explain some of this.
> >
> > Standard properties are already defined in the core schemas. So we
> > only have to say "we use this property" and any binding specific
> > constraints. In this case, there aren't any other constraints. It is
> > also needed to be listed here to mark it required and to satisfy
> > 'additionalProperties: false'.
>
> Hmm ok. I suppose I'll have to hold that information in my mind when
> reviewing bindings. Is there any tooling or some script that I can run
> on json-schema bindings to verify they're correct? Or to find something
> redundant like this where a standard property is redefined? Grep would
> work for the redundant problem I suppose.

Yes, the meta-schema enforces this at least to the extent there is a
meta-schema defined for a standard property. Even if not something for
a specific property, we limit things to a subset of json-schema
keywords. Also, you also can't define something contradicting a core
schema (e.g. { reg: { type: string } }), but that wouldn't be found
until you check actual DTs.

The meta-schema check is run with:
make dt_binding_check

This is all documented in Documentation/devicetree/writing-schema.md.

Rob

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Stephen Boyd]

Quoting Rob Herring (2019-01-11 12:49:04)
> On Fri, Jan 11, 2019 at 12:54 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Rob Herring (2019-01-11 10:27:48)
> > > On Fri, Jan 11, 2019 at 11:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > >
> > > > Any pointer to the full schema?
> > >
> > > https://github.com/robherring/yaml-bindings/blob/master/schemas/
> > >
> > > And the clock schema in particular:
> > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock.yaml
> >
> > Awesome. Thanks for the pointers! Is the clock schema posted to the list
> > somewhere?
> 
> No. Happy to post things, but I'm struggling to find anyone that cares.

Ok. I can review that document too if you post it to the list.

> 
> > > > Why does title have a full stop?
> > >
> > > Because it was there in the original. My script to extract just takes
> > > the first line of alphanumeric text.
> >
> > Ok. I think it would be good to treat them like commit subjects that
> > don't have the full stop either, so if the script is able to drop the
> > full stop it would be great.
> 
> I can just write a meta-schema to enforce that. :)

Sounds good.

> 
> Yes, the meta-schema enforces this at least to the extent there is a
> meta-schema defined for a standard property. Even if not something for
> a specific property, we limit things to a subset of json-schema
> keywords. Also, you also can't define something contradicting a core
> schema (e.g. { reg: { type: string } }), but that wouldn't be found
> until you check actual DTs.
> 
> The meta-schema check is run with:
> make dt_binding_check
> 
> This is all documented in Documentation/devicetree/writing-schema.md.
> 

Ok I'll read that document now. Would be cool if the build robots (and
myself) can somehow run the dt_binding_check on a single YAML file so we
can quickly validate the binding. Maybe even make C=2 or C=1 do that?

If the robots can then complain if the single file fails to build it
will save us tons of time. I just tried to run it but it seems to only
care about running on all the YAML files.

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Rob Herring]

On Fri, Jan 11, 2019 at 4:42 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Rob Herring (2019-01-11 12:49:04)
> > On Fri, Jan 11, 2019 at 12:54 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Rob Herring (2019-01-11 10:27:48)
> > > > On Fri, Jan 11, 2019 at 11:44 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > > >
> > > > > Any pointer to the full schema?
> > > >
> > > > https://github.com/robherring/yaml-bindings/blob/master/schemas/
> > > >
> > > > And the clock schema in particular:
> > > > https://github.com/robherring/yaml-bindings/blob/master/schemas/clock.yaml
> > >
> > > Awesome. Thanks for the pointers! Is the clock schema posted to the list
> > > somewhere?
> >
> > No. Happy to post things, but I'm struggling to find anyone that cares.
>
> Ok. I can review that document too if you post it to the list.
>
> >
> > > > > Why does title have a full stop?
> > > >
> > > > Because it was there in the original. My script to extract just takes
> > > > the first line of alphanumeric text.
> > >
> > > Ok. I think it would be good to treat them like commit subjects that
> > > don't have the full stop either, so if the script is able to drop the
> > > full stop it would be great.
> >
> > I can just write a meta-schema to enforce that. :)
>
> Sounds good.
>
> >
> > Yes, the meta-schema enforces this at least to the extent there is a
> > meta-schema defined for a standard property. Even if not something for
> > a specific property, we limit things to a subset of json-schema
> > keywords. Also, you also can't define something contradicting a core
> > schema (e.g. { reg: { type: string } }), but that wouldn't be found
> > until you check actual DTs.
> >
> > The meta-schema check is run with:
> > make dt_binding_check
> >
> > This is all documented in Documentation/devicetree/writing-schema.md.
> >
>
> Ok I'll read that document now. Would be cool if the build robots (and
> myself) can somehow run the dt_binding_check on a single YAML file so we
> can quickly validate the binding. Maybe even make C=2 or C=1 do that?
>
> If the robots can then complain if the single file fails to build it
> will save us tons of time. I just tried to run it but it seems to only
> care about running on all the YAML files.

make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/file/to/test

It will check only the schema specified and check all the DT files
against only that schema (plus any referenced schema if you use a
top-level 'allOf'. See the GIC schema patch[2] for an example of
that).

I'm running the schema check for patches in the DT patchwork instance.
For example, here's this patch[1]. So as long as it has my R-by, then
it should be good. I'm not yet running the schema check on DT files
because generally there are warnings and I don't intend to fix them
for everyone. But maybe I should anyways so we at least have the data.

Rob

[1] https://patchwork.ozlabs.org/patch/1023240/
[2] https://patchwork.ozlabs.org/patch/1023243/

Re: [PATCH] dt-bindings: clock: Convert fixed-clock binding to json-schema

[Stephen Boyd]

Quoting Rob Herring (2019-01-11 15:09:58)
> 
> make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/file/to/test
> 
> It will check only the schema specified and check all the DT files
> against only that schema (plus any referenced schema if you use a
> top-level 'allOf'. See the GIC schema patch[2] for an example of
> that).
> 
> I'm running the schema check for patches in the DT patchwork instance.
> For example, here's this patch[1]. So as long as it has my R-by, then
> it should be good. I'm not yet running the schema check on DT files
> because generally there are warnings and I don't intend to fix them
> for everyone. But maybe I should anyways so we at least have the data.
> 

Ok. I'll apply this patch to clk-next with my small fixes rolled in.