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