Re: [PATCH v6] dt-bindings: pci: tegra: Convert to json-schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Broken schema cross-reference to `pci-iommu.yaml`.
- [Medium] Missing `power-domains` in the `required` properties for the Tegra186 variant.
- [Medium] Missing `required` array for root port child nodes, failing to enforce `nvidia,num-lanes`.
- [Medium] Missing conditional requirement for `phys` and `phy-names` on root port child nodes for Tegra124+.
--

commit 9fe702ef36ffb628e7fb46da07e21ea42a9f10b3
Author: Thierry Reding <treding@nvidia.com>
Subject: dt-bindings: pci: tegra: Convert to json-schema

This commit converts the Tegra PCIe controller bindings from the
free-form text format to json-schema. It references common PCI
bindings and sets conditional constraints for various properties across
Tegra generations.

> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.yaml

[ ... ]

> +patternProperties:
> +  "^pci@1?[0-9a-f](,[0-7])?$":
> +    description: Root ports are defined as subnodes of the PCIe controller

[ ... ]

> +      nvidia,num-lanes:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: |
> +          Number of lanes to use for this port. Valid combinations
> +          are:
> +
> +          - Root port 0 uses 4 lanes, root port 1 is unused.
> +          - Both root ports use 2 lanes.
> +        enum: [ 0, 1, 2, 4 ]
> +
> +      phys:
> +        description: Must contain an phandle to a PHY for each entry in
> +          phy-names.
> +        minItems: 1
> +        maxItems: 4

[Severity: Medium]
The original text binding required phys and phy-names for root port
child nodes on Tegra124, Tegra210, and Tegra186 controllers.

By defining these as optional properties on the child nodes without an
if/then conditional in the parent node to enforce them, will device trees
for newer variants missing these PHY mappings incorrectly pass validation?

> +
> +      phy-names:
> +        description: Must include an entry for each active lane. Note that the
> +          number of entries does not have to (though usually will) be equal to
> +          the specified number of lanes in the nvidia,num-lanes property.
> +          Entries are of the form "pcie-N", where N ranges from 0 to the value
> +          specified in nvidia,num-lanes.
> +        items:
> +          enum:
> +            - pcie-0
> +            - pcie-1
> +            - pcie-2
> +            - pcie-3
> +        minItems: 1
> +        maxItems: 4
> +
> +required:
> +  - reg
> +  - reg-names

[Severity: Medium]
This marks the end of the child node patternProperties definition, which
appears to omit a required block.

The original text binding mandated nvidia,num-lanes, reg, and
assigned-addresses for root port subnodes.

Since the required array is entirely omitted from the child node, can device
trees omit the hardware-specific lane configuration without triggering a
schema validation error?

[ ... ]

> +unevaluatedProperties: false
> +
> +allOf:
> +  - $ref: /schemas/pinctrl/pinctrl-consumer.yaml#
> +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> +  - $ref: pci-iommu.yaml#

[Severity: Medium]
Because this is a relative reference, it resolves against the current
file's $id directory, effectively searching for
http://devicetree.org/schemas/pci/pci-iommu.yaml#.

Since pci-iommu.yaml is actually located in the virtio directory, will this
broken reference cause dt_binding_check to fail or skip validation for this
binding?

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: nvidia,tegra186-pcie
> +    then:
> +      properties:

[ ... ]

> +        vddio-pexctl-aud-supply:
> +          description: Power supply for PCIe side band signals. Must supply
> +            1.8 V.
> +
> +      required:
> +        - dvdd-pex-supply
> +        - hvdd-pex-pll-supply
> +        - hvdd-pex-supply
> +        - vddio-pexctl-aud-supply

[Severity: Medium]
The power-domains property is defined as a capability for the Tegra186
variant earlier in the binding.

Should power-domains be included in this required array so that incomplete
device trees for Tegra186 hardware do not incorrectly pass schema validation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902142449.735168-1-thierry.reding@kernel.org?part=1