2019-09-02 15:33:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

Convert the Syscon reboot bindings to DT schema format using
json-schema.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/power/reset/syscon-reboot.txt | 30 --------
.../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
2 files changed, 68 insertions(+), 30 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
deleted file mode 100644
index e23dea8344f8..000000000000
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Generic SYSCON mapped register reset driver
-
-This is a generic reset driver using syscon to map the reset register.
-The reset is generally performed with a write to the reset register
-defined by the register map pointed by syscon reference plus the offset
-with the value and mask defined in the reboot node.
-
-Required properties:
-- compatible: should contain "syscon-reboot"
-- regmap: this is phandle to the register map node
-- offset: offset in the register map for the reboot register (in bytes)
-- value: the reset value written to the reboot register (32 bit access)
-
-Optional properties:
-- mask: update only the register bits defined by the mask (32 bit)
-
-Legacy usage:
-If a node doesn't contain a value property but contains a mask property, the
-mask property is used as the value.
-
-Default will be little endian mode, 32 bit access only.
-
-Examples:
-
- reboot {
- compatible = "syscon-reboot";
- regmap = <&regmapnode>;
- offset = <0x0>;
- mask = <0x1>;
- };
diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
new file mode 100644
index 000000000000..a583f3dc8ef4
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic SYSCON mapped register reset driver
+
+maintainers:
+ - Sebastian Reichel <[email protected]>
+
+description: |+
+ This is a generic reset driver using syscon to map the reset register.
+ The reset is generally performed with a write to the reset register
+ defined by the register map pointed by syscon reference plus the offset
+ with the value and mask defined in the reboot node.
+ Default will be little endian mode, 32 bit access only.
+
+properties:
+ compatible:
+ const: syscon-reboot
+
+ mask:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Update only the register bits defined by the mask (32 bit).
+ maxItems: 1
+
+ offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Offset in the register map for the reboot register (in bytes).
+ maxItems: 1
+
+ regmap:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: Phandle to the register map node.
+ maxItems: 1
+
+ value:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: The reset value written to the reboot register (32 bit access).
+ maxItems: 1
+
+required:
+ - compatible
+ - regmap
+ - offset
+
+allOf:
+ - if:
+ properties:
+ value:
+ not:
+ type: array
+ then:
+ required:
+ - mask
+ else:
+ required:
+ - value
+
+examples:
+ - |
+ reboot {
+ compatible = "syscon-reboot";
+ regmap = <&regmapnode>;
+ offset = <0x0>;
+ mask = <0x1>;
+ };
--
2.17.1


2019-09-02 15:33:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/2] dt-bindings: power: syscon-poweroff: Convert bindings to json-schema

Convert the Syscon poweroff bindings to DT schema format using
json-schema.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/power/reset/syscon-poweroff.txt | 30 --------
.../bindings/power/reset/syscon-poweroff.yaml | 68 +++++++++++++++++++
2 files changed, 68 insertions(+), 30 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-poweroff.txt
create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-poweroff.yaml

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-poweroff.txt b/Documentation/devicetree/bindings/power/reset/syscon-poweroff.txt
deleted file mode 100644
index 022ed1f3bc80..000000000000
--- a/Documentation/devicetree/bindings/power/reset/syscon-poweroff.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Generic SYSCON mapped register poweroff driver
-
-This is a generic poweroff driver using syscon to map the poweroff register.
-The poweroff is generally performed with a write to the poweroff register
-defined by the register map pointed by syscon reference plus the offset
-with the value and mask defined in the poweroff node.
-
-Required properties:
-- compatible: should contain "syscon-poweroff"
-- regmap: this is phandle to the register map node
-- offset: offset in the register map for the poweroff register (in bytes)
-- value: the poweroff value written to the poweroff register (32 bit access)
-
-Optional properties:
-- mask: update only the register bits defined by the mask (32 bit)
-
-Legacy usage:
-If a node doesn't contain a value property but contains a mask property, the
-mask property is used as the value.
-
-Default will be little endian mode, 32 bit access only.
-
-Examples:
-
- poweroff {
- compatible = "syscon-poweroff";
- regmap = <&regmapnode>;
- offset = <0x0>;
- mask = <0x7a>;
- };
diff --git a/Documentation/devicetree/bindings/power/reset/syscon-poweroff.yaml b/Documentation/devicetree/bindings/power/reset/syscon-poweroff.yaml
new file mode 100644
index 000000000000..9336ffcf3ac8
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/syscon-poweroff.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/syscon-poweroff.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic SYSCON mapped register poweroff driver
+
+maintainers:
+ - Sebastian Reichel <[email protected]>
+
+description: |+
+ This is a generic poweroff driver using syscon to map the poweroff register.
+ The poweroff is generally performed with a write to the poweroff register
+ defined by the register map pointed by syscon reference plus the offset
+ with the value and mask defined in the poweroff node.
+ Default will be little endian mode, 32 bit access only.
+
+properties:
+ compatible:
+ const: syscon-poweroff
+
+ mask:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Update only the register bits defined by the mask (32 bit).
+ maxItems: 1
+
+ offset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Offset in the register map for the poweroff register (in bytes).
+ maxItems: 1
+
+ regmap:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: Phandle to the register map node.
+ maxItems: 1
+
+ value:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: The poweroff value written to the poweroff register (32 bit access).
+ maxItems: 1
+
+required:
+ - compatible
+ - regmap
+ - offset
+
+allOf:
+ - if:
+ properties:
+ value:
+ not:
+ type: array
+ then:
+ required:
+ - mask
+ else:
+ required:
+ - value
+
+examples:
+ - |
+ poweroff {
+ compatible = "syscon-poweroff";
+ regmap = <&regmapnode>;
+ offset = <0x0>;
+ mask = <0x7a>;
+ };
--
2.17.1

2019-09-03 07:16:10

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Mon, Sep 2, 2019 at 4:03 PM Krzysztof Kozlowski <[email protected]> wrote:
>
> Convert the Syscon reboot bindings to DT schema format using
> json-schema.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../bindings/power/reset/syscon-reboot.txt | 30 --------
> .../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
> 2 files changed, 68 insertions(+), 30 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml

> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> new file mode 100644
> index 000000000000..a583f3dc8ef4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic SYSCON mapped register reset driver
> +
> +maintainers:
> + - Sebastian Reichel <[email protected]>
> +
> +description: |+
> + This is a generic reset driver using syscon to map the reset register.
> + The reset is generally performed with a write to the reset register
> + defined by the register map pointed by syscon reference plus the offset
> + with the value and mask defined in the reboot node.
> + Default will be little endian mode, 32 bit access only.
> +
> +properties:
> + compatible:
> + const: syscon-reboot
> +
> + mask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Update only the register bits defined by the mask (32 bit).
> + maxItems: 1

Drop this as that is already defined for uint32.

It also doesn't actually work. The $ref has to be under an 'allOf' if
you have additional schemas. A quirk of json-schema...

> +
> + offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Offset in the register map for the reboot register (in bytes).
> + maxItems: 1
> +
> + regmap:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: Phandle to the register map node.
> + maxItems: 1
> +
> + value:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: The reset value written to the reboot register (32 bit access).
> + maxItems: 1
> +
> +required:
> + - compatible
> + - regmap
> + - offset
> +
> +allOf:
> + - if:
> + properties:
> + value:
> + not:
> + type: array

I think you could make this a bit more readable with:

if:
not:
required:
- value

However, if the tree is free of legacy usage, then you could just drop all this.

Rob

2019-09-03 07:48:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Tue, 3 Sep 2019 at 09:14, Rob Herring <[email protected]> wrote:
>
> On Mon, Sep 2, 2019 at 4:03 PM Krzysztof Kozlowski <[email protected]> wrote:
> >
> > Convert the Syscon reboot bindings to DT schema format using
> > json-schema.
> >
> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > ---
> > .../bindings/power/reset/syscon-reboot.txt | 30 --------
> > .../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
> > 2 files changed, 68 insertions(+), 30 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> > create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
>
> > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > new file mode 100644
> > index 000000000000..a583f3dc8ef4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic SYSCON mapped register reset driver
> > +
> > +maintainers:
> > + - Sebastian Reichel <[email protected]>
> > +
> > +description: |+
> > + This is a generic reset driver using syscon to map the reset register.
> > + The reset is generally performed with a write to the reset register
> > + defined by the register map pointed by syscon reference plus the offset
> > + with the value and mask defined in the reboot node.
> > + Default will be little endian mode, 32 bit access only.
> > +
> > +properties:
> > + compatible:
> > + const: syscon-reboot
> > +
> > + mask:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Update only the register bits defined by the mask (32 bit).
> > + maxItems: 1
>
> Drop this as that is already defined for uint32.
>
> It also doesn't actually work. The $ref has to be under an 'allOf' if
> you have additional schemas. A quirk of json-schema...
>
> > +
> > + offset:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Offset in the register map for the reboot register (in bytes).
> > + maxItems: 1
> > +
> > + regmap:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description: Phandle to the register map node.
> > + maxItems: 1
> > +
> > + value:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: The reset value written to the reboot register (32 bit access).
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - regmap
> > + - offset
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + value:
> > + not:
> > + type: array
>
> I think you could make this a bit more readable with:
>
> if:
> not:
> required:
> - value

I do not understand how does it work (value is not mentioned in the
required fields so why checking of it?)... but it works fine.

> However, if the tree is free of legacy usage, then you could just drop all this.

One of them - mask or value - has to be provided.

Thanks for review,
Kryzsztof

2019-09-03 09:01:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Tue, Sep 3, 2019 at 8:47 AM Krzysztof Kozlowski <[email protected]> wrote:
>
> On Tue, 3 Sep 2019 at 09:14, Rob Herring <[email protected]> wrote:
> >
> > On Mon, Sep 2, 2019 at 4:03 PM Krzysztof Kozlowski <[email protected]> wrote:
> > >
> > > Convert the Syscon reboot bindings to DT schema format using
> > > json-schema.
> > >
> > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > ---
> > > .../bindings/power/reset/syscon-reboot.txt | 30 --------
> > > .../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
> > > 2 files changed, 68 insertions(+), 30 deletions(-)
> > > delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> > > create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> >
> > > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > new file mode 100644
> > > index 000000000000..a583f3dc8ef4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > @@ -0,0 +1,68 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Generic SYSCON mapped register reset driver
> > > +
> > > +maintainers:
> > > + - Sebastian Reichel <[email protected]>
> > > +
> > > +description: |+
> > > + This is a generic reset driver using syscon to map the reset register.
> > > + The reset is generally performed with a write to the reset register
> > > + defined by the register map pointed by syscon reference plus the offset
> > > + with the value and mask defined in the reboot node.
> > > + Default will be little endian mode, 32 bit access only.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: syscon-reboot
> > > +
> > > + mask:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + description: Update only the register bits defined by the mask (32 bit).
> > > + maxItems: 1
> >
> > Drop this as that is already defined for uint32.
> >
> > It also doesn't actually work. The $ref has to be under an 'allOf' if
> > you have additional schemas. A quirk of json-schema...
> >
> > > +
> > > + offset:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + description: Offset in the register map for the reboot register (in bytes).
> > > + maxItems: 1
> > > +
> > > + regmap:
> > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > + description: Phandle to the register map node.
> > > + maxItems: 1
> > > +
> > > + value:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + description: The reset value written to the reboot register (32 bit access).
> > > + maxItems: 1
> > > +
> > > +required:
> > > + - compatible
> > > + - regmap
> > > + - offset
> > > +
> > > +allOf:
> > > + - if:
> > > + properties:
> > > + value:
> > > + not:
> > > + type: array
> >
> > I think you could make this a bit more readable with:
> >
> > if:
> > not:
> > required:
> > - value
>
> I do not understand how does it work (value is not mentioned in the
> required fields so why checking of it?)... but it works fine.

What's under required doesn't have to be listed as a property.

> > However, if the tree is free of legacy usage, then you could just drop all this.
>
> One of them - mask or value - has to be provided.

Or both, right?

Actually, a better way to express it is probably this:

oneOf:
- required: [ value ]
- required: [ mask ]
- required: [ value, mask ]

Rob

2019-09-03 11:45:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Tue, 3 Sep 2019 at 11:00, Rob Herring <[email protected]> wrote:
>
> On Tue, Sep 3, 2019 at 8:47 AM Krzysztof Kozlowski <[email protected]> wrote:
> >
> > On Tue, 3 Sep 2019 at 09:14, Rob Herring <[email protected]> wrote:
> > >
> > > On Mon, Sep 2, 2019 at 4:03 PM Krzysztof Kozlowski <[email protected]> wrote:
> > > >
> > > > Convert the Syscon reboot bindings to DT schema format using
> > > > json-schema.
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > > ---
> > > > .../bindings/power/reset/syscon-reboot.txt | 30 --------
> > > > .../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
> > > > 2 files changed, 68 insertions(+), 30 deletions(-)
> > > > delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> > > > create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > >
> > > > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > > new file mode 100644
> > > > index 000000000000..a583f3dc8ef4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > > @@ -0,0 +1,68 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Generic SYSCON mapped register reset driver
> > > > +
> > > > +maintainers:
> > > > + - Sebastian Reichel <[email protected]>
> > > > +
> > > > +description: |+
> > > > + This is a generic reset driver using syscon to map the reset register.
> > > > + The reset is generally performed with a write to the reset register
> > > > + defined by the register map pointed by syscon reference plus the offset
> > > > + with the value and mask defined in the reboot node.
> > > > + Default will be little endian mode, 32 bit access only.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: syscon-reboot
> > > > +
> > > > + mask:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: Update only the register bits defined by the mask (32 bit).
> > > > + maxItems: 1
> > >
> > > Drop this as that is already defined for uint32.
> > >
> > > It also doesn't actually work. The $ref has to be under an 'allOf' if
> > > you have additional schemas. A quirk of json-schema...
> > >
> > > > +
> > > > + offset:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: Offset in the register map for the reboot register (in bytes).
> > > > + maxItems: 1
> > > > +
> > > > + regmap:
> > > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > > + description: Phandle to the register map node.
> > > > + maxItems: 1
> > > > +
> > > > + value:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: The reset value written to the reboot register (32 bit access).
> > > > + maxItems: 1
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - regmap
> > > > + - offset
> > > > +
> > > > +allOf:
> > > > + - if:
> > > > + properties:
> > > > + value:
> > > > + not:
> > > > + type: array
> > >
> > > I think you could make this a bit more readable with:
> > >
> > > if:
> > > not:
> > > required:
> > > - value
> >
> > I do not understand how does it work (value is not mentioned in the
> > required fields so why checking of it?)... but it works fine.
>
> What's under required doesn't have to be listed as a property.
>
> > > However, if the tree is free of legacy usage, then you could just drop all this.
> >
> > One of them - mask or value - has to be provided.
>
> Or both, right?
>
> Actually, a better way to express it is probably this:
>
> oneOf:
> - required: [ value ]
> - required: [ mask ]
> - required: [ value, mask ]

This does not work mask+value would be valid everywhere:

arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: syscon-reboot:
{'regmap': [[9]], 'mask': [[1]], '$nodename': ['syscon-reboot'],
'value': [[1]], 'offset': [[1024]], 'compatible': ['syscon-reboot']}
is valid under each of {'required': ['mask']}, {'required': ['value',
'mask']}, {'required': ['value']}

2019-09-03 11:47:47

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Tue, Sep 03, 2019 at 10:00:12AM +0100, Rob Herring wrote:
> On Tue, Sep 3, 2019 at 8:47 AM Krzysztof Kozlowski <[email protected]> wrote:
> >
> > On Tue, 3 Sep 2019 at 09:14, Rob Herring <[email protected]> wrote:
> > >
> > > On Mon, Sep 2, 2019 at 4:03 PM Krzysztof Kozlowski <[email protected]> wrote:
> > > >
> > > > Convert the Syscon reboot bindings to DT schema format using
> > > > json-schema.
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > > ---
> > > > .../bindings/power/reset/syscon-reboot.txt | 30 --------
> > > > .../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
> > > > 2 files changed, 68 insertions(+), 30 deletions(-)
> > > > delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> > > > create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > >
> > > > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > > new file mode 100644
> > > > index 000000000000..a583f3dc8ef4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > > @@ -0,0 +1,68 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Generic SYSCON mapped register reset driver
> > > > +
> > > > +maintainers:
> > > > + - Sebastian Reichel <[email protected]>
> > > > +
> > > > +description: |+
> > > > + This is a generic reset driver using syscon to map the reset register.
> > > > + The reset is generally performed with a write to the reset register
> > > > + defined by the register map pointed by syscon reference plus the offset
> > > > + with the value and mask defined in the reboot node.
> > > > + Default will be little endian mode, 32 bit access only.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: syscon-reboot
> > > > +
> > > > + mask:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: Update only the register bits defined by the mask (32 bit).
> > > > + maxItems: 1
> > >
> > > Drop this as that is already defined for uint32.
> > >
> > > It also doesn't actually work. The $ref has to be under an 'allOf' if
> > > you have additional schemas. A quirk of json-schema...
> > >
> > > > +
> > > > + offset:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: Offset in the register map for the reboot register (in bytes).
> > > > + maxItems: 1
> > > > +
> > > > + regmap:
> > > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > > + description: Phandle to the register map node.
> > > > + maxItems: 1
> > > > +
> > > > + value:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + description: The reset value written to the reboot register (32 bit access).
> > > > + maxItems: 1
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - regmap
> > > > + - offset
> > > > +
> > > > +allOf:
> > > > + - if:
> > > > + properties:
> > > > + value:
> > > > + not:
> > > > + type: array
> > >
> > > I think you could make this a bit more readable with:
> > >
> > > if:
> > > not:
> > > required:
> > > - value
> >
> > I do not understand how does it work (value is not mentioned in the
> > required fields so why checking of it?)... but it works fine.
>
> What's under required doesn't have to be listed as a property.
>
> > > However, if the tree is free of legacy usage, then you could just drop all this.
> >
> > One of them - mask or value - has to be provided.
>
> Or both, right?
>
> Actually, a better way to express it is probably this:
>
> oneOf:
> - required: [ value ]
> - required: [ mask ]
> - required: [ value, mask ]

Looks good to me.

-- Sebastian


Attachments:
(No filename) (4.04 kB)
signature.asc (849.00 B)
Download all attachments

2019-09-03 13:14:20

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Tue, Sep 3, 2019 at 12:44 PM Krzysztof Kozlowski <[email protected]> wrote:
>
> On Tue, 3 Sep 2019 at 11:00, Rob Herring <[email protected]> wrote:
> >
> > On Tue, Sep 3, 2019 at 8:47 AM Krzysztof Kozlowski <[email protected]> wrote:
> > >
> > > On Tue, 3 Sep 2019 at 09:14, Rob Herring <[email protected]> wrote:
> > > >
> > > > On Mon, Sep 2, 2019 at 4:03 PM Krzysztof Kozlowski <[email protected]> wrote:
> > > > >
> > > > > Convert the Syscon reboot bindings to DT schema format using
> > > > > json-schema.
> > > > >
> > > > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > > > ---
> > > > > .../bindings/power/reset/syscon-reboot.txt | 30 --------
> > > > > .../bindings/power/reset/syscon-reboot.yaml | 68 +++++++++++++++++++
> > > > > 2 files changed, 68 insertions(+), 30 deletions(-)
> > > > > delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
> > > > > create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > >
> > > > > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..a583f3dc8ef4
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > > > @@ -0,0 +1,68 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Generic SYSCON mapped register reset driver
> > > > > +
> > > > > +maintainers:
> > > > > + - Sebastian Reichel <[email protected]>
> > > > > +
> > > > > +description: |+
> > > > > + This is a generic reset driver using syscon to map the reset register.
> > > > > + The reset is generally performed with a write to the reset register
> > > > > + defined by the register map pointed by syscon reference plus the offset
> > > > > + with the value and mask defined in the reboot node.
> > > > > + Default will be little endian mode, 32 bit access only.
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + const: syscon-reboot
> > > > > +
> > > > > + mask:
> > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > + description: Update only the register bits defined by the mask (32 bit).
> > > > > + maxItems: 1
> > > >
> > > > Drop this as that is already defined for uint32.
> > > >
> > > > It also doesn't actually work. The $ref has to be under an 'allOf' if
> > > > you have additional schemas. A quirk of json-schema...
> > > >
> > > > > +
> > > > > + offset:
> > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > + description: Offset in the register map for the reboot register (in bytes).
> > > > > + maxItems: 1
> > > > > +
> > > > > + regmap:
> > > > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > > > + description: Phandle to the register map node.
> > > > > + maxItems: 1
> > > > > +
> > > > > + value:
> > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > + description: The reset value written to the reboot register (32 bit access).
> > > > > + maxItems: 1
> > > > > +
> > > > > +required:
> > > > > + - compatible
> > > > > + - regmap
> > > > > + - offset
> > > > > +
> > > > > +allOf:
> > > > > + - if:
> > > > > + properties:
> > > > > + value:
> > > > > + not:
> > > > > + type: array
> > > >
> > > > I think you could make this a bit more readable with:
> > > >
> > > > if:
> > > > not:
> > > > required:
> > > > - value
> > >
> > > I do not understand how does it work (value is not mentioned in the
> > > required fields so why checking of it?)... but it works fine.
> >
> > What's under required doesn't have to be listed as a property.
> >
> > > > However, if the tree is free of legacy usage, then you could just drop all this.
> > >
> > > One of them - mask or value - has to be provided.
> >
> > Or both, right?
> >
> > Actually, a better way to express it is probably this:
> >
> > oneOf:
> > - required: [ value ]
> > - required: [ mask ]
> > - required: [ value, mask ]
>
> This does not work mask+value would be valid everywhere:
>
> arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: syscon-reboot:
> {'regmap': [[9]], 'mask': [[1]], '$nodename': ['syscon-reboot'],
> 'value': [[1]], 'offset': [[1024]], 'compatible': ['syscon-reboot']}
> is valid under each of {'required': ['mask']}, {'required': ['value',
> 'mask']}, {'required': ['value']}

Ahh, right. 'anyOf' is what we want:

anyOf:
- required: [ value ]
- required: [ mask ]

2019-09-03 13:31:54

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: power: syscon-reboot: Convert bindings to json-schema

On Tue, 3 Sep 2019 at 15:12, Rob Herring <[email protected]> wrote:
> > arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: syscon-reboot:
> > {'regmap': [[9]], 'mask': [[1]], '$nodename': ['syscon-reboot'],
> > 'value': [[1]], 'offset': [[1024]], 'compatible': ['syscon-reboot']}
> > is valid under each of {'required': ['mask']}, {'required': ['value',
> > 'mask']}, {'required': ['value']}
>
> Ahh, right. 'anyOf' is what we want:
>
> anyOf:
> - required: [ value ]
> - required: [ mask ]

This triggers meta-schema error:

SCHEMA Documentation/devicetree/bindings/processed-schema.yaml
Traceback (most recent call last):
File "/home/kozik/.local/lib/python3.5/site-packages/dtschema/lib.py",
line 429, in process_schema
DTValidator.check_schema(schema)
File "/home/kozik/.local/lib/python3.5/site-packages/dtschema/lib.py",
line 575, in check_schema
raise jsonschema.SchemaError.create_from(error)
jsonschema.exceptions.SchemaError: Additional properties are not
allowed ('anyOf' was unexpected)

Failed validating 'additionalProperties' in metaschema['allOf'][0]:
{'$id': 'http://devicetree.org/meta-schemas/base.yaml#',
'$schema': 'http://json-schema.org/draft-07/schema#',
'additionalProperties': False,
'allOf': [{'$ref': 'http://json-schema.org/draft-07/schema#'}],
'description': 'Metaschema for devicetree binding documentation',
'properties': {'$id': {'pattern':
'http://devicetree.org/schemas/.*\\.yaml#'},
'$schema': {'enum':
['http://devicetree.org/meta-schemas/core.yaml#',

'http://devicetree.org/meta-schemas/base.yaml#']},
'additionalProperties': {'type': 'boolean'},
'allOf': {'items': {'propertyNames': {'enum': ['$ref',
'if',
'then',
'else']}}},
'definitions': True,
'dependencies': True,
'description': True,
'else': True,
'examples': {'items': {'type': 'string'},
'type': 'array'},
'if': True,
'maintainers': {'items': {'format': 'email',
'type': 'string'},
'type': 'array'},
'oneOf': True,
'patternProperties': True,
'properties': True,
'required': True,
'select': {'allOf': [{'$ref':
'http://json-schema.org/draft-07/schema#'},
{'oneOf': [{'properties':
{'properties': True,

'required': True},
'type': 'object'},
{'type': 'boolean'}]}]},
'then': True,
'title': {'maxLength': 100},
'unevaluatedProperties': {'type': 'boolean'}},
'required': ['$id', '$schema', 'title', 'maintainers']}

Best regards,
Krzysztof