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

"Musham, Sai Krishna" <[email protected]> Wed, 29 Jul 2026 15:45:05 +0000
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-pci
Message-ID <IA4PR12MB9810115150350A1AB20C371BCDCA2@IA4PR12MB9810.namprd12.prod.outlook.com>
Public

Hi Sashiko,

> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: Wednesday, July 29, 2026 1:57 AM
> To: Musham, Sai Krishna <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: pci: Add CPM6 support to AMD MDB
> PCIe binding
>
> 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 <[email protected]>
>
> 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-
> [email protected]/
>
> > 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?
>

Thanks for the review, yes, maxItems for MDB5 is 4, I will fix it and send
in next patch.

> 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?
>

Thanks for the review, MDB5 requires 4 maxItems and CPM6 requires 5 maxItems,
I will fix it and send in next patch.

> --
> Sashiko AI review ยท https://sashiko.dev/#/patchset/20260728202044.1785986-
> [email protected]?part=1