Thread (40 messages) 40 messages, 8 authors, 2025-08-29

Re: [PATCH v2 03/10] dt-bindings: PCI: Add ASPEED PCIe RC support

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-07-16 08:27:18
Also in: linux-aspeed, linux-devicetree, linux-gpio, linux-pci, lkml, openbmc

On Tue, Jul 15, 2025 at 11:43:13AM +0800, Jacky Chou wrote:
This binding describes the required and optional properties for
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.
...
+
+  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.
+    $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.
+  - $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.

Best regards,
Krzysztof

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help