diff mbox series

[v2,13/16] dt-bindings: i2c: tegra-bpmp: Convert to json-schema

Message ID 20211119143839.1950739-14-thierry.reding@gmail.com
State Changes Requested, archived
Headers show
Series dt-bindings: Convert Tegra DT bindings to json-schema | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 2 warnings, 42 lines checked
robh/dt-meta-schema success
robh/checkpatch warning total: 0 errors, 2 warnings, 42 lines checked
robh/dt-meta-schema success
robh/dtbs-check fail build log
robh/checkpatch warning total: 0 errors, 2 warnings, 42 lines checked
robh/dt-meta-schema success

Commit Message

Thierry Reding Nov. 19, 2021, 2:38 p.m. UTC 
From: Thierry Reding <treding@nvidia.com>

Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
free-form text format to json-schema.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- add missing additionalProperties: false

 .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
 .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
 2 files changed, 42 insertions(+), 42 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml

Comments

Rob Herring Nov. 23, 2021, 4:34 p.m. UTC | #1 
On Fri, 19 Nov 2021 15:38:36 +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> free-form text format to json-schema.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - add missing additionalProperties: false
> 
>  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
>  .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
>  2 files changed, 42 insertions(+), 42 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1557177


i2c: 'pmic@3c' does not match any of the regexes: 'pinctrl-[0-9]+'
	arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dt.yaml
	arch/arm64/boot/dts/nvidia/tegra186-p3509-0000+p3636-0001.dt.yaml
	arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0000.dt.yaml
	arch/arm64/boot/dts/nvidia/tegra194-p3509-0000+p3668-0001.dt.yaml

i2c: 'pmic@3c', 'temperature-sensor@4c' do not match any of the regexes: 'pinctrl-[0-9]+'
	arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dt.yaml
Rob Herring Nov. 30, 2021, 1:44 a.m. UTC | #2 
On Fri, Nov 19, 2021 at 03:38:36PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> free-form text format to json-schema.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - add missing additionalProperties: false
> 
>  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
>  .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
>  2 files changed, 42 insertions(+), 42 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
>  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> deleted file mode 100644
> index ab240e10debc..000000000000
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> +++ /dev/null
> @@ -1,42 +0,0 @@
> -NVIDIA Tegra186 BPMP I2C controller
> -
> -In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> -devices, such as the I2C controller for the power management I2C bus. Software
> -running on other CPUs must perform IPC to the BPMP in order to execute
> -transactions on that I2C bus. This binding describes an I2C bus that is
> -accessed in such a fashion.
> -
> -The BPMP I2C node must be located directly inside the main BPMP node. See
> -../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> -
> -This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> -core I2C binding.
> -
> -Required properties:
> -- compatible:
> -    Array of strings.
> -    One of:
> -    - "nvidia,tegra186-bpmp-i2c".
> -- #address-cells: Address cells for I2C device address.
> -    Single-cell integer.
> -    Must be <1>.
> -- #size-cells:
> -    Single-cell integer.
> -    Must be <0>.
> -- nvidia,bpmp-bus-id:
> -    Single-cell integer.
> -    Indicates the I2C bus number this DT node represent, as defined by the
> -    BPMP firmware.
> -
> -Example:
> -
> -bpmp {
> -	...
> -
> -	i2c {
> -		compatible = "nvidia,tegra186-bpmp-i2c";
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		nvidia,bpmp-bus-id = <5>;
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> new file mode 100644
> index 000000000000..351e12124959
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/nvidia,tegra186-bpmp-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra186 (and later) BPMP I2C controller
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Jon Hunter <jonathanh@nvidia.com>
> +
> +description: |
> +  In Tegra186 and later, the BPMP (Boot and Power Management Processor)
> +  owns certain HW devices, such as the I2C controller for the power
> +  management I2C bus. Software running on other CPUs must perform IPC to
> +  the BPMP in order to execute transactions on that I2C bus. This
> +  binding describes an I2C bus that is accessed in such a fashion.
> +
> +  The BPMP I2C node must be located directly inside the main BPMP node.
> +  See ../firmware/nvidia,tegra186-bpmp.yaml for details of the BPMP
> +  binding.
> +
> +  This node represents an I2C controller. See ../i2c/i2c.txt for details
> +  of the core I2C binding.
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra186-bpmp-i2c
> +

> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

Covered by i2c-controller.yaml. Add a reference and then use 
unevaluatedProperties.

> +
> +  nvidia,bpmp-bus-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Indicates the I2C bus number this DT node represents,
> +      as defined by the BPMP firmware.
> +
> +additionalProperties: false
> -- 
> 2.33.1
> 
>
Thierry Reding Dec. 1, 2021, 5:42 p.m. UTC | #3 
On Mon, Nov 29, 2021 at 07:44:32PM -0600, Rob Herring wrote:
> On Fri, Nov 19, 2021 at 03:38:36PM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> > free-form text format to json-schema.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v2:
> > - add missing additionalProperties: false
> > 
> >  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
> >  .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
> >  2 files changed, 42 insertions(+), 42 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> >  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > deleted file mode 100644
> > index ab240e10debc..000000000000
> > --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > +++ /dev/null
> > @@ -1,42 +0,0 @@
> > -NVIDIA Tegra186 BPMP I2C controller
> > -
> > -In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> > -devices, such as the I2C controller for the power management I2C bus. Software
> > -running on other CPUs must perform IPC to the BPMP in order to execute
> > -transactions on that I2C bus. This binding describes an I2C bus that is
> > -accessed in such a fashion.
> > -
> > -The BPMP I2C node must be located directly inside the main BPMP node. See
> > -../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> > -
> > -This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> > -core I2C binding.
> > -
> > -Required properties:
> > -- compatible:
> > -    Array of strings.
> > -    One of:
> > -    - "nvidia,tegra186-bpmp-i2c".
> > -- #address-cells: Address cells for I2C device address.
> > -    Single-cell integer.
> > -    Must be <1>.
> > -- #size-cells:
> > -    Single-cell integer.
> > -    Must be <0>.
> > -- nvidia,bpmp-bus-id:
> > -    Single-cell integer.
> > -    Indicates the I2C bus number this DT node represent, as defined by the
> > -    BPMP firmware.
> > -
> > -Example:
> > -
> > -bpmp {
> > -	...
> > -
> > -	i2c {
> > -		compatible = "nvidia,tegra186-bpmp-i2c";
> > -		#address-cells = <1>;
> > -		#size-cells = <0>;
> > -		nvidia,bpmp-bus-id = <5>;
> > -	};
> > -};
> > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > new file mode 100644
> > index 000000000000..351e12124959
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > @@ -0,0 +1,42 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i2c/nvidia,tegra186-bpmp-i2c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra186 (and later) BPMP I2C controller
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Jon Hunter <jonathanh@nvidia.com>
> > +
> > +description: |
> > +  In Tegra186 and later, the BPMP (Boot and Power Management Processor)
> > +  owns certain HW devices, such as the I2C controller for the power
> > +  management I2C bus. Software running on other CPUs must perform IPC to
> > +  the BPMP in order to execute transactions on that I2C bus. This
> > +  binding describes an I2C bus that is accessed in such a fashion.
> > +
> > +  The BPMP I2C node must be located directly inside the main BPMP node.
> > +  See ../firmware/nvidia,tegra186-bpmp.yaml for details of the BPMP
> > +  binding.
> > +
> > +  This node represents an I2C controller. See ../i2c/i2c.txt for details
> > +  of the core I2C binding.
> > +
> > +properties:
> > +  compatible:
> > +    const: nvidia,tegra186-bpmp-i2c
> > +
> 
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> 
> Covered by i2c-controller.yaml. Add a reference and then use 
> unevaluatedProperties.

About that: I've recently noticed that this doesn't seem to work
properly. I'm using branch draft2020-12 from your github and my
understanding was that this should give us support for
unevaluatedProperties. And indeed, it no longer complains about
#address-cells and #size-cells if I remove them from this binding,
presumably because it gets them from i2c-controller.yaml.

However, a side-effect seems to be that now it also ignores any
properties that aren't defined anywhere. So for example if I touch
up the example in firmware/nvidia,tegra186-bpmp.yaml and add a bogus
"foo-bar = <0>;" property in the BPMP I2C node, then it'll blindly
accept that as valid.

The validation will flag if I set #address-cells = <2> in the BPMP
I2C node, so validation of the schema still seems to work, but for
some reason it won't flag any properties that haven't been specified
in the schema.

Do I misunderstand how this is supposed to work, or is there something
wrong with the current implementation of unevaluatedProperties?

Thierry
Rob Herring Dec. 1, 2021, 6:42 p.m. UTC | #4 
On Wed, Dec 1, 2021 at 11:42 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 07:44:32PM -0600, Rob Herring wrote:
> > On Fri, Nov 19, 2021 at 03:38:36PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> > > free-form text format to json-schema.
> > >
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > > Changes in v2:
> > > - add missing additionalProperties: false
> > >
> > >  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
> > >  .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
> > >  2 files changed, 42 insertions(+), 42 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > >  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > deleted file mode 100644
> > > index ab240e10debc..000000000000
> > > --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > +++ /dev/null
> > > @@ -1,42 +0,0 @@
> > > -NVIDIA Tegra186 BPMP I2C controller
> > > -
> > > -In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> > > -devices, such as the I2C controller for the power management I2C bus. Software
> > > -running on other CPUs must perform IPC to the BPMP in order to execute
> > > -transactions on that I2C bus. This binding describes an I2C bus that is
> > > -accessed in such a fashion.
> > > -
> > > -The BPMP I2C node must be located directly inside the main BPMP node. See
> > > -../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> > > -
> > > -This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> > > -core I2C binding.
> > > -
> > > -Required properties:
> > > -- compatible:
> > > -    Array of strings.
> > > -    One of:
> > > -    - "nvidia,tegra186-bpmp-i2c".
> > > -- #address-cells: Address cells for I2C device address.
> > > -    Single-cell integer.
> > > -    Must be <1>.
> > > -- #size-cells:
> > > -    Single-cell integer.
> > > -    Must be <0>.
> > > -- nvidia,bpmp-bus-id:
> > > -    Single-cell integer.
> > > -    Indicates the I2C bus number this DT node represent, as defined by the
> > > -    BPMP firmware.
> > > -
> > > -Example:
> > > -
> > > -bpmp {
> > > -   ...
> > > -
> > > -   i2c {
> > > -           compatible = "nvidia,tegra186-bpmp-i2c";
> > > -           #address-cells = <1>;
> > > -           #size-cells = <0>;
> > > -           nvidia,bpmp-bus-id = <5>;
> > > -   };
> > > -};
> > > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > new file mode 100644
> > > index 000000000000..351e12124959
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > @@ -0,0 +1,42 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/i2c/nvidia,tegra186-bpmp-i2c.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NVIDIA Tegra186 (and later) BPMP I2C controller
> > > +
> > > +maintainers:
> > > +  - Thierry Reding <thierry.reding@gmail.com>
> > > +  - Jon Hunter <jonathanh@nvidia.com>
> > > +
> > > +description: |
> > > +  In Tegra186 and later, the BPMP (Boot and Power Management Processor)
> > > +  owns certain HW devices, such as the I2C controller for the power
> > > +  management I2C bus. Software running on other CPUs must perform IPC to
> > > +  the BPMP in order to execute transactions on that I2C bus. This
> > > +  binding describes an I2C bus that is accessed in such a fashion.
> > > +
> > > +  The BPMP I2C node must be located directly inside the main BPMP node.
> > > +  See ../firmware/nvidia,tegra186-bpmp.yaml for details of the BPMP
> > > +  binding.
> > > +
> > > +  This node represents an I2C controller. See ../i2c/i2c.txt for details
> > > +  of the core I2C binding.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: nvidia,tegra186-bpmp-i2c
> > > +
> >
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> >
> > Covered by i2c-controller.yaml. Add a reference and then use
> > unevaluatedProperties.
>
> About that: I've recently noticed that this doesn't seem to work
> properly. I'm using branch draft2020-12 from your github and my

Use dtschema main/master branch. That branch is likely stale.

> understanding was that this should give us support for
> unevaluatedProperties. And indeed, it no longer complains about
> #address-cells and #size-cells if I remove them from this binding,
> presumably because it gets them from i2c-controller.yaml.
>
> However, a side-effect seems to be that now it also ignores any
> properties that aren't defined anywhere. So for example if I touch
> up the example in firmware/nvidia,tegra186-bpmp.yaml and add a bogus
> "foo-bar = <0>;" property in the BPMP I2C node, then it'll blindly
> accept that as valid.

Do you have unevaluatedProperties within the i2c node? It only applies
to 1 level, and you can't have a parent+child schema evaluated with
another child (or parent+child) schema. This is why the graph schema
is done the way it is and why we're splitting spi-controller.yaml
child node schema out to spi-peripheral.yaml.

> The validation will flag if I set #address-cells = <2> in the BPMP
> I2C node, so validation of the schema still seems to work, but for
> some reason it won't flag any properties that haven't been specified
> in the schema.
>
> Do I misunderstand how this is supposed to work, or is there something
> wrong with the current implementation of unevaluatedProperties?
>
> Thierry
Thierry Reding Dec. 2, 2021, 5:55 p.m. UTC | #5 
On Wed, Dec 01, 2021 at 12:42:07PM -0600, Rob Herring wrote:
> On Wed, Dec 1, 2021 at 11:42 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Mon, Nov 29, 2021 at 07:44:32PM -0600, Rob Herring wrote:
> > > On Fri, Nov 19, 2021 at 03:38:36PM +0100, Thierry Reding wrote:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > >
> > > > Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> > > > free-form text format to json-schema.
> > > >
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > > Changes in v2:
> > > > - add missing additionalProperties: false
> > > >
> > > >  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
> > > >  .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
> > > >  2 files changed, 42 insertions(+), 42 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > > deleted file mode 100644
> > > > index ab240e10debc..000000000000
> > > > --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > > +++ /dev/null
> > > > @@ -1,42 +0,0 @@
> > > > -NVIDIA Tegra186 BPMP I2C controller
> > > > -
> > > > -In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> > > > -devices, such as the I2C controller for the power management I2C bus. Software
> > > > -running on other CPUs must perform IPC to the BPMP in order to execute
> > > > -transactions on that I2C bus. This binding describes an I2C bus that is
> > > > -accessed in such a fashion.
> > > > -
> > > > -The BPMP I2C node must be located directly inside the main BPMP node. See
> > > > -../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> > > > -
> > > > -This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> > > > -core I2C binding.
> > > > -
> > > > -Required properties:
> > > > -- compatible:
> > > > -    Array of strings.
> > > > -    One of:
> > > > -    - "nvidia,tegra186-bpmp-i2c".
> > > > -- #address-cells: Address cells for I2C device address.
> > > > -    Single-cell integer.
> > > > -    Must be <1>.
> > > > -- #size-cells:
> > > > -    Single-cell integer.
> > > > -    Must be <0>.
> > > > -- nvidia,bpmp-bus-id:
> > > > -    Single-cell integer.
> > > > -    Indicates the I2C bus number this DT node represent, as defined by the
> > > > -    BPMP firmware.
> > > > -
> > > > -Example:
> > > > -
> > > > -bpmp {
> > > > -   ...
> > > > -
> > > > -   i2c {
> > > > -           compatible = "nvidia,tegra186-bpmp-i2c";
> > > > -           #address-cells = <1>;
> > > > -           #size-cells = <0>;
> > > > -           nvidia,bpmp-bus-id = <5>;
> > > > -   };
> > > > -};
> > > > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > > new file mode 100644
> > > > index 000000000000..351e12124959
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > > @@ -0,0 +1,42 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/i2c/nvidia,tegra186-bpmp-i2c.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: NVIDIA Tegra186 (and later) BPMP I2C controller
> > > > +
> > > > +maintainers:
> > > > +  - Thierry Reding <thierry.reding@gmail.com>
> > > > +  - Jon Hunter <jonathanh@nvidia.com>
> > > > +
> > > > +description: |
> > > > +  In Tegra186 and later, the BPMP (Boot and Power Management Processor)
> > > > +  owns certain HW devices, such as the I2C controller for the power
> > > > +  management I2C bus. Software running on other CPUs must perform IPC to
> > > > +  the BPMP in order to execute transactions on that I2C bus. This
> > > > +  binding describes an I2C bus that is accessed in such a fashion.
> > > > +
> > > > +  The BPMP I2C node must be located directly inside the main BPMP node.
> > > > +  See ../firmware/nvidia,tegra186-bpmp.yaml for details of the BPMP
> > > > +  binding.
> > > > +
> > > > +  This node represents an I2C controller. See ../i2c/i2c.txt for details
> > > > +  of the core I2C binding.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: nvidia,tegra186-bpmp-i2c
> > > > +
> > >
> > > > +  "#address-cells":
> > > > +    const: 1
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 0
> > >
> > > Covered by i2c-controller.yaml. Add a reference and then use
> > > unevaluatedProperties.
> >
> > About that: I've recently noticed that this doesn't seem to work
> > properly. I'm using branch draft2020-12 from your github and my
> 
> Use dtschema main/master branch. That branch is likely stale.

That seems to have helped somewhat. I do occasionally see warnings now
about unevaluated properties being unexpected. I can still reproduce the
issue, though, see below.

> > understanding was that this should give us support for
> > unevaluatedProperties. And indeed, it no longer complains about
> > #address-cells and #size-cells if I remove them from this binding,
> > presumably because it gets them from i2c-controller.yaml.
> >
> > However, a side-effect seems to be that now it also ignores any
> > properties that aren't defined anywhere. So for example if I touch
> > up the example in firmware/nvidia,tegra186-bpmp.yaml and add a bogus
> > "foo-bar = <0>;" property in the BPMP I2C node, then it'll blindly
> > accept that as valid.
> 
> Do you have unevaluatedProperties within the i2c node? It only applies
> to 1 level, and you can't have a parent+child schema evaluated with
> another child (or parent+child) schema. This is why the graph schema
> is done the way it is and why we're splitting spi-controller.yaml
> child node schema out to spi-peripheral.yaml.

Let me give an example based on a schema that's already upstream. So
looking at this:

	Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml

it does include spi-controller.yaml via an allOf: [ $ref: ... ], so it
uses unevaluatedProperties to validate against any generic SPI
controller properties. For example, #address-cells and #size-cells are
validated based on the schema from spi-controller.yaml.

However, if I now apply the following patch to add an undocumented
property to the example, then validation doesn't fail as I would expect
it to.

--- >8 ---
diff --git a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
index 35a8045b2c70..e9342faf5194 100644
--- a/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
+++ b/Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
@@ -104,6 +104,7 @@ examples:
             resets = <&tegra_car 211>;
             dmas = <&apbdma 5>, <&apbdma 5>;
             dma-names = "rx", "tx";
+            foo-something = <42>;
 
             flash@0 {
                     compatible = "spi-nor";
--- >8 ---

I would expect the validation to fail for foo-something because it isn't
defined in any of the schemas.

Or is this one of the cases that you mentioned above. I must admit I did
not follow what exactly is expected to work and what isn't. The QSPI
controller example from above seems simple enough.

Thierry
Rob Herring Dec. 2, 2021, 9:08 p.m. UTC | #6 
On Thu, Dec 2, 2021 at 11:55 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Wed, Dec 01, 2021 at 12:42:07PM -0600, Rob Herring wrote:
> > On Wed, Dec 1, 2021 at 11:42 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> > >
> > > On Mon, Nov 29, 2021 at 07:44:32PM -0600, Rob Herring wrote:
> > > > On Fri, Nov 19, 2021 at 03:38:36PM +0100, Thierry Reding wrote:
> > > > > From: Thierry Reding <treding@nvidia.com>
> > > > >
> > > > > Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> > > > > free-form text format to json-schema.
> > > > >
> > > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > > ---
> > > > > Changes in v2:
> > > > > - add missing additionalProperties: false
> > > > >
> > > > >  .../bindings/i2c/nvidia,tegra186-bpmp-i2c.txt | 42 -------------------
> > > > >  .../i2c/nvidia,tegra186-bpmp-i2c.yaml         | 42 +++++++++++++++++++
> > > > >  2 files changed, 42 insertions(+), 42 deletions(-)
> > > > >  delete mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > > >  create mode 100644 Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > > > deleted file mode 100644
> > > > > index ab240e10debc..000000000000
> > > > > --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
> > > > > +++ /dev/null
> > > > > @@ -1,42 +0,0 @@
> > > > > -NVIDIA Tegra186 BPMP I2C controller
> > > > > -
> > > > > -In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
> > > > > -devices, such as the I2C controller for the power management I2C bus. Software
> > > > > -running on other CPUs must perform IPC to the BPMP in order to execute
> > > > > -transactions on that I2C bus. This binding describes an I2C bus that is
> > > > > -accessed in such a fashion.
> > > > > -
> > > > > -The BPMP I2C node must be located directly inside the main BPMP node. See
> > > > > -../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
> > > > > -
> > > > > -This node represents an I2C controller. See ../i2c/i2c.txt for details of the
> > > > > -core I2C binding.
> > > > > -
> > > > > -Required properties:
> > > > > -- compatible:
> > > > > -    Array of strings.
> > > > > -    One of:
> > > > > -    - "nvidia,tegra186-bpmp-i2c".
> > > > > -- #address-cells: Address cells for I2C device address.
> > > > > -    Single-cell integer.
> > > > > -    Must be <1>.
> > > > > -- #size-cells:
> > > > > -    Single-cell integer.
> > > > > -    Must be <0>.
> > > > > -- nvidia,bpmp-bus-id:
> > > > > -    Single-cell integer.
> > > > > -    Indicates the I2C bus number this DT node represent, as defined by the
> > > > > -    BPMP firmware.
> > > > > -
> > > > > -Example:
> > > > > -
> > > > > -bpmp {
> > > > > -   ...
> > > > > -
> > > > > -   i2c {
> > > > > -           compatible = "nvidia,tegra186-bpmp-i2c";
> > > > > -           #address-cells = <1>;
> > > > > -           #size-cells = <0>;
> > > > > -           nvidia,bpmp-bus-id = <5>;
> > > > > -   };
> > > > > -};
> > > > > diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..351e12124959
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
> > > > > @@ -0,0 +1,42 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/i2c/nvidia,tegra186-bpmp-i2c.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: NVIDIA Tegra186 (and later) BPMP I2C controller
> > > > > +
> > > > > +maintainers:
> > > > > +  - Thierry Reding <thierry.reding@gmail.com>
> > > > > +  - Jon Hunter <jonathanh@nvidia.com>
> > > > > +
> > > > > +description: |
> > > > > +  In Tegra186 and later, the BPMP (Boot and Power Management Processor)
> > > > > +  owns certain HW devices, such as the I2C controller for the power
> > > > > +  management I2C bus. Software running on other CPUs must perform IPC to
> > > > > +  the BPMP in order to execute transactions on that I2C bus. This
> > > > > +  binding describes an I2C bus that is accessed in such a fashion.
> > > > > +
> > > > > +  The BPMP I2C node must be located directly inside the main BPMP node.
> > > > > +  See ../firmware/nvidia,tegra186-bpmp.yaml for details of the BPMP
> > > > > +  binding.
> > > > > +
> > > > > +  This node represents an I2C controller. See ../i2c/i2c.txt for details
> > > > > +  of the core I2C binding.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: nvidia,tegra186-bpmp-i2c
> > > > > +
> > > >
> > > > > +  "#address-cells":
> > > > > +    const: 1
> > > > > +
> > > > > +  "#size-cells":
> > > > > +    const: 0
> > > >
> > > > Covered by i2c-controller.yaml. Add a reference and then use
> > > > unevaluatedProperties.
> > >
> > > About that: I've recently noticed that this doesn't seem to work
> > > properly. I'm using branch draft2020-12 from your github and my
> >
> > Use dtschema main/master branch. That branch is likely stale.
>
> That seems to have helped somewhat. I do occasionally see warnings now
> about unevaluated properties being unexpected. I can still reproduce the
> issue, though, see below.
>
> > > understanding was that this should give us support for
> > > unevaluatedProperties. And indeed, it no longer complains about
> > > #address-cells and #size-cells if I remove them from this binding,
> > > presumably because it gets them from i2c-controller.yaml.
> > >
> > > However, a side-effect seems to be that now it also ignores any
> > > properties that aren't defined anywhere. So for example if I touch
> > > up the example in firmware/nvidia,tegra186-bpmp.yaml and add a bogus
> > > "foo-bar = <0>;" property in the BPMP I2C node, then it'll blindly
> > > accept that as valid.
> >
> > Do you have unevaluatedProperties within the i2c node? It only applies
> > to 1 level, and you can't have a parent+child schema evaluated with
> > another child (or parent+child) schema. This is why the graph schema
> > is done the way it is and why we're splitting spi-controller.yaml
> > child node schema out to spi-peripheral.yaml.
>
> Let me give an example based on a schema that's already upstream. So
> looking at this:
>
>         Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.yaml
>
> it does include spi-controller.yaml via an allOf: [ $ref: ... ], so it
> uses unevaluatedProperties to validate against any generic SPI
> controller properties. For example, #address-cells and #size-cells are
> validated based on the schema from spi-controller.yaml.
>
> However, if I now apply the following patch to add an undocumented
> property to the example, then validation doesn't fail as I would expect
> it to.

Indeed you are right. The problem is 'additionalProperties: true' in
spi-controller.yaml makes everything evaluated. I thought
'additionalProperties: true' was equivalent to the default, but that's
not how it's working. Now to figure out if this is correct operation
or not. No wonder there were relatively few fixes when
'unevaluatedProperties' got implemented...

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
deleted file mode 100644
index ab240e10debc..000000000000
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.txt
+++ /dev/null
@@ -1,42 +0,0 @@ 
-NVIDIA Tegra186 BPMP I2C controller
-
-In Tegra186, the BPMP (Boot and Power Management Processor) owns certain HW
-devices, such as the I2C controller for the power management I2C bus. Software
-running on other CPUs must perform IPC to the BPMP in order to execute
-transactions on that I2C bus. This binding describes an I2C bus that is
-accessed in such a fashion.
-
-The BPMP I2C node must be located directly inside the main BPMP node. See
-../firmware/nvidia,tegra186-bpmp.txt for details of the BPMP binding.
-
-This node represents an I2C controller. See ../i2c/i2c.txt for details of the
-core I2C binding.
-
-Required properties:
-- compatible:
-    Array of strings.
-    One of:
-    - "nvidia,tegra186-bpmp-i2c".
-- #address-cells: Address cells for I2C device address.
-    Single-cell integer.
-    Must be <1>.
-- #size-cells:
-    Single-cell integer.
-    Must be <0>.
-- nvidia,bpmp-bus-id:
-    Single-cell integer.
-    Indicates the I2C bus number this DT node represent, as defined by the
-    BPMP firmware.
-
-Example:
-
-bpmp {
-	...
-
-	i2c {
-		compatible = "nvidia,tegra186-bpmp-i2c";
-		#address-cells = <1>;
-		#size-cells = <0>;
-		nvidia,bpmp-bus-id = <5>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
new file mode 100644
index 000000000000..351e12124959
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra186-bpmp-i2c.yaml
@@ -0,0 +1,42 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/nvidia,tegra186-bpmp-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra186 (and later) BPMP I2C controller
+
+maintainers:
+  - Thierry Reding <thierry.reding@gmail.com>
+  - Jon Hunter <jonathanh@nvidia.com>
+
+description: |
+  In Tegra186 and later, the BPMP (Boot and Power Management Processor)
+  owns certain HW devices, such as the I2C controller for the power
+  management I2C bus. Software running on other CPUs must perform IPC to
+  the BPMP in order to execute transactions on that I2C bus. This
+  binding describes an I2C bus that is accessed in such a fashion.
+
+  The BPMP I2C node must be located directly inside the main BPMP node.
+  See ../firmware/nvidia,tegra186-bpmp.yaml for details of the BPMP
+  binding.
+
+  This node represents an I2C controller. See ../i2c/i2c.txt for details
+  of the core I2C binding.
+
+properties:
+  compatible:
+    const: nvidia,tegra186-bpmp-i2c
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  nvidia,bpmp-bus-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Indicates the I2C bus number this DT node represents,
+      as defined by the BPMP firmware.
+
+additionalProperties: false