Hi Krzysztof, Thank you for your reply. > No, describe the hardware, not "this binding". > > > configuring the PCIe RC node, including support for syscon phandles, > > MSI, clocks, resets, and interrupt mapping. The schema enforces strict > > property validation and provides a comprehensive example for reference. > > Don't say what schema does or does not. It's completely redundant. > Describe the hardware. > > Your entire commit is redundantn and not helpful at all. > I will revise the commit message in the next version. Thanks for your guidance. > > > > ... > > > + > > + aspeed,ahbc: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + Phandle to the ASPEED AHB Controller (AHBC) syscon node. > > + This reference is used by the PCIe controller to access > > + system-level configuration registers related to the AHB bus. > > + To enable AHB access for the PCIe controller. > > + > > + aspeed,pciecfg: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + Phandle to the ASPEED PCIe configuration syscon node. > > + This reference allows the PCIe controller to access > > + SoC-specific PCIe configuration registers. There are the others > > + functions such PCIe RC and PCIe EP will use this common register > > + to configure the SoC interfaces. > > + > > + aspeed,pciephy: > > No, phys are not syscons. I already told you that in v1. > I will remove the aspeed,pciephy syscon reference and rework this part to use the standard phys binding properly. Sorry for overlooking your previous feedback in v1. Thanks again for your patience. > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + Phandle to the ASPEED PCIe PHY syscon node. > > + This property provides access to the PCIe PHY control > > + registers required for link initialization and management. > > + The other functions such PCIe RC and PCIe EP will use this > > + common register to configure the PHY interfaces and get some > > + information from the PHY. > > + > > + interrupt-controller: > > + description: Interrupt controller node for handling legacy PCI > interrupts. > > + type: object > > + properties: > > + '#address-cells': > > + const: 0 > > + '#interrupt-cells': > > + const: 1 > > + interrupt-controller: true > > + > > + required: > > + - '#address-cells' > > + - '#interrupt-cells' > > + - interrupt-controller > > + > > + additionalProperties: false > > + > > +allOf: > > + - $ref: /schemas/pci/pci-bus-common.yaml# > > No other binding references this. Don't write completely different code than all > other SoCs. This entire binding is written such way. > Agreed. I will remove it in next version. > > + - $ref: /schemas/pci/pci-host-bridge.yaml# > > + - $ref: /schemas/interrupt-controller/msi-controller.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: aspeed,ast2600-pcie > > + then: > > + required: > > + - aspeed,ahbc > > + else: > > + properties: > > + aspeed,ahbc: false > > + > > +required: > > + - reg > > + - interrupts > > + - bus-range > > + - ranges > > + - resets > > + - reset-names > > + - msi-parent > > + - msi-controller > > + - aspeed,pciecfg > > + - interrupt-map-mask > > + - interrupt-map > > + - interrupt-controller > > + > > +unevaluatedProperties: false > > + > > +patternProperties: > > + "^pcie@[0-9a-f,]+$": > > Why do you need it? Also, order things according to example schema. > Thanks for your question. In the v1 discussion, another reviewer suggested that we should support a multi-port structure for the PCIe root complex, where each port is represented as a child node (e.g., pcie@...). That's why patternProperties was added here — to explicitly allow such subnodes and validate them properly. Thanks, Jacky
