Re: [PATCH v2 3/4] dt-bindings: PCI: Add StarFive JH7110 PCIe controller
From: Rob Herring <robh@kernel.org>
Date: 2023-08-10 22:47:34
Also in:
linux-pci, lkml
On Thu, Jul 27, 2023 at 06:39:48PM +0800, Minda Chen wrote:
quoted hunk ↗ jump to hunk
Add StarFive JH7110 SoC PCIe controller dt-bindings. JH7110 using PLDA XpressRICH PCIe host controller IP. Signed-off-by: Minda Chen <minda.chen@starfivetech.com> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> --- .../bindings/pci/starfive,jh7110-pcie.yaml | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yamldiff --git a/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml new file mode 100644 index 000000000000..9273e029fb20 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml@@ -0,0 +1,133 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/starfive,jh7110-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive JH7110 PCIe host controller + +maintainers: + - Kevin Xie <kevin.xie@starfivetech.com> + +allOf: + - $ref: /schemas/pci/pci-bus.yaml#
Can go in common schema. Unless endpoint mode is or is going to be supported?
+ - $ref: plda,xpressrich3-axi-common.yaml# + - $ref: /schemas/interrupt-controller/msi-controller.yaml#
This is a bit odd. Do you really need to define the host bridge as an msi-controller? Many host bridges are, but they don't need to be defined as one in DT because PCIe spec defines its own way to do MSIs.
+ - $ref: /schemas/gpio/gpio-consumer-common.yaml#
No, you don't need to reference this. You need to define what properties from it you use (reset-gpios). Though if this is for PERST#, then I'd use "perst-gpios" instead. And why is that not common? It's board specific, not SoC, whether or not there's GPIO control of it.
+ +properties: + compatible: + const: starfive,jh7110-pcie + + clocks: + items: + - description: NOC bus clock + - description: Transport layer clock + - description: AXI MST0 clock + - description: APB clock + + clock-names: + items: + - const: noc + - const: tl + - const: axi_mst0 + - const: apb + + resets: + items: + - description: AXI MST0 reset + - description: AXI SLAVE0 reset + - description: AXI SLAVE reset + - description: PCIE BRIDGE reset + - description: PCIE CORE reset + - description: PCIE APB reset + + reset-names: + items: + - const: mst0 + - const: slv0 + - const: slv + - const: brg + - const: core + - const: apb + + starfive,stg-syscon: + $ref: /schemas/types.yaml#/definitions/phandle-array + items: + - items: + - description: phandle to System Register Controller stg_syscon node. + - description: register0 offset of STG_SYSCONSAIF__SYSCFG register for PCIe. + - description: register1 offset of STG_SYSCONSAIF__SYSCFG register for PCIe. + - description: register2 offset of STG_SYSCONSAIF__SYSCFG register for PCIe. + - description: register3 offset of STG_SYSCONSAIF__SYSCFG register for PCIe. + description: + The phandle to System Register Controller syscon node and the offset + of STG_SYSCONSAIF__SYSCFG register for PCIe. Total 4 regsisters offset + for PCIe.
Perhaps somewhere in here state what these registers are for.
+ + phys: + description: + Specified PHY is attached to PCIe controller. + maxItems: 1 + +required: + - compatible + - clocks + - resets + - starfive,stg-syscon
+ - "#interrupt-cells" + - interrupt-map-mask + - interrupt-map
These can all be common. So could 'compatible' though it is implicitly required anyways as without it, the schema is never applied.
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pcie0: pcie@2b000000 {
+ compatible = "starfive,jh7110-pcie";
+ reg = <0x9 0x40000000 0x0 0x10000000>,
+ <0x0 0x2b000000 0x0 0x1000000>;
+ reg-names = "cfg", "apb";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ device_type = "pci";
+ ranges = <0x82000000 0x0 0x30000000 0x0 0x30000000 0x0 0x08000000>,
+ <0xc3000000 0x9 0x00000000 0x9 0x00000000 0x0 0x40000000>;
+ starfive,stg-syscon = <&stg_syscon 0xc0 0xc4 0x130 0x1b8>;
+ bus-range = <0x0 0xff>;
+ interrupt-parent = <&plic>;
+ interrupts = <56>;
+ interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+ interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc0 0x1>,
+ <0x0 0x0 0x0 0x2 &pcie_intc0 0x2>,
+ <0x0 0x0 0x0 0x3 &pcie_intc0 0x3>,
+ <0x0 0x0 0x0 0x4 &pcie_intc0 0x4>;
+ msi-parent = <&pcie0>;Weird!?
+ msi-controller;
+ clocks = <&syscrg 86>,
+ <&stgcrg 10>,
+ <&stgcrg 8>,
+ <&stgcrg 9>;
+ clock-names = "noc", "tl", "axi_mst0", "apb";
+ resets = <&stgcrg 11>,
+ <&stgcrg 12>,
+ <&stgcrg 13>,
+ <&stgcrg 14>,
+ <&stgcrg 15>,
+ <&stgcrg 16>;
+ reset-gpios = <&gpios 26 GPIO_ACTIVE_LOW>;
+ phys = <&pciephy0>;
+
+ pcie_intc0: interrupt-controller {
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ interrupt-controller;
+ };
+ };
+ };
--
2.17.1