Re: [PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 10, 2021 at 06:50:26PM +0300, Mikko Perttunen wrote:
> On 10.8.2021 18.43, Thierry Reding wrote:
> > On Fri, Aug 06, 2021 at 03:34:48PM +0300, Mikko Perttunen wrote:
> > > Convert the original Host1x bindings to YAML and add new bindings for
> > > NVDEC, now in a more appropriate location. The old text bindings
> > > for Host1x and engines are still kept at display/tegra/ since they
> > > encompass a lot more engines that haven't been converted over yet.
> > > 
> > > Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> > > ---
> > > v2:
> > > * Fix issues pointed out in v1
> > > * Add T194 nvidia,instance property
> > > ---
> > >   .../gpu/host1x/nvidia,tegra20-host1x.yaml     | 131 ++++++++++++++++++
> > >   .../gpu/host1x/nvidia,tegra210-nvdec.yaml     | 109 +++++++++++++++
> > >   MAINTAINERS                                   |   1 +
> > >   3 files changed, 241 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
> > >   create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> > 
> > Can we split off the NVDEC bindings addition into a separate patch? I've
> > been working on converting the existing host1x bindings in full to json-
> > schema and this partial conversion would conflict with that effort.
> > 
> > I assume that NVDEC itself validates properly even if host1x hasn't been
> > converted yet?
> 
> Sure. I thought I had some problems with this before but can't see any now.
> 
> > 
> > > diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> > > new file mode 100644
> > > index 000000000000..fc535bb7aee0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> > > @@ -0,0 +1,109 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/gpu/host1x/nvidia,tegra210-nvdec.yaml#";
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> > > +
> > > +title: Device tree binding for NVIDIA Tegra NVDEC
> > > +
> > > +description: |
> > > +  NVDEC is the hardware video decoder present on NVIDIA Tegra210
> > > +  and newer chips. It is located on the Host1x bus and typically
> > > +  programmed through Host1x channels.
> > > +
> > > +maintainers:
> > > +  - Thierry Reding <treding@xxxxxxxxx>
> > > +  - Mikko Perttunen <mperttunen@xxxxxxxxxx>
> > > +
> > > +properties:
> > > +  $nodename:
> > > +    pattern: "^nvdec@[0-9a-f]*$"
> > > +
> > > +  compatible:
> > > +    enum:
> > > +      - nvidia,tegra210-nvdec
> > > +      - nvidia,tegra186-nvdec
> > > +      - nvidia,tegra194-nvdec
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: nvdec
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  reset-names:
> > > +    items:
> > > +      - const: nvdec
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  iommus:
> > > +    maxItems: 1
> > > +
> > > +  interconnects:
> > > +    items:
> > > +      - description: DMA read memory client
> > > +      - description: DMA read 2 memory client
> > > +      - description: DMA write memory client
> > > +
> > > +  interconnect-names:
> > > +    items:
> > > +      - const: dma-mem
> > > +      - const: read2
> > 
> > The convention that we've used so far has been to start numbering these
> > at 0 and use a dash, so this would be "read-1".
> 
> Will fix.
> 
> > 
> > > +      - const: write
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - clock-names
> > > +  - resets
> > > +  - reset-names
> > > +  - power-domains
> > > +
> > > +if:
> > > +  properties:
> > > +    compatible:
> > > +      contains:
> > > +        const: nvidia,tegra194-host1x
> > > +then:
> > > +  properties:
> > > +    nvidia,instance:
> > > +      items:
> > > +        - description: 0 for NVDEC0, or 1 for NVDEC1
> > 
> > I know we had discussed this before, but looking at the driver patch, I
> > don't actually see this being used now, so I wonder if we still need it.
> > 
> > > +additionalProperties: true
> > 
> > Maybe this should have a comment noting that this should really be
> > unevaluatedProperties: false, but we can't use that because the tooling
> > doesn't support it yet?
> 
> I can add such a comment if desired. Honestly, I don't really know what
> 'unevaluatedProperties' means or does -- the explanation in
> example-schema.yaml doesn't seem like it's relevant here and I cannot find
> any other documentation.

It's basically like additionalProperties, except that it applies to
properties evaluated via if: blocks.

So with additionalProperties: false, the presence of the nvidia,instance
property in a Tegra194 DTS file would cause a validation failure because
it's a property that was not defined in the properties: block.

With unevaluatedProperties: false, on the other hand, the properties
that are defined in the if: block above will become a evaluated
properties and therefore a Tegra194 DTS with the nvidia,instance
property present would succeed validation. Unless, of course, if it
contained additional properties that are not defined in any of the
properties: blocks (either unconditional or conditional ones).

So in other words, the additionalProperties schema applies to all
unconditionally defined properties, whereas unevaluatedProperties
applies to all (conditionally and unconditionally) defined properties.

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux