From: Thierry Reding <thierry.reding@gmail.com>
To: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH 07/38] dt-bindings: display: tegra: Convert to json-schema
Date: Thu, 18 Jun 2020 16:16:30 +0200 [thread overview]
Message-ID: <20200618141630.GB3663225@ulmo> (raw)
In-Reply-To: <20200617231326.GD2975260@bogus>
[-- Attachment #1: Type: text/plain, Size: 33709 bytes --]
On Wed, Jun 17, 2020 at 05:13:26PM -0600, Rob Herring wrote:
> On Fri, Jun 12, 2020 at 04:18:32PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Convert the Tegra host1x controller bindings from the free-form text
> > format to json-schema.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > .../display/tegra/nvidia,tegra20-host1x.txt | 516 ------
> > .../display/tegra/nvidia,tegra20-host1x.yaml | 1418 +++++++++++++++++
> > 2 files changed, 1418 insertions(+), 516 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> > create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>
>
> > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > new file mode 100644
> > index 000000000000..3347e1b3c8f0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> > @@ -0,0 +1,1418 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-host1x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra host1x controller
> > +
> > +maintainers:
> > + - Thierry Reding <thierry.reding@gmail.com>
> > + - Jon Hunter <jonathanh@nvidia.com>
> > +
> > +description: The host1x top-level node defines a number of children, each
> > + representing one of the host1x client modules defined in this binding.
> > +
> > +properties:
> > + # required
> > + compatible:
> > + oneOf:
> > + - description: NVIDIA Tegra20
> > + items:
> > + - const: nvidia,tegra20-host1x
> > +
> > + - description: NVIDIA Tegra30
> > + items:
> > + - const: nvidia,tegra30-host1x
> > +
> > + - description: NVIDIA Tegra114
> > + items:
> > + - const: nvidia,tegra114-host1x
> > +
> > + - description: NVIDIA Tegra124
> > + items:
> > + - const: nvidia,tegra124-host1x
> > +
> > + - description: NVIDIA Tegra132
> > + items:
> > + - const: nvidia,tegra132-host1x
> > + - const: nvidia,tegra124-host1x
> > +
> > + - description: NVIDIA Tegra210
> > + items:
> > + - const: nvidia,tegra210-host1x
> > +
> > + - description: NVIDIA Tegra186
> > + items:
> > + - const: nvidia,tegra186-host1x
> > +
> > + - description: NVIDIA Tegra194
> > + items:
> > + - const: nvidia,tegra194-host1x
>
> I don't think the descriptions really add much. I'd rather see all the
> single entry cases as 1 'enum'.
Okay, done.
> > +
> > + interrupts:
> > + items:
> > + - description: host1x syncpoint interrupt
> > + - description: host1x general interrupt
> > + minItems: 1
> > + maxItems: 2
> > +
> > + interrupt-names:
> > + items:
> > + - const: syncpt
> > + - const: host1x
> > +
> > + '#address-cells':
> > + description: The number of cells used to represent physical base addresses
> > + in the host1x address space.
> > + enum: [1, 2]
> > +
> > + '#size-cells':
> > + description: The number of cells used to represent the size of an address
> > + range in the host1x address space.
> > + enum: [1, 2]
> > +
> > + # required
>
> Odd comment...
There were a couple of others like this. I've removed them all now.
>
> > + ranges:
> > + description: The mapping of the host1x address space to the CPU address
> > + space.
>
> That's every 'ranges'. If you know how many entries, then define
> 'maxItems'. If not, 'ranges: true' is enough.
Done.
> > +
> > + clocks:
> > + description: Must contain one entry, for the module clock. See
> > + ../clocks/clock-bindings.txt for details.
> > +
> > + clock-names:
> > + items:
> > + - const: host1x
> > +
> > + resets:
> > + description: Must contain an entry for each entry in reset-names. See
> > + ../reset/reset.txt for details.
> > +
> > + reset-names:
> > + items:
> > + - const: host1x
> > +
> > + # optional
> > + iommus:
> > + $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
>
> This is already applied to every 'iommus' property, so you just need to
> define how many entries.
Done.
> > +
> > + memory-controllers:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > +
> > +required:
> > + - compatible
> > + - interrupts
> > + - interrupt-names
> > + - '#address-cells'
> > + - '#size-cells'
> > + - ranges
> > + - reg
> > + - clocks
> > + - clock-names
> > + - resets
> > + - reset-names
> > +
> > +unevaluatedProperties: false
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra186-host1x
> > + - nvidia,tegra194-host1x
> > + then:
> > + properties:
> > + reg-names:
> > + items:
> > + - const: hypervisor
> > + - const: vm
> > +
> > + reg:
> > + items:
> > + - description: physical base address and length of the register
> > + region assigned to the VM
> > + - description: physical base address and length of the register
> > + region used by the hypervisor
> > +
> > + required:
> > + - reg-names
> > + else:
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + oneOf:
> > + - const: nvidia,tegra20-host1x
> > + - const: nvidia,tegra30-host1x
> > + - const: nvidia,tegra114-host1x
> > + - const: nvidia,tegra124-host1x
>
> Use 'enum'.
Done.
> > +
> > + then:
> > + patternProperties:
> > + "^vi@[0-9a-f]+$":
> > + description: video input
> > + type: object
> > + properties:
> > + compatible:
> > + oneOf:
> > + - const: nvidia,tegra20-vi
> > + - const: nvidia,tegra30-vi
> > + - const: nvidia,tegra114-vi
> > + - const: nvidia,tegra124-vi
>
> Use a 'enum' for these 4.
Done.
>
> > + - items:
> > + - const: nvidia,tegra132-vi
> > + - const: nvidia,tegra124-vi
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + resets: true
> > +
> > + reset-names:
> > + items:
> > + - const: vi
> > + else:
> > + patternProperties:
> > + "^vi@[0-9a-f]+$":
> > + description: video input
> > + type: object
> > + properties:
> > + compatible:
> > + oneOf:
> > + - const: nvidia,tegra210-vi
> > + - const: nvidia,tegra186-vi
> > + - const: nvidia,tegra194-vi
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + power-domains: true
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + oneOf:
> > + - const: nvidia,tegra210-host1x
> > + then:
> > + patternProperties:
> > + "^vi@[0-9a-f]+$":
>
> type: object
Done.
>
> > + patternProperties:
> > + "^csi@[0-9a-f]+$":
> > + description: camera sensor interface
> > + type: object
> > + properties:
> > + compatible:
> > + enum:
> > + - nvidia,tegra210-csi
> > +
> > + reg: true
> > +
> > + clocks: true
> > +
> > + clock-names:
> > + items:
> > + - const: csi
> > + - const: cilab
> > + - const: cilcd
> > + - const: cile
> > + - const: csi_tpg
> > +
> > + power-domains: true
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra20-host1x
> > + - nvidia,tegra30-host1x
> > + - nvidia,tegra114-host1x
> > + then:
> > + patternProperties:
> > + "^epp@[0-9a-f]+$":
> > + description: encoder pre-processor
> > + type: object
> > + properties:
> > + compatible:
> > + enum:
> > + - nvidia,tegra20-epp
> > + - nvidia,tegra30-epp
> > + - nvidia,tegra114-epp
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: epp
> > +
> > + iommus:
> > + $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
> > +
> > + unevaluatedProperties: false
>
> I think 'additionalProperties: false' will work here.
Yes, it does.
>
> > +
> > + "^mpe@[0-9a-f]+$":
> > + description: video encoder
> > + type: object
> > + properties:
> > + compatible:
> > + enum:
> > + - nvidia,tegra20-mpe
> > + - nvidia,tegra30-mpe
> > + - nvidia,tegra114-mpe
> > +
> > + reg:
> > + minItems: 1
> > + maxItems: 1
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 1
> > + maxItems: 1
> > +
> > + resets:
> > + minItems: 1
> > + maxItems: 1
> > +
> > + reset-names:
> > + items:
> > + - const: mpe
> > +
> > + iommus:
> > + $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
> > +
> > + unevaluatedProperties: false
> > +
> > + "^gr2d@[0-9a-f]+$":
> > + description: 2D graphics engine
> > + type: object
> > + properties:
> > + compatible:
> > + enum:
> > + - nvidia,tegra20-gr2d
> > + - nvidia,tegra30-gr2d
> > + - nvidia,tegra114-gr2d
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: 2d
> > +
> > + iommus:
> > + $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
> > +
> > + unevaluatedProperties: false
> > +
> > + "^gr3d@[0-9a-f]+$":
> > + description: 3D graphics engine
> > + type: object
> > + properties:
> > + compatible:
> > + enum:
> > + - nvidia,tegra20-gr3d
> > + - nvidia,tegra30-gr3d
> > + - nvidia,tegra114-gr3d
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + iommus:
> > + $ref: "/schemas/iommu/iommu.yaml#/properties/iommus"
> > +
> > + allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: nvidia,tegra30-gr3d
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: primary module clock
> > + - description: secondary module clock
> > +
> > + clock-names:
> > + items:
> > + - const: 3d
> > + - const: 3d2
> > +
> > + resets:
> > + items:
> > + - description: primary module reset
> > + - description: secondary module reset
> > +
> > + reset-names:
> > + items:
> > + - const: 3d
> > + - const: 3d2
> > + else:
> > + properties:
> > + clocks:
> > + items:
> > + - description: module clock
> > +
> > + clock-names:
> > + items:
> > + - const: 3d
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: 3d
> > +
> > + unevaluatedProperties: false
> > +
> > + "^tvo@[0-9a-f]+$":
> > + description: TV encoder output
> > + type: object
> > + properties:
> > + # required
> > + compatible:
> > + enum:
> > + - nvidia,tegra20-tvo
> > + - nvidia,tegra30-tvo
> > + - nvidia,tegra114-tvo
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + status:
> > + $ref: "/schemas/dt-core.yaml#/properties/status"
> > +
> > + unevaluatedProperties: false
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra20-host1x
> > + - nvidia,tegra30-host1x
> > + - nvidia,tegra114-host1x
> > + - nvidia,tegra124-host1x
> > + - nvidia,tegra132-host1x
> > + - nvidia,tegra210-host1x
> > + then:
> > + patternProperties:
> > + "^dc@[0-9a-f]+$":
> > + description: display controller
> > + type: object
> > + properties:
> > + compatible:
> > + oneOf:
> > + - const: nvidia,tegra20-dc
> > + - const: nvidia,tegra30-dc
> > + - const: nvidia,tegra114-dc
> > + - const: nvidia,tegra124-dc
> > + - items:
> > + - const: nvidia,tegra124-dc
> > + - const: nvidia,tegra132-dc
> > + - const: nvidia,tegra210-dc
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: display controller pixel clock
> > +
> > + clock-names:
> > + items:
> > + - const: dc
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: dc
> > +
> > + iommus:
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + nvidia,head:
> > + description: The number of the display controller head. This is
> > + used to setup the various types of output to receive video
> > + data from the given head.
> > + $ref: "/schemas/types.yaml#/definitions/uint32"
> > +
> > + allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra20-dc
> > + - nvidia,tegra30-dc
> > + - nvidia,tegra114-dc
> > + then:
> > + properties:
> > + rgb:
> > + description: Each display controller node has a child node,
> > + named "rgb", that represents the RGB output associated
> > + with the controller.
> > + type: object
> > + properties:
> > + nvidia,ddc-i2c-bus:
> > + description: phandle of an I2C controller used for DDC
> > + EDID probing
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + nvidia,hpd-gpio:
> > + description: specifies a GPIO used for hotplug
> > + detection
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + nvidia,edid:
> > + description: supplies a binary EDID blob
> > + $ref: "/schemas/types.yaml#/definitions/uint8-array"
> > +
> > + nvidia,panel:
> > + description: phandle of a display panel
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + unevaluatedProperties: false
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra186-host1x
> > + - nvidia,tegra194-host1x
> > + then:
> > + patternProperties:
> > + "^display-hub@[0-9a-f]+$":
> > + properties:
> > + compatible:
> > + oneOf:
> > + - description: NVIDIA Tegra186
> > + const: nvidia,tegra186-display
> > +
> > + - description: NVIDIA Tegra194
> > + const: nvidia,tegra194-display
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 1
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + ranges:
> > + $ref: "/schemas/dt-core.yaml#/properties/ranges"
> > +
> > + status:
> > + $ref: "/schemas/dt-core.yaml#/properties/status"
> > +
> > + resets:
> > + items:
> > + - description: display hub reset
> > + - description: window group 0 reset
> > + - description: window group 1 reset
> > + - description: window group 2 reset
> > + - description: window group 3 reset
> > + - description: window group 4 reset
> > + - description: window group 5 reset
> > +
> > + reset-names:
> > + items:
> > + - const: misc
> > + - const: wgrp0
> > + - const: wgrp1
> > + - const: wgrp2
> > + - const: wgrp3
> > + - const: wgrp4
> > + - const: wgrp5
> > +
> > + power-domains:
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + patternProperties:
> > + "^display@[0-9a-f]+$":
> > + description: display controller
> > + type: object
> > + properties:
> > + compatible:
> > + enum:
> > + - nvidia,tegra186-dc
> > + - nvidia,tegra194-dc
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: display controller pixel clock
> > +
> > + clock-names:
> > + items:
> > + - const: dc
> > +
> > + resets:
> > + items:
> > + - description: display controller reset
> > +
> > + reset-names:
> > + items:
> > + - const: dc
> > +
> > + power-domains:
> > + description: A list of phandle and specifiers that identify
> > + the power domains that this display controller is part of.
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + iommus:
> > + description: a phandle and specifier identifying the SMMU
> > + master interface of this display controller.
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + memory-controllers:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > +
> > + nvidia,outputs:
> > + description: A list of phandles of outputs that this display
> > + controller can drive.
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + nvidia,head:
> > + description: The number of the display controller head. This
> > + is used to setup the various types of output to receive
> > + video data from the given head.
> > + $ref: "/schemas/types.yaml#/definitions/uint32"
> > +
> > + unevaluatedProperties: false
> > +
> > + allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: nvidia,tegra186-display
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: display core clock
> > + - description: display stream compression clock
> > + - description: display hub clock
> > +
> > + clock-names:
> > + items:
> > + - const: disp
> > + - const: dsc
> > + - const: hub
> > + else:
> > + properties:
> > + clocks:
> > + items:
> > + - description: display core clock
> > + - description: display hub clock
> > +
> > + clock-names:
> > + items:
> > + - const: disp
> > + - const: hub
> > +
> > + unevaluatedProperties: false
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra20-host1x
> > + - nvidia,tegra30-host1x
> > + - nvidia,tegra114-host1x
> > + - nvidia,tegra124-host1x
> > + then:
> > + patternProperties:
> > + "^hdmi@[0-9a-f]+$":
> > + description: High Definition Multimedia Interface
> > + type: object
> > + properties:
> > + # required
> > + compatible:
> > + oneOf:
> > + - const: nvidia,tegra20-hdmi
> > + - const: nvidia,tegra30-hdmi
> > + - const: nvidia,tegra114-hdmi
> > + - const: nvidia,tegra124-hdmi
> > + - items:
> > + - const: nvidia,tegra132-hdmi
> > + - const: nvidia,tegra124-hdmi
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: module clock
> > + - description: parent clock
> > +
> > + clock-names:
> > + items:
> > + - const: hdmi
> > + - const: parent
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: hdmi
> > +
> > + hdmi-supply:
> > + description: supply for the +5V HDMI connector pin
> > +
> > + vdd-supply:
> > + description: regulator for supply voltage
> > +
> > + pll-supply:
> > + description: regulator for PLL
> > +
> > + # optional
> > + nvidia,ddc-i2c-bus:
> > + description: phandle of an I2C controller used for DDC EDID
> > + probing
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + nvidia,hpd-gpio:
> > + description: specifies a GPIO used for hotplug detection
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
>
> *-gpio has a type already. Just 'maxItems: 1' needed.
Yes, that seems to work fine.
> > +
> > + nvidia,edid:
> > + description: supplies a binary EDID blob
> > + $ref: "/schemas/types.yaml#/definitions/uint8-array"
> > +
> > + nvidia,panel:
> > + description: phandle of a display panel
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + status:
> > + $ref: "/schemas/dt-core.yaml#/properties/status"
> > +
> > + phandle:
> > + $ref: "/schemas/types.yaml#/definitions/uint32"
> > +
> > + unevaluatedProperties: false
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - nvidia,tegra124-host1x
> > + - nvidia,tegra210-host1x
> > + - nvidia,tegra186-host1x
> > + - nvidia,tegra194-host1x
> > + then:
> > + patternProperties:
> > + "^sor@[0-9a-f]+$":
> > + description: |
> > + The Serial Output Resource (SOR) can be used to drive HDMI, LVDS,
> > + eDP and DP outputs.
> > +
> > + See ../pinctrl/nvidia,tegra124-dpaux-padctl.txt for information
> > + regarding the DPAUX pad controller bindings.
> > + type: object
> > + properties:
> > + # required
> > + compatible:
> > + oneOf:
> > + - const: nvidia,tegra124-sor
> > + - items:
> > + - const: nvidia,tegra132-sor
> > + - const: nvidia,tegra124-sor
> > + - const: nvidia,tegra210-sor
> > + - const: nvidia,tegra210-sor1
> > + - const: nvidia,tegra186-sor
> > + - const: nvidia,tegra186-sor1
> > + - const: nvidia,tegra194-sor
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: sor
> > +
> > + status:
> > + $ref: "/schemas/dt-core.yaml#/properties/status"
>
> 'status' should never need to be listed.
This seems to be needed at least when I try to validate against a single
binding, like so:
$ make DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml dtbs_check
I assume that that somehow prevents the tooling from looking at any of
the other bindings, which in turn then causes status and other standard
properties to never be defined and then it flags them as extra and
causes a failure.
I think I've even seen this trigger on dt_binding_check if I happened to
have status in there. Now, you've mentioned elsewhere that we shouldn't
use "status" in examples, so that would work around this. However, I
think I've seen this happen as well in examples that referenced some
node via phandle, and then dt_binding_check would emit an error about
phandle being undefined.
Perhaps this is a problem with the tooling? Should we instruct the
scripts to always include the core schema even if we're only testing a
single YAML file via DT_SCHEMA_FILES?
> > +
> > + power-domains:
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + avdd-io-hdmi-dp-supply:
> > + description: I/O supply for HDMI/DP
> > +
> > + vdd-hdmi-dp-pll-supply:
> > + description: PLL supply for HDMI/DP
> > +
> > + hdmi-supply:
> > + description: +5.0V HDMI connector supply
> > +
> > + # Tegra186 and later
> > + nvidia,interface:
> > + description: index of the SOR interface
> > + $ref: "/schemas/types.yaml#/definitions/uint32"
> > +
> > + # optional
> > + nvidia,ddc-i2c-bus:
> > + description: phandle of an I2C controller used for DDC EDID
> > + probing
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + nvidia,hpd-gpio:
> > + description: specifies a GPIO used for hotplug detection
> > + $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > +
> > + nvidia,edid:
> > + description: supplies a binary EDID blob
> > + $ref: "/schemas/types.yaml#/definitions/uint8-array"
> > +
> > + nvidia,panel:
> > + description: phandle of a display panel
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + nvidia,xbar-cfg:
> > + description: 5 cells containing the crossbar configuration.
> > + Each lane of the SOR, identified by the cell's index, is
> > + mapped via the crossbar to the pad specified by the cell's
> > + value.
> > + $ref: "/schemas/types.yaml#/definitions/uint32-array"
> > +
> > + # optional when driving an eDP output
> > + nvidia,dpaux:
> > + description: phandle to a DispayPort AUX interface
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
> > +
> > + pinctrl-names: true
> > + phandle:
> > + $ref: "/schemas/types.yaml#/definitions/uint32"
>
> 'phandle' shouldn't need to be listed.
>
> > +
> > + patternProperties:
> > + "^pinctrl-[0-9]+$": true
>
> pinctrl properties are automatically added, but maybe not if under an
> 'if' schema. Really, I think probably either this should be split
> into multiple schema files or all of these child nodes should be
> described at the top-level. I'm not sure it's really important to define
> which set of child nodes belong or not for each chip.
I'm not too worried about the set of child nodes for each chip, but I
think having this all in one file underlines the importance of the
hierarchy. If these were discrete bindings for each of the compatible
strings it'd be easy for someone to create them as standalone nodes in
device tree, but that's not something that would work. All of these
devices are children of host1x and they do depend on host1x for a lot
of the functionality, so the hierarchy must be respected.
> I'm stopping there. I think the rest is more of the same comments.
I've made a pass over the whole file and fixed the issues that you
pointed out above in other places.
Sounds like the biggest remaining issue is with the duplicated standard
properties. I'm not a huge fan of giving up on doing the right thing
because the tooling can't deal with it. I think we should fix the
tooling to do the right thing. So if there's something in the core DT
schema then it should apply regardless of what mode we run in. Much of
the above issues should go away once that's fixed.
Any thoughts on making some of the schema files "always included"? I
haven't looked at this side of the tooling at all yet, so I'm not sure
how difficult that would be, but if you're okay with it conceptually I
can take a closer look.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-06-18 14:16 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-12 14:18 [PATCH 00/38] dt-bindings: json-schema conversions and cleanups Thierry Reding
2020-06-12 14:18 ` [PATCH 01/38] dt-bindings: interrupt-controller: arm,gic: Add compatible for Tegra186 AGIC Thierry Reding
2020-06-12 14:18 ` [PATCH 02/38] dt-bindings: memory: nvidia: Mark memory controller as interconnect provider Thierry Reding
2020-06-12 14:18 ` [PATCH 03/38] dt-bindings: memory: Increase number of reg entries on Tegra194 Thierry Reding
2020-06-12 14:18 ` [PATCH 04/38] dt-bindings: firmware: Convert Tegra186 BPMP bindings to json-schema Thierry Reding
2020-06-17 22:49 ` Rob Herring
2020-06-12 14:18 ` [PATCH 05/38] dt-bindings: firmware: tegra186-bpmp: Document interconnect paths Thierry Reding
2020-06-17 22:50 ` Rob Herring
2020-06-12 14:18 ` [PATCH 06/38] dt-bindings: display: tegra: Document display-hub Thierry Reding
2020-06-17 22:55 ` Rob Herring
2020-06-18 10:27 ` Thierry Reding
2020-06-18 18:17 ` Rob Herring
2020-06-19 6:45 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 07/38] dt-bindings: display: tegra: Convert to json-schema Thierry Reding
2020-06-12 15:54 ` Dmitry Osipenko
2020-06-16 14:51 ` Thierry Reding
2020-06-17 23:13 ` Rob Herring
2020-06-18 14:16 ` Thierry Reding [this message]
2020-06-18 15:23 ` Rob Herring
2020-06-19 8:08 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 08/38] dt-bindings: display: tegra: Document interconnect paths Thierry Reding
2020-06-12 15:52 ` Dmitry Osipenko
2020-06-16 14:47 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 09/38] dt-bindings: gpu: tegra: Convert to json-schema Thierry Reding
2020-06-18 2:29 ` Rob Herring
2020-06-12 14:18 ` [PATCH 10/38] dt-bindings: gpu: tegra: Document interconnect paths Thierry Reding
2020-06-12 14:18 ` [PATCH 11/38] dt-bindings: mmc: tegra: Convert to json-schema Thierry Reding
2020-06-12 14:18 ` [PATCH 12/38] dt-bindings: mmc: tegra: Document interconnect paths Thierry Reding
2020-06-12 14:18 ` [PATCH 13/38] dt-bindings: pci: tegra: Convert to json-schema Thierry Reding
2020-06-12 14:18 ` [PATCH 14/38] dt-bindings: pci: tegra: Document interconnect paths Thierry Reding
2020-06-12 14:18 ` [PATCH 15/38] dt-bindings: sound: tegra: hda: Convert to json-schema Thierry Reding
2020-06-12 14:18 ` [PATCH 16/38] dt-bindings: sound: tegra: hda: Document interconnect paths Thierry Reding
2020-06-12 14:18 ` [PATCH 17/38] dt-bindings: usb: tegra-xusb: Convert to json-schema Thierry Reding
2020-06-12 14:18 ` [PATCH 18/38] dt-bindings: usb: tegra-xusb: Document interconnect paths Thierry Reding
2020-06-12 14:18 ` [PATCH 19/38] dt-bindings: net: dwc-qos-ethernet: Convert to json-schema Thierry Reding
2020-06-12 14:18 ` [PATCH 20/38] dt-bindings: net: dwc-qos-ethernet: Document interconnect paths Thierry Reding
2020-06-12 14:18 ` [PATCH 21/38] dt-bindings: sound: sgtl5000: Convert to json-schema Thierry Reding
2020-06-18 2:41 ` Rob Herring
2020-06-12 14:18 ` [PATCH 22/38] dt-bindings: gpio: tegra186: Use unique include guard Thierry Reding
2020-06-12 14:18 ` [PATCH 23/38] dt-bindings: gpio: tegra186: Convert to json-schema Thierry Reding
2020-06-18 2:44 ` Rob Herring
2020-06-12 14:18 ` [PATCH 24/38] dt-bindings: mfd: max77620: " Thierry Reding
2020-06-12 14:18 ` [PATCH 25/38] dt-bindings: gpio: tegra: " Thierry Reding
2020-06-17 4:24 ` Dmitry Osipenko
2020-06-17 14:17 ` Thierry Reding
2020-06-17 14:24 ` Dmitry Osipenko
2020-06-17 14:33 ` Dmitry Osipenko
2020-06-17 16:50 ` Thierry Reding
2020-06-18 15:07 ` Dmitry Osipenko
2020-06-12 14:18 ` [PATCH 26/38] dt-bindings: pci: iommu: " Thierry Reding
2020-06-18 2:34 ` Rob Herring
2020-06-18 14:18 ` Thierry Reding
2020-06-19 6:45 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 27/38] dt-bindings: tegra: Add missing compatible strings Thierry Reding
2020-06-12 14:18 ` [PATCH 28/38] dt-bindings: phy: tegra-xusb: Convert to json-schema Thierry Reding
2020-06-18 2:38 ` Rob Herring
2020-06-19 6:47 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 29/38] dt-bindings: tegra: pmc: Increase clock limit for power domains Thierry Reding
2020-06-12 14:18 ` [PATCH 30/38] dt-bindings: panel: Allow reg property for DSI panels Thierry Reding
2020-06-12 14:29 ` Rob Herring
2020-06-16 14:35 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 31/38] dt-bindings: panel: simple: Use unevaluatedProperties Thierry Reding
2020-06-12 14:28 ` Rob Herring
2020-06-16 14:33 ` Thierry Reding
2020-06-12 14:18 ` [PATCH 32/38] dt-bindings: leds: Document rfkill* trigger Thierry Reding
2020-06-12 14:18 ` [PATCH 33/38] dt-bindings: memory-controller: Document Tegra132 EMC Thierry Reding
2020-06-12 14:18 ` [PATCH 34/38] dt-bindings: memory-controller: Fix "reg" entries on Tegra194 Thierry Reding
2020-06-12 14:19 ` [PATCH 35/38] dt-bindings: memory: Update Tegra210 EMC bindings Thierry Reding
2020-06-18 15:36 ` Rob Herring
2020-06-12 14:19 ` [PATCH 36/38] dt-bindings: power: supply: sbs-battery: Document TI BQ20Z45 compatible Thierry Reding
2020-06-12 14:19 ` [PATCH 37/38] dt-bindings: pwm: Explicitly include pwm.yaml Thierry Reding
2020-06-18 2:51 ` Rob Herring
2020-06-19 7:46 ` Thierry Reding
2020-06-19 18:05 ` Rob Herring
2020-06-12 14:19 ` [PATCH 38/38] dt-bindings: serial: Document Tegra-specific properties Thierry Reding
2020-06-18 2:47 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200618141630.GB3663225@ulmo \
--to=thierry.reding@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).