On Thu, Dec 2, 2021 at 11:55 AM Thierry Reding <thierry.reding@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxxx>
> > > > >
> > > > > Convert the NVIDIA Tegra186 (and later) BPMP I2C bindings from the
> > > > > free-form text format to json-schema.
> > > > >
> > > > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > > > > ---
> > > > > 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@xxxxxxxxx>
> > > > > + - Jon Hunter <jonathanh@xxxxxxxxxx>
> > > > > +
> > > > > +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
