All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema
@ 2022-04-22 11:48 Dmitry Baryshkov
  2022-04-22 11:48 ` [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML Dmitry Baryshkov
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Convert pci/qcom,pcie schema to YAML description. The first patch
introduces several warnings which are fixed by the other patches in the
series.

Note regarding the snps,dw-pcie compatibility. The Qualcomm PCIe
controller uses Synopsys PCIe IP core. However it is not just fused to
the address space. Accessing PCIe registers requires several clocks and
regulators to be powered up. Thus it can be assumed that the qcom,pcie
bindings are not fully compatible with the snps,dw-pcie schema.

Dmitry Baryshkov (6):
  dt-bindings: pci/qcom,pcie: convert to YAML
  dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset
  dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback
  arm64: dts: qcom: stop using snps,dw-pcie falback
  arm: dts: qcom: stop using snps,dw-pcie falback

 .../devicetree/bindings/pci/qcom,pcie.txt     | 397 ----------
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 701 ++++++++++++++++++
 arch/arm/boot/dts/qcom-apq8064.dtsi           |   2 +-
 arch/arm/boot/dts/qcom-ipq4019.dtsi           |   2 +-
 arch/arm64/boot/dts/qcom/msm8996.dtsi         |   6 +-
 arch/arm64/boot/dts/qcom/qcs404.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |   4 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |   6 +-
 8 files changed, 712 insertions(+), 408 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.yaml

-- 
2.35.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
@ 2022-04-22 11:48 ` Dmitry Baryshkov
  2022-04-22 13:10   ` Krzysztof Kozlowski
  2022-04-22 11:48 ` [PATCH 2/6] dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset Dmitry Baryshkov
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Changes to examples:
 - Inline clock and reset numbers rather than including dt-bindings
   files because of conflicts between the headers
 - Split ranges properties to follow current practice

Changes to the schema:
 - Fixed the ordering of clock-names/reset-names according to
   the dtsi files.
 - Mark vdda-supply as required only for apq/ipq8064 (as it was marked
   as generally required in the txt file).

Note: while it was not clearly described in text schema, the majority of
Qualcomm platforms follow the snps,dw-pcie schema and use two
compatibility strings in the DT files: platform-specific one and a
fallback to the generic snps,dw-pcie one. This will be sorted out in the
next patches.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.txt     | 397 ------------
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 607 ++++++++++++++++++
 2 files changed, 607 insertions(+), 397 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
deleted file mode 100644
index 0adb56d5645e..000000000000
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ /dev/null
@@ -1,397 +0,0 @@
-* Qualcomm PCI express root complex
-
-- compatible:
-	Usage: required
-	Value type: <stringlist>
-	Definition: Value should contain
-			- "qcom,pcie-ipq8064" for ipq8064
-			- "qcom,pcie-ipq8064-v2" for ipq8064 rev 2 or ipq8065
-			- "qcom,pcie-apq8064" for apq8064
-			- "qcom,pcie-apq8084" for apq8084
-			- "qcom,pcie-msm8996" for msm8996 or apq8096
-			- "qcom,pcie-ipq4019" for ipq4019
-			- "qcom,pcie-ipq8074" for ipq8074
-			- "qcom,pcie-qcs404" for qcs404
-			- "qcom,pcie-sc8180x" for sc8180x
-			- "qcom,pcie-sdm845" for sdm845
-			- "qcom,pcie-sm8250" for sm8250
-			- "qcom,pcie-sm8450-pcie0" for PCIe0 on sm8450
-			- "qcom,pcie-sm8450-pcie1" for PCIe1 on sm8450
-			- "qcom,pcie-ipq6018" for ipq6018
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: Register ranges as listed in the reg-names property
-
-- reg-names:
-	Usage: required
-	Value type: <stringlist>
-	Definition: Must include the following entries
-			- "parf"   Qualcomm specific registers
-			- "dbi"	   DesignWare PCIe registers
-			- "elbi"   External local bus interface registers
-			- "config" PCIe configuration space
-			- "atu"    ATU address space (optional)
-
-- device_type:
-	Usage: required
-	Value type: <string>
-	Definition: Should be "pci". As specified in snps,dw-pcie.yaml
-
-- #address-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: Should be 3. As specified in snps,dw-pcie.yaml
-
-- #size-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: Should be 2. As specified in snps,dw-pcie.yaml
-
-- ranges:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: As specified in snps,dw-pcie.yaml
-
-- interrupts:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: MSI interrupt
-
-- interrupt-names:
-	Usage: required
-	Value type: <stringlist>
-	Definition: Should contain "msi"
-
-- #interrupt-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: Should be 1. As specified in snps,dw-pcie.yaml
-
-- interrupt-map-mask:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: As specified in snps,dw-pcie.yaml
-
-- interrupt-map:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: As specified in snps,dw-pcie.yaml
-
-- clocks:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: List of phandle and clock specifier pairs as listed
-		    in clock-names property
-
-- clock-names:
-	Usage: required
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "iface"	Configuration AHB clock
-
-- clock-names:
-	Usage: required for ipq/apq8064
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "core"	Clocks the pcie hw block
-			- "phy"		Clocks the pcie PHY block
-			- "aux" 	Clocks the pcie AUX block
-			- "ref" 	Clocks the pcie ref block
-- clock-names:
-	Usage: required for apq8084/ipq4019
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "aux"		Auxiliary (AUX) clock
-			- "bus_master"	Master AXI clock
-			- "bus_slave"	Slave AXI clock
-
-- clock-names:
-	Usage: required for msm8996/apq8096
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "pipe"	Pipe Clock driving internal logic
-			- "aux"		Auxiliary (AUX) clock
-			- "cfg"		Configuration clock
-			- "bus_master"	Master AXI clock
-			- "bus_slave"	Slave AXI clock
-
-- clock-names:
-	Usage: required for ipq8074
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "iface"	PCIe to SysNOC BIU clock
-			- "axi_m"	AXI Master clock
-			- "axi_s"	AXI Slave clock
-			- "ahb"		AHB clock
-			- "aux"		Auxiliary clock
-
-- clock-names:
-	Usage: required for ipq6018
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "iface"	PCIe to SysNOC BIU clock
-			- "axi_m"	AXI Master clock
-			- "axi_s"	AXI Slave clock
-			- "axi_bridge"	AXI bridge clock
-			- "rchng"
-
-- clock-names:
-	Usage: required for qcs404
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "iface"	AHB clock
-			- "aux"		Auxiliary clock
-			- "master_bus"	AXI Master clock
-			- "slave_bus"	AXI Slave clock
-
-- clock-names:
-	Usage: required for sdm845
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "aux"		Auxiliary clock
-			- "cfg"		Configuration clock
-			- "bus_master"	Master AXI clock
-			- "bus_slave"	Slave AXI clock
-			- "slave_q2a"	Slave Q2A clock
-			- "tbu"		PCIe TBU clock
-			- "pipe"	PIPE clock
-
-- clock-names:
-	Usage: required for sc8180x and sm8250
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "aux"		Auxiliary clock
-			- "cfg"		Configuration clock
-			- "bus_master"	Master AXI clock
-			- "bus_slave"	Slave AXI clock
-			- "slave_q2a"	Slave Q2A clock
-			- "tbu"		PCIe TBU clock
-			- "ddrss_sf_tbu" PCIe SF TBU clock
-			- "pipe"	PIPE clock
-
-- clock-names:
-	Usage: required for sm8450-pcie0 and sm8450-pcie1
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "aux"         Auxiliary clock
-			- "cfg"         Configuration clock
-			- "bus_master"  Master AXI clock
-			- "bus_slave"   Slave AXI clock
-			- "slave_q2a"   Slave Q2A clock
-			- "tbu"         PCIe TBU clock
-			- "ddrss_sf_tbu" PCIe SF TBU clock
-			- "pipe"        PIPE clock
-			- "pipe_mux"    PIPE MUX
-			- "phy_pipe"    PIPE output clock
-			- "ref"         REFERENCE clock
-			- "aggre0"	Aggre NoC PCIe0 AXI clock, only for sm8450-pcie0
-			- "aggre1"	Aggre NoC PCIe1 AXI clock
-
-- resets:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: List of phandle and reset specifier pairs as listed
-		    in reset-names property
-
-- reset-names:
-	Usage: required for ipq/apq8064
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "axi"  AXI reset
-			- "ahb"  AHB reset
-			- "por"  POR reset
-			- "pci"  PCI reset
-			- "phy"  PHY reset
-
-- reset-names:
-	Usage: required for apq8084
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "core" Core reset
-
-- reset-names:
-	Usage: required for ipq/apq8064
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "axi_m"		AXI master reset
-			- "axi_s"		AXI slave reset
-			- "pipe"		PIPE reset
-			- "axi_m_vmid"		VMID reset
-			- "axi_s_xpu"		XPU reset
-			- "parf"		PARF reset
-			- "phy"			PHY reset
-			- "axi_m_sticky"	AXI sticky reset
-			- "pipe_sticky"		PIPE sticky reset
-			- "pwr"			PWR reset
-			- "ahb"			AHB reset
-			- "phy_ahb"		PHY AHB reset
-			- "ext"			EXT reset
-
-- reset-names:
-	Usage: required for ipq8074
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "pipe"		PIPE reset
-			- "sleep"		Sleep reset
-			- "sticky"		Core Sticky reset
-			- "axi_m"		AXI Master reset
-			- "axi_s"		AXI Slave reset
-			- "ahb"			AHB Reset
-			- "axi_m_sticky"	AXI Master Sticky reset
-
-- reset-names:
-	Usage: required for ipq6018
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "pipe"		PIPE reset
-			- "sleep"		Sleep reset
-			- "sticky"		Core Sticky reset
-			- "axi_m"		AXI Master reset
-			- "axi_s"		AXI Slave reset
-			- "ahb"			AHB Reset
-			- "axi_m_sticky"	AXI Master Sticky reset
-			- "axi_s_sticky"	AXI Slave Sticky reset
-
-- reset-names:
-	Usage: required for qcs404
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "axi_m"		AXI Master reset
-			- "axi_s"		AXI Slave reset
-			- "axi_m_sticky"	AXI Master Sticky reset
-			- "pipe_sticky"		PIPE sticky reset
-			- "pwr"			PWR reset
-			- "ahb"			AHB reset
-
-- reset-names:
-	Usage: required for sc8180x, sdm845, sm8250 and sm8450
-	Value type: <stringlist>
-	Definition: Should contain the following entries
-			- "pci"			PCIe core reset
-
-- power-domains:
-	Usage: required for apq8084 and msm8996/apq8096
-	Value type: <prop-encoded-array>
-	Definition: A phandle and power domain specifier pair to the
-		    power domain which is responsible for collapsing
-		    and restoring power to the peripheral
-
-- vdda-supply:
-	Usage: required
-	Value type: <phandle>
-	Definition: A phandle to the core analog power supply
-
-- vdda_phy-supply:
-	Usage: required for ipq/apq8064
-	Value type: <phandle>
-	Definition: A phandle to the analog power supply for PHY
-
-- vdda_refclk-supply:
-	Usage: required for ipq/apq8064
-	Value type: <phandle>
-	Definition: A phandle to the analog power supply for IC which generates
-		    reference clock
-- vddpe-3v3-supply:
-	Usage: optional
-	Value type: <phandle>
-	Definition: A phandle to the PCIe endpoint power supply
-
-- phys:
-	Usage: required for apq8084 and qcs404
-	Value type: <phandle>
-	Definition: List of phandle(s) as listed in phy-names property
-
-- phy-names:
-	Usage: required for apq8084 and qcs404
-	Value type: <stringlist>
-	Definition: Should contain "pciephy"
-
-- <name>-gpios:
-	Usage: optional
-	Value type: <prop-encoded-array>
-	Definition: List of phandle and GPIO specifier pairs. Should contain
-			- "perst-gpios"	PCIe endpoint reset signal line
-			- "wake-gpios"	PCIe endpoint wake signal line
-
-* Example for ipq/apq8064
-	pcie@1b500000 {
-		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
-		reg = <0x1b500000 0x1000
-		       0x1b502000 0x80
-		       0x1b600000 0x100
-		       0x0ff00000 0x100000>;
-		reg-names = "dbi", "elbi", "parf", "config";
-		device_type = "pci";
-		linux,pci-domain = <0>;
-		bus-range = <0x00 0xff>;
-		num-lanes = <1>;
-		#address-cells = <3>;
-		#size-cells = <2>;
-		ranges = <0x81000000 0 0 0x0fe00000 0 0x00100000   /* I/O */
-			  0x82000000 0 0 0x08000000 0 0x07e00000>; /* memory */
-		interrupts = <GIC_SPI 238 IRQ_TYPE_NONE>;
-		interrupt-names = "msi";
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0x7>;
-		interrupt-map = <0 0 0 1 &intc 0 36 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
-				<0 0 0 2 &intc 0 37 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
-				<0 0 0 3 &intc 0 38 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
-				<0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
-		clocks = <&gcc PCIE_A_CLK>,
-			 <&gcc PCIE_H_CLK>,
-			 <&gcc PCIE_PHY_CLK>,
-			 <&gcc PCIE_AUX_CLK>,
-			 <&gcc PCIE_ALT_REF_CLK>;
-		clock-names = "core", "iface", "phy", "aux", "ref";
-		resets = <&gcc PCIE_ACLK_RESET>,
-			 <&gcc PCIE_HCLK_RESET>,
-			 <&gcc PCIE_POR_RESET>,
-			 <&gcc PCIE_PCI_RESET>,
-			 <&gcc PCIE_PHY_RESET>,
-			 <&gcc PCIE_EXT_RESET>;
-		reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
-		pinctrl-0 = <&pcie_pins_default>;
-		pinctrl-names = "default";
-	};
-
-* Example for apq8084
-	pcie0@fc520000 {
-		compatible = "qcom,pcie-apq8084", "snps,dw-pcie";
-		reg = <0xfc520000 0x2000>,
-		      <0xff000000 0x1000>,
-		      <0xff001000 0x1000>,
-		      <0xff002000 0x2000>;
-		reg-names = "parf", "dbi", "elbi", "config";
-		device_type = "pci";
-		linux,pci-domain = <0>;
-		bus-range = <0x00 0xff>;
-		num-lanes = <1>;
-		#address-cells = <3>;
-		#size-cells = <2>;
-		ranges = <0x81000000 0 0          0xff200000 0 0x00100000   /* I/O */
-			  0x82000000 0 0x00300000 0xff300000 0 0x00d00000>; /* memory */
-		interrupts = <GIC_SPI 243 IRQ_TYPE_NONE>;
-		interrupt-names = "msi";
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0x7>;
-		interrupt-map = <0 0 0 1 &intc 0 244 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
-				<0 0 0 2 &intc 0 245 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
-				<0 0 0 3 &intc 0 247 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
-				<0 0 0 4 &intc 0 248 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
-		clocks = <&gcc GCC_PCIE_0_CFG_AHB_CLK>,
-			 <&gcc GCC_PCIE_0_MSTR_AXI_CLK>,
-			 <&gcc GCC_PCIE_0_SLV_AXI_CLK>,
-			 <&gcc GCC_PCIE_0_AUX_CLK>;
-		clock-names = "iface", "master_bus", "slave_bus", "aux";
-		resets = <&gcc GCC_PCIE_0_BCR>;
-		reset-names = "core";
-		power-domains = <&gcc PCIE0_GDSC>;
-		vdda-supply = <&pma8084_l3>;
-		phys = <&pciephy0>;
-		phy-names = "pciephy";
-		perst-gpio = <&tlmm 70 GPIO_ACTIVE_LOW>;
-		pinctrl-0 = <&pcie0_pins_default>;
-		pinctrl-names = "default";
-	};
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
new file mode 100644
index 000000000000..89a1021df9bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -0,0 +1,607 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/qcom,pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm PCI express root complex
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+  - Stanimir Varbanov <svarbanov@mm-sol.com>
+
+description: |
+  Qualcomm PCIe root complex controller is bansed on the Synopsys DesignWare
+  PCIe IP.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,pcie-ipq8064
+          - qcom,pcie-ipq8064-v2
+          - qcom,pcie-apq8064
+          - qcom,pcie-apq8084
+          - qcom,pcie-msm8996
+          - qcom,pcie-ipq4019
+          - qcom,pcie-ipq8074
+          - qcom,pcie-qcs404
+          - qcom,pcie-sc8180x
+          - qcom,pcie-sdm845
+          - qcom,pcie-sm8250
+          - qcom,pcie-sm8450-pcie0
+          - qcom,pcie-sm8450-pcie1
+          - qcom,pcie-ipq6018
+      - const: snps,dw-pcie
+
+  reg:
+    minItems: 4
+    maxItems: 5
+
+  reg-names:
+    minItems: 4
+    maxItems: 5
+    items:
+      enum:
+        - parf # Qualcomm specific registers
+        - dbi # DesignWare PCIe registers
+        - elbi # External local bus interface registers
+        - config # PCIe configuration space
+        - atu # ATU address space (optional)
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    items:
+      - const: "msi"
+
+  clocks: true
+
+  vdda-supply:
+    description: A phandle to the core analog power supply
+
+  vdda_phy-supply:
+    description: A phandle to the core analog power supply for PHY
+
+  vdda_refclk-supply:
+    description: A phandle to the core analog power supply for IC which generates reference clock
+
+  vddpe-3v3-supply:
+    description: A phandle to the PCIe endpoint power supply
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    items:
+      - const: "pciephy"
+
+  perst-gpio:
+    description: GPIO pin number of PERST# signal
+    maxItems: 1
+    deprecated: true
+
+  perst-gpios:
+    description: GPIO controlled connection to PERST# signal
+    maxItems: 1
+
+  wake-gpio:
+    description: GPIO pin number of WAKE# signal
+    maxItems: 1
+    deprecated: true
+
+  wake-gpios:
+    description: GPIO controlled connection to WAKE# signal
+    maxItems: 1
+
+  iommu-map: true
+  iommu-map-mask: true
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8064
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          items:
+            - const: core # Clocks the pcie hw block
+            - const: iface # Configuration AHB clock
+            - const: phy # Clocks the pcie PHY block
+        resets:
+          minItems: 5
+          maxItems: 5
+        reset-names:
+          items:
+            - const: axi # AXI reset
+            - const: ahb # AHB reset
+            - const: por # POR reset
+            - const: pci # PCI reset
+            - const: phy # PHY reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-ipq8064
+              - qcom,pcie-ipq8064v2
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 5
+        clock-names:
+          items:
+            - const: core # Clocks the pcie hw block
+            - const: iface # Configuration AHB clock
+            - const: phy # Clocks the pcie PHY block
+            - const: aux # Clocks the pcie AUX block
+            - const: ref # Clocks the pcie ref block
+        resets:
+          minItems: 6
+          maxItems: 6
+        reset-names:
+          items:
+            - const: axi # AXI reset
+            - const: ahb # AHB reset
+            - const: por # POR reset
+            - const: pci # PCI reset
+            - const: phy # PHY reset
+            - const: ext # EXT reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8084
+    then:
+      properties:
+        clocks:
+          minItems: 4
+          maxItems: 4
+        clock-names:
+          items:
+            - const: iface # Configuration AHB clock
+            - const: master_bus # Master AXI clock
+            - const: slave_bus # Slave AXI clock
+            - const: aux # Auxiliary (AUX) clock
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: core # Core reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-ipq4019
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          items:
+            - const: aux # Auxiliary (AUX) clock
+            - const: master_bus # Master AXI clock
+            - const: slave_bus # Slave AXI clock
+        resets:
+          minItems: 12
+          maxItems: 12
+        reset-names:
+          items:
+            - const: axi_m # AXI master reset
+            - const: axi_s # AXI slave reset
+            - const: pipe # PIPE reset
+            - const: axi_m_vmid # VMID reset
+            - const: axi_s_xpu # XPU reset
+            - const: parf # PARF reset
+            - const: phy # PHY reset
+            - const: axi_m_sticky # AXI sticky reset
+            - const: pipe_sticky # PIPE sticky reset
+            - const: pwr # PWR reset
+            - const: ahb # AHB reset
+            - const: phy_ahb # PHY AHB reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-msm8996
+    then:
+      oneOf:
+        - properties:
+            clock-names:
+              items:
+                - const: pipe # Pipe Clock driving internal logic
+                - const: aux # Auxiliary (AUX) clock
+                - const: cfg # Configuration clock
+                - const: bus_master # Master AXI clock
+                - const: bus_slave # Slave AXI clock
+        - properties:
+            clock-names:
+              items:
+                - const: pipe # Pipe Clock driving internal logic
+                - const: bus_master # Master AXI clock
+                - const: bus_slave # Slave AXI clock
+                - const: cfg # Configuration clock
+                - const: aux # Auxiliary (AUX) clock
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 5
+        resets: false
+        reset-names: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-ipq8074
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 5
+        clock-names:
+          items:
+            - const: iface # PCIe to SysNOC BIU clock
+            - const: axi_m # AXI Master clock
+            - const: axi_s # AXI Slave clock
+            - const: ahb # AHB clock
+            - const: aux # Auxiliary clock
+        resets:
+          minItems: 7
+          maxItems: 7
+        reset-names:
+          items:
+            - const: pipe # PIPE reset
+            - const: sleep # Sleep reset
+            - const: sticky # Core Sticky reset
+            - const: axi_m # AXI Master reset
+            - const: axi_s # AXI Slave reset
+            - const: ahb # AHB Reset
+            - const: axi_m_sticky # AXI Master Sticky reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-ipq6018
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 5
+        clock-names:
+          items:
+            - const: iface # PCIe to SysNOC BIU clock
+            - const: axi_m # AXI Master clock
+            - const: axi_s # AXI Slave clock
+            - const: axi_bridge # AXI bridge clock
+            - const: rchng
+        resets:
+          minItems: 8
+          maxItems: 8
+        reset-names:
+          items:
+            - const: pipe # PIPE reset
+            - const: sleep # Sleep reset
+            - const: sticky # Core Sticky reset
+            - const: axi_m # AXI Master reset
+            - const: axi_s # AXI Slave reset
+            - const: ahb # AHB Reset
+            - const: axi_m_sticky # AXI Master Sticky reset
+            - const: axi_s_sticky # AXI Slave Sticky reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-qcs404
+    then:
+      properties:
+        clocks:
+          minItems: 4
+          maxItems: 4
+        clock-names:
+          items:
+            - const: iface # AHB clock
+            - const: aux # Auxiliary clock
+            - const: master_bus # AXI Master clock
+            - const: slave_bus # AXI Slave clock
+        resets:
+          minItems: 6
+          maxItems: 6
+        reset-names:
+          items:
+            - const: axi_m # AXI Master reset
+            - const: axi_s # AXI Slave reset
+            - const: axi_m_sticky # AXI Master Sticky reset
+            - const: pipe_sticky # PIPE sticky reset
+            - const: pwr # PWR reset
+            - const: ahb # AHB reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sdm845
+    then:
+      oneOf:
+          # Unfortunately the "optional" ref clock is used in the middle of the list
+        - properties:
+            clocks:
+              minItems: 8
+              maxItems: 8
+            clock-names:
+              items:
+                - const: pipe # PIPE clock
+                - const: aux # Auxiliary clock
+                - const: cfg # Configuration clock
+                - const: bus_master # Master AXI clock
+                - const: bus_slave # Slave AXI clock
+                - const: slave_q2a # Slave Q2A clock
+                - const: ref # REFERENCE clock
+                - const: tbu # PCIe TBU clock
+        - properties:
+            clocks:
+              minItems: 7
+              maxItems: 7
+            clock-names:
+              items:
+                - const: pipe # PIPE clock
+                - const: aux # Auxiliary clock
+                - const: cfg # Configuration clock
+                - const: bus_master # Master AXI clock
+                - const: bus_slave # Slave AXI clock
+                - const: slave_q2a # Slave Q2A clock
+                - const: tbu # PCIe TBU clock
+      properties:
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: pci # PCIe core reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sc8180x
+              - qcom,pcie-sm8250
+    then:
+      oneOf:
+          # Unfortunately the "optional" ref clock is used in the middle of the list
+        - properties:
+            clocks:
+              minItems: 9
+              maxItems: 9
+            clock-names:
+              items:
+                - const: pipe # PIPE clock
+                - const: aux # Auxiliary clock
+                - const: cfg # Configuration clock
+                - const: bus_master # Master AXI clock
+                - const: bus_slave # Slave AXI clock
+                - const: slave_q2a # Slave Q2A clock
+                - const: ref # REFERENCE clock
+                - const: tbu # PCIe TBU clock
+                - const: ddrss_sf_tbu # PCIe SF TBU clock
+        - properties:
+            clocks:
+              minItems: 8
+              maxItems: 8
+            clock-names:
+              items:
+                - const: pipe # PIPE clock
+                - const: aux # Auxiliary clock
+                - const: cfg # Configuration clock
+                - const: bus_master # Master AXI clock
+                - const: bus_slave # Slave AXI clock
+                - const: slave_q2a # Slave Q2A clock
+                - const: tbu # PCIe TBU clock
+                - const: ddrss_sf_tbu # PCIe SF TBU clock
+      properties:
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: pci # PCIe core reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sm8450-pcie0
+    then:
+      properties:
+        clocks:
+          minItems: 12
+          maxItems: 12
+        clock-names:
+          items:
+            - const: pipe # PIPE clock
+            - const: pipe_mux # PIPE MUX
+            - const: phy_pipe # PIPE output clock
+            - const: ref # REFERENCE clock
+            - const: aux # Auxiliary clock
+            - const: cfg # Configuration clock
+            - const: bus_master # Master AXI clock
+            - const: bus_slave # Slave AXI clock
+            - const: slave_q2a # Slave Q2A clock
+            - const: ddrss_sf_tbu # PCIe SF TBU clock
+            - const: aggre0 # Aggre NoC PCIe0 AXI clock
+            - const: aggre1 # Aggre NoC PCIe1 AXI clock
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: pci # PCIe core reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sm8450-pcie1
+    then:
+      properties:
+        clocks:
+          minItems: 11
+          maxItems: 11
+        clock-names:
+          items:
+            - const: pipe # PIPE clock
+            - const: pipe_mux # PIPE MUX
+            - const: phy_pipe # PIPE output clock
+            - const: ref # REFERENCE clock
+            - const: aux # Auxiliary clock
+            - const: cfg # Configuration clock
+            - const: bus_master # Master AXI clock
+            - const: bus_slave # Slave AXI clock
+            - const: slave_q2a # Slave Q2A clock
+            - const: ddrss_sf_tbu # PCIe SF TBU clock
+            - const: aggre1 # Aggre NoC PCIe1 AXI clock
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: pci # PCIe core reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8064
+              - qcom,pcie-ipq8064
+              - qcom,pcie-ipq8064v2
+    then:
+      required:
+        - vdda-supply
+        - vdda_phy-supply
+        - vdda_refclk-supply
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - qcom,pcie-apq8064
+                - qcom,pcie-ipq4019
+                - qcom,pcie-ipq8064
+                - qcom,pcie-ipq8064v2
+                - qcom,pcie-ipq8074
+                - qcom,pcie-qcs404
+    then:
+      properties:
+        power-domains:
+          minItems: 1
+          maxItems: 1
+      required:
+        - power-domains
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pcie@1b500000 {
+      compatible = "qcom,pcie-ipq8064", "snps,dw-pcie";
+      reg = <0x1b500000 0x1000
+             0x1b502000 0x80
+             0x1b600000 0x100
+             0x0ff00000 0x100000>;
+      reg-names = "dbi", "elbi", "parf", "config";
+      device_type = "pci";
+      linux,pci-domain = <0>;
+      bus-range = <0x00 0xff>;
+      num-lanes = <1>;
+      #address-cells = <3>;
+      #size-cells = <2>;
+      ranges = <0x81000000 0 0 0x0fe00000 0 0x00100000>, 
+               <0x82000000 0 0 0x08000000 0 0x07e00000>; 
+      interrupts = <GIC_SPI 238 IRQ_TYPE_NONE>;
+      interrupt-names = "msi";
+      #interrupt-cells = <1>;
+      interrupt-map-mask = <0 0 0 0x7>;
+      interrupt-map = <0 0 0 1 &intc 0 36 IRQ_TYPE_LEVEL_HIGH>, 
+                      <0 0 0 2 &intc 0 37 IRQ_TYPE_LEVEL_HIGH>, 
+                      <0 0 0 3 &intc 0 38 IRQ_TYPE_LEVEL_HIGH>, 
+                      <0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>; 
+      clocks = <&gcc 41>,
+               <&gcc 43>,
+               <&gcc 44>,
+               <&gcc 42>,
+               <&gcc 248>;
+      clock-names = "core", "iface", "phy", "aux", "ref";
+      resets = <&gcc 27>,
+               <&gcc 26>,
+               <&gcc 25>,
+               <&gcc 24>,
+               <&gcc 23>,
+               <&gcc 22>;
+      reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
+      pinctrl-0 = <&pcie_pins_default>;
+      pinctrl-names = "default";
+      vdda-supply = <&pm8921_s3>;
+      vdda_phy-supply = <&pm8921_lvs6>;
+      vdda_refclk-supply = <&ext_3p3v>;
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/gpio/gpio.h>
+    pcie@fc520000 {
+      compatible = "qcom,pcie-apq8084", "snps,dw-pcie";
+      reg = <0xfc520000 0x2000>,
+            <0xff000000 0x1000>,
+            <0xff001000 0x1000>,
+            <0xff002000 0x2000>;
+      reg-names = "parf", "dbi", "elbi", "config";
+      device_type = "pci";
+      linux,pci-domain = <0>;
+      bus-range = <0x00 0xff>;
+      num-lanes = <1>;
+      #address-cells = <3>;
+      #size-cells = <2>;
+      ranges = <0x81000000 0 0          0xff200000 0 0x00100000>, 
+               <0x82000000 0 0x00300000 0xff300000 0 0x00d00000>; 
+      interrupts = <GIC_SPI 243 IRQ_TYPE_NONE>;
+      interrupt-names = "msi";
+      #interrupt-cells = <1>;
+      interrupt-map-mask = <0 0 0 0x7>;
+      interrupt-map = <0 0 0 1 &intc 0 244 IRQ_TYPE_LEVEL_HIGH>, 
+                      <0 0 0 2 &intc 0 245 IRQ_TYPE_LEVEL_HIGH>, 
+                      <0 0 0 3 &intc 0 247 IRQ_TYPE_LEVEL_HIGH>, 
+                      <0 0 0 4 &intc 0 248 IRQ_TYPE_LEVEL_HIGH>; 
+      clocks = <&gcc 324>,
+               <&gcc 325>,
+               <&gcc 327>,
+               <&gcc 323>;
+      clock-names = "iface", "master_bus", "slave_bus", "aux";
+      resets = <&gcc 81>;
+      reset-names = "core";
+      power-domains = <&gcc 1>;
+      vdda-supply = <&pma8084_l3>;
+      phys = <&pciephy0>;
+      phy-names = "pciephy";
+      perst-gpio = <&tlmm 70 GPIO_ACTIVE_LOW>;
+      pinctrl-0 = <&pcie0_pins_default>;
+      pinctrl-names = "default";
+    };
+...
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/6] dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
  2022-04-22 11:48 ` [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML Dmitry Baryshkov
@ 2022-04-22 11:48 ` Dmitry Baryshkov
  2022-04-22 13:11   ` Krzysztof Kozlowski
  2022-04-22 11:48 ` [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly Dmitry Baryshkov
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Add support for sc7280-specific clock and reset definitions.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 89a1021df9bc..7210057d1511 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -26,6 +26,7 @@ properties:
           - qcom,pcie-ipq4019
           - qcom,pcie-ipq8074
           - qcom,pcie-qcs404
+          - qcom,pcie-sc7280
           - qcom,pcie-sc8180x
           - qcom,pcie-sdm845
           - qcom,pcie-sm8250
@@ -337,6 +338,35 @@ allOf:
             - const: pipe_sticky # PIPE sticky reset
             - const: pwr # PWR reset
             - const: ahb # AHB reset
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sc7280
+    then:
+      properties:
+        clocks:
+          minItems: 11
+          maxItems: 11
+        clock-names:
+          items:
+            - const: pipe # PIPE clock
+            - const: pipe_mux # PIPE MUX
+            - const: phy_pipe # PIPE output clock
+            - const: ref # REFERENCE clock
+            - const: aux # Auxiliary clock
+            - const: cfg # Configuration clock
+            - const: bus_master # Master AXI clock
+            - const: bus_slave # Slave AXI clock
+            - const: slave_q2a # Slave Q2A clock
+            - const: tbu # PCIe TBU clock
+            - const: ddrss_sf_tbu # PCIe SF TBU clock
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: pci # PCIe core reset
   - if:
       properties:
         compatible:
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
  2022-04-22 11:48 ` [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML Dmitry Baryshkov
  2022-04-22 11:48 ` [PATCH 2/6] dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset Dmitry Baryshkov
@ 2022-04-22 11:48 ` Dmitry Baryshkov
  2022-04-22 12:55   ` Krzysztof Kozlowski
  2022-04-22 11:48 ` [PATCH 4/6] dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback Dmitry Baryshkov
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Instead of specifying the enum of possible reg-names, specify them
explicitly. This allows us to specify which chipsets need the "atu"
regions, which do not. Also it clearly describes which platforms
enumerate PCIe cores using the dbi region and which use parf region for
that.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 96 ++++++++++++++++---
 1 file changed, 81 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 7210057d1511..e78e63ea4b25 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -35,21 +35,6 @@ properties:
           - qcom,pcie-ipq6018
       - const: snps,dw-pcie
 
-  reg:
-    minItems: 4
-    maxItems: 5
-
-  reg-names:
-    minItems: 4
-    maxItems: 5
-    items:
-      enum:
-        - parf # Qualcomm specific registers
-        - dbi # DesignWare PCIe registers
-        - elbi # External local bus interface registers
-        - config # PCIe configuration space
-        - atu # ATU address space (optional)
-
   interrupts:
     maxItems: 1
 
@@ -108,6 +93,87 @@ required:
 
 allOf:
   - $ref: /schemas/pci/pci-bus.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8064
+              - qcom,pcie-ipq4019
+              - qcom,pcie-ipq8064
+              - qcom,pcie-ipq8064v2
+              - qcom,pcie-ipq8074
+              - qcom,pcie-qcs404
+    then:
+      properties:
+        reg:
+          minItems: 4
+          maxItems: 4
+        reg-names:
+          items:
+            - const: dbi # DesignWare PCIe registers
+            - const: elbi # External local bus interface registers
+            - const: parf # Qualcomm specific registers
+            - const: config # PCIe configuration space
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-ipq6018
+    then:
+      properties:
+        reg:
+          minItems: 5
+          maxItems: 5
+        reg-names:
+          items:
+            - const: dbi # DesignWare PCIe registers
+            - const: elbi # External local bus interface registers
+            - const: atu # ATU address space (optional)
+            - const: parf # Qualcomm specific registers
+            - const: config # PCIe configuration space
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8084
+              - qcom,pcie-msm8996
+              - qcom,pcie-sdm845
+    then:
+      properties:
+        reg:
+          minItems: 4
+          maxItems: 4
+        reg-names:
+          items:
+            - const: parf # Qualcomm specific registers
+            - const: dbi # DesignWare PCIe registers
+            - const: elbi # External local bus interface registers
+            - const: config # PCIe configuration space
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sc7280
+              - qcom,pcie-sc8180x
+              - qcom,pcie-sm8250
+              - qcom,pcie-sm8450-pcie0
+              - qcom,pcie-sm8450-pcie1
+    then:
+      properties:
+        reg:
+          minItems: 5
+          maxItems: 5
+        reg-names:
+          items:
+            - const: parf # Qualcomm specific registers
+            - const: dbi # DesignWare PCIe registers
+            - const: elbi # External local bus interface registers
+            - const: atu # ATU address space (optional)
+            - const: config # PCIe configuration space
   - if:
       properties:
         compatible:
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/6] dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
                   ` (2 preceding siblings ...)
  2022-04-22 11:48 ` [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly Dmitry Baryshkov
@ 2022-04-22 11:48 ` Dmitry Baryshkov
  2022-04-22 13:13   ` Krzysztof Kozlowski
  2022-04-22 11:48 ` [PATCH 5/6] arm64: dts: qcom: stop using snps,dw-pcie falback Dmitry Baryshkov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Qualcomm PCIe devices are not really compatible with the snps,dw-pcie.
Unlike the generic IP core, they have special requirements regarding
enabling clocks, toggling resets, using the PHY, etc.

This is not to mention that platform snps-dw-pcie driver expects to find
two IRQs declared, while Qualcomm platforms use just one.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 38 +++++++++----------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index e78e63ea4b25..31c11a9f716e 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -16,24 +16,22 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - qcom,pcie-ipq8064
-          - qcom,pcie-ipq8064-v2
-          - qcom,pcie-apq8064
-          - qcom,pcie-apq8084
-          - qcom,pcie-msm8996
-          - qcom,pcie-ipq4019
-          - qcom,pcie-ipq8074
-          - qcom,pcie-qcs404
-          - qcom,pcie-sc7280
-          - qcom,pcie-sc8180x
-          - qcom,pcie-sdm845
-          - qcom,pcie-sm8250
-          - qcom,pcie-sm8450-pcie0
-          - qcom,pcie-sm8450-pcie1
-          - qcom,pcie-ipq6018
-      - const: snps,dw-pcie
+    enum:
+      - qcom,pcie-ipq8064
+      - qcom,pcie-ipq8064-v2
+      - qcom,pcie-apq8064
+      - qcom,pcie-apq8084
+      - qcom,pcie-msm8996
+      - qcom,pcie-ipq4019
+      - qcom,pcie-ipq8074
+      - qcom,pcie-qcs404
+      - qcom,pcie-sc7280
+      - qcom,pcie-sc8180x
+      - qcom,pcie-sdm845
+      - qcom,pcie-sm8250
+      - qcom,pcie-sm8450-pcie0
+      - qcom,pcie-sm8450-pcie1
+      - qcom,pcie-ipq6018
 
   interrupts:
     maxItems: 1
@@ -618,7 +616,7 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     pcie@1b500000 {
-      compatible = "qcom,pcie-ipq8064", "snps,dw-pcie";
+      compatible = "qcom,pcie-ipq8064";
       reg = <0x1b500000 0x1000
              0x1b502000 0x80
              0x1b600000 0x100
@@ -663,7 +661,7 @@ examples:
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/gpio/gpio.h>
     pcie@fc520000 {
-      compatible = "qcom,pcie-apq8084", "snps,dw-pcie";
+      compatible = "qcom,pcie-apq8084";
       reg = <0xfc520000 0x2000>,
             <0xff000000 0x1000>,
             <0xff001000 0x1000>,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/6] arm64: dts: qcom: stop using snps,dw-pcie falback
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
                   ` (3 preceding siblings ...)
  2022-04-22 11:48 ` [PATCH 4/6] dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback Dmitry Baryshkov
@ 2022-04-22 11:48 ` Dmitry Baryshkov
  2022-04-22 13:20   ` Krzysztof Kozlowski
  2022-04-22 11:48 ` [PATCH 6/6] arm: " Dmitry Baryshkov
  2022-04-22 13:19 ` [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Krzysztof Kozlowski
  6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Qualcomm PCIe devices are not really compatible with the snps,dw-pcie.
Unlike the generic IP core, they have special requirements regarding
enabling clocks, toggling resets, using the PHY, etc.

This is not to mention that platform snps-dw-pcie driver expects to find
two IRQs declared, while Qualcomm platforms use just one.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 6 +++---
 arch/arm64/boot/dts/qcom/qcs404.dtsi  | 2 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi  | 4 ++--
 arch/arm64/boot/dts/qcom/sm8250.dtsi  | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index f0f81c23c16f..b577b9046938 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1574,7 +1574,7 @@ agnoc@0 {
 			ranges;
 
 			pcie0: pcie@600000 {
-				compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
+				compatible = "qcom,pcie-msm8996";
 				status = "disabled";
 				power-domains = <&gcc PCIE0_GDSC>;
 				bus-range = <0x00 0xff>;
@@ -1626,7 +1626,7 @@ pcie0: pcie@600000 {
 			};
 
 			pcie1: pcie@608000 {
-				compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
+				compatible = "qcom,pcie-msm8996";
 				power-domains = <&gcc PCIE1_GDSC>;
 				bus-range = <0x00 0xff>;
 				num-lanes = <1>;
@@ -1679,7 +1679,7 @@ pcie1: pcie@608000 {
 			};
 
 			pcie2: pcie@610000 {
-				compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
+				compatible = "qcom,pcie-msm8996";
 				power-domains = <&gcc PCIE2_GDSC>;
 				bus-range = <0x00 0xff>;
 				num-lanes = <1>;
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 3f06f7cd3cf2..2386081463e3 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -1280,7 +1280,7 @@ glink-edge {
 		};
 
 		pcie: pci@10000000 {
-			compatible = "qcom,pcie-qcs404", "snps,dw-pcie";
+			compatible = "qcom,pcie-qcs404";
 			reg =  <0x10000000 0xf1d>,
 			       <0x10000f20 0xa8>,
 			       <0x07780000 0x2000>,
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index b31bf62e8680..85dfa0842003 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2027,7 +2027,7 @@ llcc: system-cache-controller@1100000 {
 		};
 
 		pcie0: pci@1c00000 {
-			compatible = "qcom,pcie-sdm845", "snps,dw-pcie";
+			compatible = "qcom,pcie-sdm845";
 			reg = <0 0x01c00000 0 0x2000>,
 			      <0 0x60000000 0 0xf1d>,
 			      <0 0x60000f20 0 0xa8>,
@@ -2132,7 +2132,7 @@ pcie0_lane: phy@1c06200 {
 		};
 
 		pcie1: pci@1c08000 {
-			compatible = "qcom,pcie-sdm845", "snps,dw-pcie";
+			compatible = "qcom,pcie-sdm845";
 			reg = <0 0x01c08000 0 0x2000>,
 			      <0 0x40000000 0 0xf1d>,
 			      <0 0x40000f20 0 0xa8>,
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index af8f22636436..410272a1e19b 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1789,7 +1789,7 @@ mmss_noc: interconnect@1740000 {
 		};
 
 		pcie0: pci@1c00000 {
-			compatible = "qcom,pcie-sm8250", "snps,dw-pcie";
+			compatible = "qcom,pcie-sm8250";
 			reg = <0 0x01c00000 0 0x3000>,
 			      <0 0x60000000 0 0xf1d>,
 			      <0 0x60000f20 0 0xa8>,
@@ -1888,7 +1888,7 @@ pcie0_lane: phy@1c06200 {
 		};
 
 		pcie1: pci@1c08000 {
-			compatible = "qcom,pcie-sm8250", "snps,dw-pcie";
+			compatible = "qcom,pcie-sm8250";
 			reg = <0 0x01c08000 0 0x3000>,
 			      <0 0x40000000 0 0xf1d>,
 			      <0 0x40000f20 0 0xa8>,
@@ -1994,7 +1994,7 @@ pcie1_lane: phy@1c0e200 {
 		};
 
 		pcie2: pci@1c10000 {
-			compatible = "qcom,pcie-sm8250", "snps,dw-pcie";
+			compatible = "qcom,pcie-sm8250";
 			reg = <0 0x01c10000 0 0x3000>,
 			      <0 0x64000000 0 0xf1d>,
 			      <0 0x64000f20 0 0xa8>,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/6] arm: dts: qcom: stop using snps,dw-pcie falback
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
                   ` (4 preceding siblings ...)
  2022-04-22 11:48 ` [PATCH 5/6] arm64: dts: qcom: stop using snps,dw-pcie falback Dmitry Baryshkov
@ 2022-04-22 11:48 ` Dmitry Baryshkov
  2022-04-22 13:19   ` Krzysztof Kozlowski
  2022-04-22 13:19 ` [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Krzysztof Kozlowski
  6 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 11:48 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

Qualcomm PCIe devices are not really compatible with the snps,dw-pcie.
Unlike the generic IP core, they have special requirements regarding
enabling clocks, toggling resets, using the PHY, etc.

This is not to mention that platform snps-dw-pcie driver expects to find
two IRQs declared, while Qualcomm platforms use just one.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8064.dtsi | 2 +-
 arch/arm/boot/dts/qcom-ipq4019.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index a1c8ae516d21..ec2f98671a8c 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -1370,7 +1370,7 @@ gfx3d1: iommu@7d00000 {
 		};
 
 		pcie: pci@1b500000 {
-			compatible = "qcom,pcie-apq8064", "snps,dw-pcie";
+			compatible = "qcom,pcie-apq8064";
 			reg = <0x1b500000 0x1000>,
 			      <0x1b502000 0x80>,
 			      <0x1b600000 0x100>,
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index a9d0566a3190..1e814dbe135e 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -412,7 +412,7 @@ restart@4ab000 {
 		};
 
 		pcie0: pci@40000000 {
-			compatible = "qcom,pcie-ipq4019", "snps,dw-pcie";
+			compatible = "qcom,pcie-ipq4019";
 			reg =  <0x40000000 0xf1d
 				0x40000f20 0xa8
 				0x80000 0x2000
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  2022-04-22 11:48 ` [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly Dmitry Baryshkov
@ 2022-04-22 12:55   ` Krzysztof Kozlowski
  2022-04-22 15:47     ` Dmitry Baryshkov
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 12:55 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Instead of specifying the enum of possible reg-names, specify them
> explicitly. This allows us to specify which chipsets need the "atu"
> regions, which do not. Also it clearly describes which platforms
> enumerate PCIe cores using the dbi region and which use parf region for
> that.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 96 ++++++++++++++++---
>  1 file changed, 81 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 7210057d1511..e78e63ea4b25 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -35,21 +35,6 @@ properties:
>            - qcom,pcie-ipq6018
>        - const: snps,dw-pcie
>  
> -  reg:
> -    minItems: 4
> -    maxItems: 5

This should stay.

> -
> -  reg-names:
> -    minItems: 4
> -    maxItems: 5
> -    items:
> -      enum:
> -        - parf # Qualcomm specific registers
> -        - dbi # DesignWare PCIe registers
> -        - elbi # External local bus interface registers
> -        - config # PCIe configuration space
> -        - atu # ATU address space (optional)

Move one of your lists for specific compatibles here and name last
element optional (minItems: 4).

You will need to fix the order of regs in DTS to match the one defined here.

> -
>    interrupts:
>      maxItems: 1
>  
> @@ -108,6 +93,87 @@ required:
>  
>  allOf:
>    - $ref: /schemas/pci/pci-bus.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-apq8064
> +              - qcom,pcie-ipq4019
> +              - qcom,pcie-ipq8064
> +              - qcom,pcie-ipq8064v2
> +              - qcom,pcie-ipq8074
> +              - qcom,pcie-qcs404
> +    then:
> +      properties:
> +        reg:
> +          minItems: 4
> +          maxItems: 4

Only maxItems: 4

> +        reg-names:
> +          items:
> +            - const: dbi # DesignWare PCIe registers
> +            - const: elbi # External local bus interface registers
> +            - const: parf # Qualcomm specific registers
> +            - const: config # PCIe configuration space

No need for this, instead only maxItems:4

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-ipq6018
> +    then:
> +      properties:
> +        reg:
> +          minItems: 5
> +          maxItems: 5

Only minItems:5 should be needed.

> +        reg-names:
> +          items:
> +            - const: dbi # DesignWare PCIe registers
> +            - const: elbi # External local bus interface registers
> +            - const: atu # ATU address space (optional)
> +            - const: parf # Qualcomm specific registers
> +            - const: config # PCIe configuration space

This can be removed.

All other cases should be merged with the ones here.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML
  2022-04-22 11:48 ` [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML Dmitry Baryshkov
@ 2022-04-22 13:10   ` Krzysztof Kozlowski
  2022-04-22 16:49     ` Dmitry Baryshkov
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 13:10 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Changes to examples:
>  - Inline clock and reset numbers rather than including dt-bindings
>    files because of conflicts between the headers
>  - Split ranges properties to follow current practice
> 
> Changes to the schema:
>  - Fixed the ordering of clock-names/reset-names according to
>    the dtsi files.
>  - Mark vdda-supply as required only for apq/ipq8064 (as it was marked
>    as generally required in the txt file).
> 
> Note: while it was not clearly described in text schema, the majority of
> Qualcomm platforms follow the snps,dw-pcie schema and use two
> compatibility strings in the DT files: platform-specific one and a
> fallback to the generic snps,dw-pcie one. This will be sorted out in the
> next patches.

I don't get why you add snps,dw-pcie fallback here, even though original
bindings (except examples, which are not bindings) were not mentioning
it. Maybe just skip it?

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../devicetree/bindings/pci/qcom,pcie.txt     | 397 ------------
>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 607 ++++++++++++++++++
>  2 files changed, 607 insertions(+), 397 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
>  create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> 

(...)

> -	};
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> new file mode 100644
> index 000000000000..89a1021df9bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -0,0 +1,607 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/qcom,pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm PCI express root complex
> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +  - Stanimir Varbanov <svarbanov@mm-sol.com>
> +
> +description: |
> +  Qualcomm PCIe root complex controller is bansed on the Synopsys DesignWare
> +  PCIe IP.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,pcie-ipq8064
> +          - qcom,pcie-ipq8064-v2
> +          - qcom,pcie-apq8064
> +          - qcom,pcie-apq8084
> +          - qcom,pcie-msm8996
> +          - qcom,pcie-ipq4019
> +          - qcom,pcie-ipq8074
> +          - qcom,pcie-qcs404
> +          - qcom,pcie-sc8180x
> +          - qcom,pcie-sdm845
> +          - qcom,pcie-sm8250
> +          - qcom,pcie-sm8450-pcie0
> +          - qcom,pcie-sm8450-pcie1
> +          - qcom,pcie-ipq6018
> +      - const: snps,dw-pcie
> +
> +  reg:
> +    minItems: 4
> +    maxItems: 5
> +
> +  reg-names:
> +    minItems: 4
> +    maxItems: 5
> +    items:
> +      enum:
> +        - parf # Qualcomm specific registers
> +        - dbi # DesignWare PCIe registers
> +        - elbi # External local bus interface registers
> +        - config # PCIe configuration space
> +        - atu # ATU address space (optional)> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    items:
> +      - const: "msi"
> +
> +  clocks: true

min/maxItems

same for clock-names

> +
> +  vdda-supply:
> +    description: A phandle to the core analog power supply
> +
> +  vdda_phy-supply:
> +    description: A phandle to the core analog power supply for PHY
> +
> +  vdda_refclk-supply:
> +    description: A phandle to the core analog power supply for IC which generates reference clock
> +
> +  vddpe-3v3-supply:
> +    description: A phandle to the PCIe endpoint power supply
> +
> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    items:
> +      - const: "pciephy"
> +
> +  perst-gpio:
> +    description: GPIO pin number of PERST# signal
> +    maxItems: 1
> +    deprecated: true

Old binding did not have it.

> +
> +  perst-gpios:
> +    description: GPIO controlled connection to PERST# signal
> +    maxItems: 1
> +

You miss here power-domains, resets and reset-names with min/maxItems.

> +  wake-gpio:
> +    description: GPIO pin number of WAKE# signal
> +    maxItems: 1
> +    deprecated: true

Old binding did not have it.

> +
> +  wake-gpios:
> +    description: GPIO controlled connection to WAKE# signal
> +    maxItems: 1
> +
> +  iommu-map: true
> +  iommu-map-mask: true

Not present in old binding. If this is trully needed, mention it in
commit msg.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - interrupt-names

What about interrupt-cells, clocks, clock-names, resets, reset-names?

> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-apq8064
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: core # Clocks the pcie hw block
> +            - const: iface # Configuration AHB clock
> +            - const: phy # Clocks the pcie PHY block
> +        resets:
> +          minItems: 5
> +          maxItems: 5
> +        reset-names:
> +          items:
> +            - const: axi # AXI reset
> +            - const: ahb # AHB reset
> +            - const: por # POR reset
> +            - const: pci # PCI reset
> +            - const: phy # PHY reset

Missing required properties e.g. some supplies.
Plus one blank line.

The same applies below to other ifs.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-ipq8064
> +              - qcom,pcie-ipq8064v2

(...)

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    pcie@1b500000 {
> +      compatible = "qcom,pcie-ipq8064", "snps,dw-pcie";
> +      reg = <0x1b500000 0x1000
> +             0x1b502000 0x80
> +             0x1b600000 0x100
> +             0x0ff00000 0x100000>;

Convert the example to match current bindings, so reg should be split.

> +      reg-names = "dbi", "elbi", "parf", "config";
> +      device_type = "pci";
> +      linux,pci-domain = <0>;
> +      bus-range = <0x00 0xff>;
> +      num-lanes = <1>;
> +      #address-cells = <3>;
> +      #size-cells = <2>;
> +      ranges = <0x81000000 0 0 0x0fe00000 0 0x00100000>, 
> +               <0x82000000 0 0 0x08000000 0 0x07e00000>; 
> +      interrupts = <GIC_SPI 238 IRQ_TYPE_NONE>;

Looks like wrong IRQ flag.

> +      interrupt-names = "msi";
> +      #interrupt-cells = <1>;
> +      interrupt-map-mask = <0 0 0 0x7>;
> +      interrupt-map = <0 0 0 1 &intc 0 36 IRQ_TYPE_LEVEL_HIGH>, 
> +                      <0 0 0 2 &intc 0 37 IRQ_TYPE_LEVEL_HIGH>, 
> +                      <0 0 0 3 &intc 0 38 IRQ_TYPE_LEVEL_HIGH>, 
> +                      <0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>; 
> +      clocks = <&gcc 41>,
> +               <&gcc 43>,
> +               <&gcc 44>,
> +               <&gcc 42>,
> +               <&gcc 248>;
> +      clock-names = "core", "iface", "phy", "aux", "ref";
> +      resets = <&gcc 27>,
> +               <&gcc 26>,
> +               <&gcc 25>,
> +               <&gcc 24>,
> +               <&gcc 23>,
> +               <&gcc 22>;
> +      reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
> +      pinctrl-0 = <&pcie_pins_default>;
> +      pinctrl-names = "default";
> +      vdda-supply = <&pm8921_s3>;
> +      vdda_phy-supply = <&pm8921_lvs6>;
> +      vdda_refclk-supply = <&ext_3p3v>;
> +    };
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    pcie@fc520000 {
> +      compatible = "qcom,pcie-apq8084", "snps,dw-pcie";
> +      reg = <0xfc520000 0x2000>,
> +            <0xff000000 0x1000>,
> +            <0xff001000 0x1000>,
> +            <0xff002000 0x2000>;
> +      reg-names = "parf", "dbi", "elbi", "config";
> +      device_type = "pci";
> +      linux,pci-domain = <0>;
> +      bus-range = <0x00 0xff>;
> +      num-lanes = <1>;
> +      #address-cells = <3>;
> +      #size-cells = <2>;
> +      ranges = <0x81000000 0 0          0xff200000 0 0x00100000>, 
> +               <0x82000000 0 0x00300000 0xff300000 0 0x00d00000>; 
> +      interrupts = <GIC_SPI 243 IRQ_TYPE_NONE>;

Ditto.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/6] dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset
  2022-04-22 11:48 ` [PATCH 2/6] dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset Dmitry Baryshkov
@ 2022-04-22 13:11   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 13:11 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Add support for sc7280-specific clock and reset definitions.

Add it at the end, please. First all the cleanups and changes, then new
devices.

Best regards,
Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/6] dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback
  2022-04-22 11:48 ` [PATCH 4/6] dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback Dmitry Baryshkov
@ 2022-04-22 13:13   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 13:13 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Qualcomm PCIe devices are not really compatible with the snps,dw-pcie.
> Unlike the generic IP core, they have special requirements regarding
> enabling clocks, toggling resets, using the PHY, etc.
> 
> This is not to mention that platform snps-dw-pcie driver expects to find
> two IRQs declared, while Qualcomm platforms use just one.

Removal of fallback is ok, but the original bindings never mentioned
compatibility with snps.

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema
  2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
                   ` (5 preceding siblings ...)
  2022-04-22 11:48 ` [PATCH 6/6] arm: " Dmitry Baryshkov
@ 2022-04-22 13:19 ` Krzysztof Kozlowski
  6 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 13:19 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Convert pci/qcom,pcie schema to YAML description. The first patch
> introduces several warnings which are fixed by the other patches in the
> series.
> 
> Note regarding the snps,dw-pcie compatibility. The Qualcomm PCIe
> controller uses Synopsys PCIe IP core. However it is not just fused to
> the address space. Accessing PCIe registers requires several clocks and
> regulators to be powered up. Thus it can be assumed that the qcom,pcie
> bindings are not fully compatible with the snps,dw-pcie schema.

You can still reference snps schema, if there are no real
imcompatibilities. Few other bindings do like this.

One thing is not being actually compatible with snps but second is being
not compatible with the schema itself, so not being able to re-use
common parts. I think only the first part is true in this case.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/6] arm: dts: qcom: stop using snps,dw-pcie falback
  2022-04-22 11:48 ` [PATCH 6/6] arm: " Dmitry Baryshkov
@ 2022-04-22 13:19   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 13:19 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Qualcomm PCIe devices are not really compatible with the snps,dw-pcie.
> Unlike the generic IP core, they have special requirements regarding
> enabling clocks, toggling resets, using the PHY, etc.
> 
> This is not to mention that platform snps-dw-pcie driver expects to find
> two IRQs declared, while Qualcomm platforms use just one.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm/boot/dts/qcom-apq8064.dtsi | 2 +-
>  arch/arm/boot/dts/qcom-ipq4019.dtsi | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/6] arm64: dts: qcom: stop using snps,dw-pcie falback
  2022-04-22 11:48 ` [PATCH 5/6] arm64: dts: qcom: stop using snps,dw-pcie falback Dmitry Baryshkov
@ 2022-04-22 13:20   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 13:20 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
	Stanimir Varbanov
  Cc: Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> Qualcomm PCIe devices are not really compatible with the snps,dw-pcie.
> Unlike the generic IP core, they have special requirements regarding
> enabling clocks, toggling resets, using the PHY, etc.
> 
> This is not to mention that platform snps-dw-pcie driver expects to find
> two IRQs declared, while Qualcomm platforms use just one.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 6 +++---
>  arch/arm64/boot/dts/qcom/qcs404.dtsi  | 2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi  | 4 ++--
>  arch/arm64/boot/dts/qcom/sm8250.dtsi  | 6 +++---
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  2022-04-22 12:55   ` Krzysztof Kozlowski
@ 2022-04-22 15:47     ` Dmitry Baryshkov
  2022-04-22 15:51       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 15:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, Stanimir Varbanov,
	Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On Fri, 22 Apr 2022 at 15:55, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> > Instead of specifying the enum of possible reg-names, specify them
> > explicitly. This allows us to specify which chipsets need the "atu"
> > regions, which do not. Also it clearly describes which platforms
> > enumerate PCIe cores using the dbi region and which use parf region for
> > that.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../devicetree/bindings/pci/qcom,pcie.yaml    | 96 ++++++++++++++++---
> >  1 file changed, 81 insertions(+), 15 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index 7210057d1511..e78e63ea4b25 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -35,21 +35,6 @@ properties:
> >            - qcom,pcie-ipq6018
> >        - const: snps,dw-pcie
> >
> > -  reg:
> > -    minItems: 4
> > -    maxItems: 5
>
> This should stay.
>
> > -
> > -  reg-names:
> > -    minItems: 4
> > -    maxItems: 5
> > -    items:
> > -      enum:
> > -        - parf # Qualcomm specific registers
> > -        - dbi # DesignWare PCIe registers
> > -        - elbi # External local bus interface registers
> > -        - config # PCIe configuration space
> > -        - atu # ATU address space (optional)
>
> Move one of your lists for specific compatibles here and name last
> element optional (minItems: 4).
>
> You will need to fix the order of regs in DTS to match the one defined here.

I see your idea. I wanted to be explicit, which platforms need atu and
which do not. You'd prefer not to.
Let's probably drop this for now. The bindings proposed in patch 1
work for now. I will work on updating reg-names later.

>
> > -
> >    interrupts:
> >      maxItems: 1
> >
> > @@ -108,6 +93,87 @@ required:
> >
> >  allOf:
> >    - $ref: /schemas/pci/pci-bus.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-apq8064
> > +              - qcom,pcie-ipq4019
> > +              - qcom,pcie-ipq8064
> > +              - qcom,pcie-ipq8064v2
> > +              - qcom,pcie-ipq8074
> > +              - qcom,pcie-qcs404
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 4
> > +          maxItems: 4
>
> Only maxItems: 4
>
> > +        reg-names:
> > +          items:
> > +            - const: dbi # DesignWare PCIe registers
> > +            - const: elbi # External local bus interface registers
> > +            - const: parf # Qualcomm specific registers
> > +            - const: config # PCIe configuration space
>
> No need for this, instead only maxItems:4
>
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-ipq6018
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 5
> > +          maxItems: 5
>
> Only minItems:5 should be needed.
>
> > +        reg-names:
> > +          items:
> > +            - const: dbi # DesignWare PCIe registers
> > +            - const: elbi # External local bus interface registers
> > +            - const: atu # ATU address space (optional)
> > +            - const: parf # Qualcomm specific registers
> > +            - const: config # PCIe configuration space
>
> This can be removed.
>
> All other cases should be merged with the ones here.
>
> Best regards,
> Krzysztof



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  2022-04-22 15:47     ` Dmitry Baryshkov
@ 2022-04-22 15:51       ` Krzysztof Kozlowski
  2022-04-22 19:09         ` Dmitry Baryshkov
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-22 15:51 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, Stanimir Varbanov,
	Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 17:47, Dmitry Baryshkov wrote:
> On Fri, 22 Apr 2022 at 15:55, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 22/04/2022 13:48, Dmitry Baryshkov wrote:
>>> Instead of specifying the enum of possible reg-names, specify them
>>> explicitly. This allows us to specify which chipsets need the "atu"
>>> regions, which do not. Also it clearly describes which platforms
>>> enumerate PCIe cores using the dbi region and which use parf region for
>>> that.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 96 ++++++++++++++++---
>>>  1 file changed, 81 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> index 7210057d1511..e78e63ea4b25 100644
>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>> @@ -35,21 +35,6 @@ properties:
>>>            - qcom,pcie-ipq6018
>>>        - const: snps,dw-pcie
>>>
>>> -  reg:
>>> -    minItems: 4
>>> -    maxItems: 5
>>
>> This should stay.
>>
>>> -
>>> -  reg-names:
>>> -    minItems: 4
>>> -    maxItems: 5
>>> -    items:
>>> -      enum:
>>> -        - parf # Qualcomm specific registers
>>> -        - dbi # DesignWare PCIe registers
>>> -        - elbi # External local bus interface registers
>>> -        - config # PCIe configuration space
>>> -        - atu # ATU address space (optional)
>>
>> Move one of your lists for specific compatibles here and name last
>> element optional (minItems: 4).
>>
>> You will need to fix the order of regs in DTS to match the one defined here.
> 
> I see your idea. I wanted to be explicit, which platforms need atu and
> which do not. You'd prefer not to.

Opposite, I wish platforms to be specific, which need atu which not.
However I wish the strictly defined, same order for everyone because it
looks possible.

> Let's probably drop this for now. The bindings proposed in patch 1
> work for now. I will work on updating reg-names later.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML
  2022-04-22 13:10   ` Krzysztof Kozlowski
@ 2022-04-22 16:49     ` Dmitry Baryshkov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 16:49 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, Stanimir Varbanov,
	Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On Fri, 22 Apr 2022 at 16:10, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/04/2022 13:48, Dmitry Baryshkov wrote:
> > Changes to examples:
> >  - Inline clock and reset numbers rather than including dt-bindings
> >    files because of conflicts between the headers
> >  - Split ranges properties to follow current practice
> >
> > Changes to the schema:
> >  - Fixed the ordering of clock-names/reset-names according to
> >    the dtsi files.
> >  - Mark vdda-supply as required only for apq/ipq8064 (as it was marked
> >    as generally required in the txt file).
> >
> > Note: while it was not clearly described in text schema, the majority of
> > Qualcomm platforms follow the snps,dw-pcie schema and use two
> > compatibility strings in the DT files: platform-specific one and a
> > fallback to the generic snps,dw-pcie one. This will be sorted out in the
> > next patches.
>
> I don't get why you add snps,dw-pcie fallback here, even though original
> bindings (except examples, which are not bindings) were not mentioning
> it. Maybe just skip it?

Ack, I'll squash the snps,dw-pcie patch into this one.

>
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../devicetree/bindings/pci/qcom,pcie.txt     | 397 ------------
> >  .../devicetree/bindings/pci/qcom,pcie.yaml    | 607 ++++++++++++++++++
> >  2 files changed, 607 insertions(+), 397 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
> >  create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >
>
> (...)
>
> > -     };
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > new file mode 100644
> > index 000000000000..89a1021df9bc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -0,0 +1,607 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/qcom,pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm PCI express root complex
> > +
> > +maintainers:
> > +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> > +  - Stanimir Varbanov <svarbanov@mm-sol.com>
> > +
> > +description: |
> > +  Qualcomm PCIe root complex controller is bansed on the Synopsys DesignWare
> > +  PCIe IP.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - qcom,pcie-ipq8064
> > +          - qcom,pcie-ipq8064-v2
> > +          - qcom,pcie-apq8064
> > +          - qcom,pcie-apq8084
> > +          - qcom,pcie-msm8996
> > +          - qcom,pcie-ipq4019
> > +          - qcom,pcie-ipq8074
> > +          - qcom,pcie-qcs404
> > +          - qcom,pcie-sc8180x
> > +          - qcom,pcie-sdm845
> > +          - qcom,pcie-sm8250
> > +          - qcom,pcie-sm8450-pcie0
> > +          - qcom,pcie-sm8450-pcie1
> > +          - qcom,pcie-ipq6018
> > +      - const: snps,dw-pcie
> > +
> > +  reg:
> > +    minItems: 4
> > +    maxItems: 5
> > +
> > +  reg-names:
> > +    minItems: 4
> > +    maxItems: 5
> > +    items:
> > +      enum:
> > +        - parf # Qualcomm specific registers
> > +        - dbi # DesignWare PCIe registers
> > +        - elbi # External local bus interface registers
> > +        - config # PCIe configuration space
> > +        - atu # ATU address space (optional)> +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: "msi"
> > +
> > +  clocks: true
>
> min/maxItems
>
> same for clock-names
>
> > +
> > +  vdda-supply:
> > +    description: A phandle to the core analog power supply
> > +
> > +  vdda_phy-supply:
> > +    description: A phandle to the core analog power supply for PHY
> > +
> > +  vdda_refclk-supply:
> > +    description: A phandle to the core analog power supply for IC which generates reference clock
> > +
> > +  vddpe-3v3-supply:
> > +    description: A phandle to the PCIe endpoint power supply
> > +
> > +  phys:
> > +    maxItems: 1
> > +
> > +  phy-names:
> > +    items:
> > +      - const: "pciephy"
> > +
> > +  perst-gpio:
> > +    description: GPIO pin number of PERST# signal
> > +    maxItems: 1
> > +    deprecated: true
>
> Old binding did not have it.

Ack, dropping.

>
> > +
> > +  perst-gpios:
> > +    description: GPIO controlled connection to PERST# signal
> > +    maxItems: 1
> > +
>
> You miss here power-domains, resets and reset-names with min/maxItems.

power-domains are described later in the non-8064 cases.

Will add everything here.

>
> > +  wake-gpio:
> > +    description: GPIO pin number of WAKE# signal
> > +    maxItems: 1
> > +    deprecated: true
>

Ack dropping.

>
> > +
> > +  wake-gpios:
> > +    description: GPIO controlled connection to WAKE# signal
> > +    maxItems: 1
> > +
> > +  iommu-map: true
> > +  iommu-map-mask: true
>
> Not present in old binding. If this is trully needed, mention it in
> commit msg.

They are used on newer platforms. Will mention it.

>
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - interrupt-names
>
> What about interrupt-cells, clocks, clock-names, resets, reset-names?

Ugh. Missed them, adding back.

>
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-bus.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-apq8064
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 3
> > +          maxItems: 3
> > +        clock-names:
> > +          items:
> > +            - const: core # Clocks the pcie hw block
> > +            - const: iface # Configuration AHB clock
> > +            - const: phy # Clocks the pcie PHY block
> > +        resets:
> > +          minItems: 5
> > +          maxItems: 5
> > +        reset-names:
> > +          items:
> > +            - const: axi # AXI reset
> > +            - const: ahb # AHB reset
> > +            - const: por # POR reset
> > +            - const: pci # PCI reset
> > +            - const: phy # PHY reset
>
> Missing required properties e.g. some supplies.

Ok, I'll merge all apq8064 and ipq8064 entries into a single if/then condition.

> Plus one blank line.
>
> The same applies below to other ifs.
>
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-ipq8064
> > +              - qcom,pcie-ipq8064v2
>
> (...)
>
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    pcie@1b500000 {
> > +      compatible = "qcom,pcie-ipq8064", "snps,dw-pcie";
> > +      reg = <0x1b500000 0x1000
> > +             0x1b502000 0x80
> > +             0x1b600000 0x100
> > +             0x0ff00000 0x100000>;
>
> Convert the example to match current bindings, so reg should be split.

I wonder why the dt_bindings check didn't warn me about it.

>
> > +      reg-names = "dbi", "elbi", "parf", "config";
> > +      device_type = "pci";
> > +      linux,pci-domain = <0>;
> > +      bus-range = <0x00 0xff>;
> > +      num-lanes = <1>;
> > +      #address-cells = <3>;
> > +      #size-cells = <2>;
> > +      ranges = <0x81000000 0 0 0x0fe00000 0 0x00100000>,
> > +               <0x82000000 0 0 0x08000000 0 0x07e00000>;
> > +      interrupts = <GIC_SPI 238 IRQ_TYPE_NONE>;
>
> Looks like wrong IRQ flag.

Ack

>
> > +      interrupt-names = "msi";
> > +      #interrupt-cells = <1>;
> > +      interrupt-map-mask = <0 0 0 0x7>;
> > +      interrupt-map = <0 0 0 1 &intc 0 36 IRQ_TYPE_LEVEL_HIGH>,
> > +                      <0 0 0 2 &intc 0 37 IRQ_TYPE_LEVEL_HIGH>,
> > +                      <0 0 0 3 &intc 0 38 IRQ_TYPE_LEVEL_HIGH>,
> > +                      <0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>;
> > +      clocks = <&gcc 41>,
> > +               <&gcc 43>,
> > +               <&gcc 44>,
> > +               <&gcc 42>,
> > +               <&gcc 248>;
> > +      clock-names = "core", "iface", "phy", "aux", "ref";
> > +      resets = <&gcc 27>,
> > +               <&gcc 26>,
> > +               <&gcc 25>,
> > +               <&gcc 24>,
> > +               <&gcc 23>,
> > +               <&gcc 22>;
> > +      reset-names = "axi", "ahb", "por", "pci", "phy", "ext";
> > +      pinctrl-0 = <&pcie_pins_default>;
> > +      pinctrl-names = "default";
> > +      vdda-supply = <&pm8921_s3>;
> > +      vdda_phy-supply = <&pm8921_lvs6>;
> > +      vdda_refclk-supply = <&ext_3p3v>;
> > +    };
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    pcie@fc520000 {
> > +      compatible = "qcom,pcie-apq8084", "snps,dw-pcie";
> > +      reg = <0xfc520000 0x2000>,
> > +            <0xff000000 0x1000>,
> > +            <0xff001000 0x1000>,
> > +            <0xff002000 0x2000>;
> > +      reg-names = "parf", "dbi", "elbi", "config";
> > +      device_type = "pci";
> > +      linux,pci-domain = <0>;
> > +      bus-range = <0x00 0xff>;
> > +      num-lanes = <1>;
> > +      #address-cells = <3>;
> > +      #size-cells = <2>;
> > +      ranges = <0x81000000 0 0          0xff200000 0 0x00100000>,
> > +               <0x82000000 0 0x00300000 0xff300000 0 0x00d00000>;
> > +      interrupts = <GIC_SPI 243 IRQ_TYPE_NONE>;
>
> Ditto.

Ack


--
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  2022-04-22 15:51       ` Krzysztof Kozlowski
@ 2022-04-22 19:09         ` Dmitry Baryshkov
  2022-04-23  9:48           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2022-04-22 19:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, Stanimir Varbanov,
	Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 18:51, Krzysztof Kozlowski wrote:
> On 22/04/2022 17:47, Dmitry Baryshkov wrote:
>> On Fri, 22 Apr 2022 at 15:55, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 22/04/2022 13:48, Dmitry Baryshkov wrote:
>>>> Instead of specifying the enum of possible reg-names, specify them
>>>> explicitly. This allows us to specify which chipsets need the "atu"
>>>> regions, which do not. Also it clearly describes which platforms
>>>> enumerate PCIe cores using the dbi region and which use parf region for
>>>> that.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> ---
>>>>   .../devicetree/bindings/pci/qcom,pcie.yaml    | 96 ++++++++++++++++---
>>>>   1 file changed, 81 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> index 7210057d1511..e78e63ea4b25 100644
>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>>>> @@ -35,21 +35,6 @@ properties:
>>>>             - qcom,pcie-ipq6018
>>>>         - const: snps,dw-pcie
>>>>
>>>> -  reg:
>>>> -    minItems: 4
>>>> -    maxItems: 5
>>>
>>> This should stay.
>>>
>>>> -
>>>> -  reg-names:
>>>> -    minItems: 4
>>>> -    maxItems: 5
>>>> -    items:
>>>> -      enum:
>>>> -        - parf # Qualcomm specific registers
>>>> -        - dbi # DesignWare PCIe registers
>>>> -        - elbi # External local bus interface registers
>>>> -        - config # PCIe configuration space
>>>> -        - atu # ATU address space (optional)
>>>
>>> Move one of your lists for specific compatibles here and name last
>>> element optional (minItems: 4).
>>>
>>> You will need to fix the order of regs in DTS to match the one defined here.
>>
>> I see your idea. I wanted to be explicit, which platforms need atu and
>> which do not. You'd prefer not to.
> 
> Opposite, I wish platforms to be specific, which need atu which not.
> However I wish the strictly defined, same order for everyone because it
> looks possible.

Well, the same order is not possible, since for some devices the first, 
address-defining reg is "parf", for others it is "dbi". So, there will 
be two "families" of the devices. Unless we want to change the DT 
address of the unit.

>> Let's probably drop this for now. The bindings proposed in patch 1
>> work for now. I will work on updating reg-names later.
> 
> 
> Best regards,
> Krzysztof


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly
  2022-04-22 19:09         ` Dmitry Baryshkov
@ 2022-04-23  9:48           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-23  9:48 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, Stanimir Varbanov,
	Bjorn Helgaas, Vinod Koul, linux-arm-msm, linux-pci, devicetree

On 22/04/2022 21:09, Dmitry Baryshkov wrote:
>>
>> Opposite, I wish platforms to be specific, which need atu which not.
>> However I wish the strictly defined, same order for everyone because it
>> looks possible.
> 
> Well, the same order is not possible, since for some devices the first, 
> address-defining reg is "parf", for others it is "dbi". So, there will 
> be two "families" of the devices. Unless we want to change the DT 
> address of the unit.

I missed that, so my feedback is not correct. Better to not change the
unit addresses. You can still leave the "reg" and "reg-names" with
generic constraints in the main properties part, but the rest looks
actually correct.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-04-23  9:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22 11:48 [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Dmitry Baryshkov
2022-04-22 11:48 ` [PATCH 1/6] dt-bindings: pci/qcom,pcie: convert to YAML Dmitry Baryshkov
2022-04-22 13:10   ` Krzysztof Kozlowski
2022-04-22 16:49     ` Dmitry Baryshkov
2022-04-22 11:48 ` [PATCH 2/6] dt-bindings: pci/qcom,pcie: add schema for sc7280 chipset Dmitry Baryshkov
2022-04-22 13:11   ` Krzysztof Kozlowski
2022-04-22 11:48 ` [PATCH 3/6] dt-bindings: pci/qcom-pcie: specify reg-names explicitly Dmitry Baryshkov
2022-04-22 12:55   ` Krzysztof Kozlowski
2022-04-22 15:47     ` Dmitry Baryshkov
2022-04-22 15:51       ` Krzysztof Kozlowski
2022-04-22 19:09         ` Dmitry Baryshkov
2022-04-23  9:48           ` Krzysztof Kozlowski
2022-04-22 11:48 ` [PATCH 4/6] dt-bindings: pci/qcom,pcie: stop using snps,dw-pcie fallback Dmitry Baryshkov
2022-04-22 13:13   ` Krzysztof Kozlowski
2022-04-22 11:48 ` [PATCH 5/6] arm64: dts: qcom: stop using snps,dw-pcie falback Dmitry Baryshkov
2022-04-22 13:20   ` Krzysztof Kozlowski
2022-04-22 11:48 ` [PATCH 6/6] arm: " Dmitry Baryshkov
2022-04-22 13:19   ` Krzysztof Kozlowski
2022-04-22 13:19 ` [PATCH 0/6] dt-bindings: YAMLify pci/qcom,pcie schema Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.