Re: [PATCH v1 1/5] dt-bindings: PCI: Convert the existing nvidia,tegra-pcie.txt bindings documentation into a YAML schema

[Anand Moon <linux.amoon@gmail.com>]

Hi Rob

On Tue, 30 Sept 2025 at 20:07, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Sep 29, 2025 at 10:25 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Rob
> >
> > On Mon, 29 Sept 2025 at 19:19, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Sep 29, 2025 at 2:40 AM Anand Moon <linux.amoon@gmail.com> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > Thanks for your review comments
> > > >
> > > > On Fri, 26 Sept 2025 at 19:26, Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > On Fri, Sep 26, 2025 at 2:29 AM Anand Moon <linux.amoon@gmail.com> wrote:
> > > > > >
> > > > > > Convert the legacy text-based binding documentation for
> > > > > > nvidia,tegra-pcie into a nvidia,tegra-pcie.yaml YAML schema, following
> > > > >
> > > > > s/YAML/DT/
> > > > >
> > > > Ok,
> > > > > > the Devicetree Schema format. This improves validation coverage and enables
> > > > > > dtbs_check compliance for Tegra PCIe nodes.
> > > > >
> > > > > Your subject needs some work too. 'existing' and 'bindings
> > > > > documentation' are redundant.
> > > > >
> > > > Here is the simplified version
> > > >
> > > > dt-bindings: PCI: Convert the nvidia,tegra-pcie bindings documentation
> > > > into a YAML schema
> > >
> > > Still doesn't fit on one line and you say bindings twice:
> > >
> > > dt-bindings: PCI: Convert nvidia,tegra-pcie to DT schema
> > >
> > Ok
> > > >
> > > > Convert the existing text-based DT bindings documentation for the
> > > > NVIDIA Tegra PCIe host controller to a YAML schema format.
> > >
> > > s/YAML/DT/
> > >
> > > Lots of things are YAML. Only one thing is DT schema.
> > Ok, understood.
> > >
> > > >
> > > > > >
> > > > > > Cc: Jon Hunter <jonathanh@nvidia.com>
> > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > > > ---
> > > > > > v1: new patch in this series.
> > > > > > ---
> > > > > >  .../bindings/pci/nvidia,tegra-pcie.yaml       | 651 +++++++++++++++++
> > > > > >  .../bindings/pci/nvidia,tegra20-pcie.txt      | 670 ------------------
> > > > > >  2 files changed, 651 insertions(+), 670 deletions(-)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml
> > > > > >  delete mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..dd8cba125b53
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml
> > > > > > @@ -0,0 +1,651 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/pci/nvidia,tegra-pcie.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: NVIDIA Tegra PCIe Controller
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Thierry Reding <thierry.reding@gmail.com>
> > > > > > +  - Jon Hunter <jonathanh@nvidia.com>
> > > > > > +
> > > > > > +description: |
> > > > >
> > > > > Don't need '|'.
> > > > >
> > > > Ok
> > > > > > +  PCIe controller found on NVIDIA Tegra SoCs including Tgra20, Tegra30,
> > > > > > +  Tegra124, Tegra210, and Tegra186. Supports multiple root ports and
> > > > > > +  platform-specific clock, reset, and power supply configurations.
> > > > >
> > > > > I would suggest not listing every SoC here unless the list is not going to grow.
> > > > >
> > > > Here is the short format.
> > > >   PCIe controller found on NVIDIA Tegra SoCs which supports multiple
> > > >   root ports and platform-specific clock, reset, and power supply
> > > >   configurations.
> > > > Ok
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    oneOf:
> > > > >
> > > > > Only 1 entry here, don't need 'oneOf'.
> > > >
> > > > I am observing the following warning if I remove this.
> > > >
> > > >  make ARCH=arm64 -j$(nproc) dt_binding_check
> > > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml
> > > >   CHKDT   ./Documentation/devicetree/bindings
> > > > /media/nvme0/mainline/linux-tegra-6.y-devel/Documentation/devicetree/bindings/pci/nvidia,tegra-pcie.yaml:
> > > > properties:compatible: [{'items': [{'enum': ['nvidia,tegra20-pcie',
> > > > 'nvidia,tegra30-pcie', 'nvidia,tegra124-pcie', 'nvidia,tegra210-pcie',
> > > > 'nvidia,tegra186-pcie']}]}] is not of type 'object', 'boolean'
> > >
> > > Because you made 'compatible' a list rather than a schema/map/dict.
> > > IOW, You need to remove the '-' as well.
> > >
> > Ok fixed.
> > >
> > > > > > +  nvidia,num-lanes:
> > > > > > +    description: Number of PCIe lanes used
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > >
> > > > > The examples show this in child nodes.
> > > > yes it patternProperties example I missed this.
> > > >
> > > > patternProperties:
> > > >   "^pci@[0-9a-f]+$":
> > > >     type: object
> > > >
> > > >     properties:
> > > >       reg:
> > > >         maxItems: 1
> > > >
> > > >       nvidia,num-lanes:
> > > >         description: Number of PCIe lanes used
> > > >         $ref: /schemas/types.yaml#/definitions/uint32
> > > >         minimum: 1
> > > >
> > > >     unevaluatedProperties: false
> > >
> > > What about all the other properties in the child nodes? You need a
> > > $ref to pci-pci-bridge.yaml as well.
> > Thanks for the input.
> >
> > patternProperties:
> >   "^pci@[0-9a-f]+$":
> >     type: object
> >     allOf:
> >       - $ref: /schemas/pci/pci-host-bridge.yaml#
>
> That's not the one you need. Read my reply again.
>
I'm sorry, I missed pci-pci-bridge.yaml
> >       - properties:
>
> properties doesn't need to go under allOf. Actually, don't need allOf
> here at all.
>
> >           reg:
> >             maxItems: 1
>
> >           "#address-cells":
> >             const: 3
> >           "#size-cells":
> >             const: 2
>
> These 2 are already defined in the referenced schema.
Earlier, I had tried to search for these reference schemas,
but I could not find them.
>
> >           nvidia,num-lanes:
> >             description: Number of PCIe lanes used
> >             $ref: /schemas/types.yaml#/definitions/uint32
> >             minimum: 1
>
> I assume there's a maximum <=16?
I will check and update
>
> blank line here and between all DT properties.
>
> >         required:
> >           - "#address-cells"
> >           - "#size-cells"
>
> These 2 are already required in the referenced schema.
Ok dropped
>
> >           - nvidia,num-lanes
> >     unevaluatedProperties: false
> >
> > > Rob
Thanks
-Anand