Re: [PATCH v2 1/2] dt-bindings: pci: Add CPM6 support to AMD MDB PCIe binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The conditional blocks for 'amd,versal2-mdb-host' and 'amd,versal2-cpm6-host' fail to enforce the strict number of register regions, allowing incomplete or invalid device trees to silently pass schema validation.
--

--- Patch [1]: [PATCH v2 1/2] dt-bindings: pci: Add CPM6 support to AMD MDB PCIe binding ---
commit b6d71a08cf4101b94f4ec914fe86d18c5cd2b7fa
Author: Sai Krishna Musham <sai.krishna.musham@amd.com>

dt-bindings: pci: Add CPM6 support to AMD MDB PCIe binding

The AMD CPM6 PCIe controller is based on the Synopsys DesignWare PCIe IP.
This adds "intr" to the vendor-specific reg-names for the per-controller
interrupt register region and updates the amd,versal2-mdb-host.yaml with
separate register definitions for MDB5 and CPM6.

Link: https://lore.kernel.org/all/20260402180006.486229-2-sai.krishna.musham@amd.com/

> diff --git a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> index 406c15e1dee1b..d22de24b41cae 100644
> --- a/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
> +++ b/Documentation/devicetree/bindings/pci/amd,versal2-mdb-host.yaml
[ ... ]
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amd,versal2-mdb-host
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: MDB System Level Control and Status Register (SLCR) Base
> +            - description: configuration region
> +            - description: data bus interface
> +            - description: address translation unit register
> +        reg-names:
> +          items:
> +            - const: slcr
> +            - const: config
> +            - const: dbi
> +            - const: atu

[Severity: Medium]
Since the global maxItems for reg and reg-names is increased to 5 in this
patch, does this conditional block for amd,versal2-mdb-host need to explicitly
specify maxItems: 4?

Without it, could a device tree with an invalid 5th region silently pass
schema validation for MDB5?

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - amd,versal2-cpm6-host
> +              - amd,versal2-cpm6-host1
> +    then:
> +      properties:
> +        reg:
> +          minItems: 4
> +          items:
> +            - description: MDB System Level Control and Status Register (SLCR) Base
> +            - description: configuration region
> +            - description: data bus interface
> +            - description: address translation unit register
> +            - description: CPM6 PCIe error and event interrupt registers
> +        reg-names:
> +          minItems: 4
> +          items:
> +            - const: slcr
> +            - const: config
> +            - const: dbi
> +            - const: atu
> +            - const: intr

[Severity: Medium]
Does setting minItems: 4 here inadvertently make the 5th CPM6-specific region
(intr) optional?

If CPM6 always requires all 5 regions, should minItems: 4 be removed so that
validation enforces the full array length?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728202044.1785986-1-sai.krishna.musham@amd.com?part=1