linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/4] Spilt PCIe node to comply with hardware design
@ 2020-09-14 11:26 Chuanjia Liu
  2020-09-14 11:26 ` [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings Chuanjia Liu
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-14 11:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger
  Cc: linux-pci, linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee

Split the PCIe node for MT2712 and MT7622 platform to fix MSI issue
and comply with the hardware design.

change note:
  v6:Fix yaml error. Make sure driver compatible with old and 
     new DTS format.
  v5:rebase for 5.9-rc1, no code change. 
  v4:change commit message due to bayes statistical bogofilter
     considers this series patch SPAM.
  v3:rebase for 5.8-rc1. Only collect ack of Ryder, No code change.
  v2:change the allocation of MT2712 PCIe MMIO space due to the
     allocation size is not right in v1.

Chuanjia Liu (4):
 dt-bindings: pci: mediatek: Modified the Device tree bindings
 PCI: mediatek: Add new method to get shared pcie-cfg base and irq
 arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622
 ARM: dts: mediatek: Modified MT7629 PCIe node

 .../bindings/pci/mediatek-pcie-cfg.yaml       |  37 +++++
 .../devicetree/bindings/pci/mediatek-pcie.txt | 139 +++++++++++-------
 arch/arm/boot/dts/mt7629-rfb.dts              |   3 +-
 arch/arm/boot/dts/mt7629.dtsi                 |  22 +--
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi     |  75 ++++++----
 .../dts/mediatek/mt7622-bananapi-bpi-r64.dts  |  16 +-
 arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts  |   6 +-
 arch/arm64/boot/dts/mediatek/mt7622.dtsi      |  66 ++++++---
 drivers/pci/controller/pcie-mediatek.c        |  23 ++-
 9 files changed, 253 insertions(+), 134 deletions(-)

-- 
2.18.0


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

* [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings
  2020-09-14 11:26 [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Chuanjia Liu
@ 2020-09-14 11:26 ` Chuanjia Liu
  2020-09-22 23:31   ` Rob Herring
  2020-09-14 11:26 ` [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq Chuanjia Liu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-14 11:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger
  Cc: linux-pci, linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee, Chuanjia Liu

Split the PCIe node and add pciecfg node to fix MSI issue.

Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
 .../bindings/pci/mediatek-pcie-cfg.yaml       |  37 +++++
 .../devicetree/bindings/pci/mediatek-pcie.txt | 139 +++++++++++-------
 2 files changed, 123 insertions(+), 53 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
new file mode 100644
index 000000000000..cd72973c99d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/mediatek-pcie-cfg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek PCIECFG controller
+
+maintainers:
+  - Chuanjia Liu <chuanjia.liu@mediatek.com>
+  - Jianjun Wang <jianjun.wang@mediatek.com>
+
+description: |
+  The MediaTek PCIECFG controller controls some feature about
+  LTSSM, ASPM and so on.
+
+properties:
+  compatible:
+      items:
+        - enum:
+            - mediatek,generic-pciecfg
+        - const: syscon
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    pciecfg: pciecfg@1a140000 {
+        compatible = "mediatek,generic-pciecfg", "syscon";
+        reg = <0x1a140000 0x1000>;
+    };
+...
diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
index 7468d666763a..f849703dfb17 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
@@ -8,7 +8,7 @@ Required properties:
 	"mediatek,mt7623-pcie"
 	"mediatek,mt7629-pcie"
 - device_type: Must be "pci"
-- reg: Base addresses and lengths of the PCIe subsys and root ports.
+- reg: Base addresses and lengths of the root ports.
 - reg-names: Names of the above areas to use during resource lookup.
 - #address-cells: Address representation for root ports (must be 3)
 - #size-cells: Size representation for root ports (must be 2)
@@ -19,10 +19,10 @@ Required properties:
    - sys_ckN :transaction layer and data link layer clock
   Required entries for MT2701/MT7623:
    - free_ck :for reference clock of PCIe subsys
-  Required entries for MT2712/MT7622:
+  Required entries for MT2712/MT7622/MT7629:
    - ahb_ckN :AHB slave interface operating clock for CSR access and RC
 	      initiated MMIO access
-  Required entries for MT7622:
+  Required entries for MT7622/MT7629:
    - axi_ckN :application layer MMIO channel operating clock
    - aux_ckN :pe2_mac_bridge and pe2_mac_core operating clock when
 	      pcie_mac_ck/pcie_pipe_ck is turned off
@@ -47,7 +47,7 @@ Required properties for MT7623/MT2701:
 - reset-names: Must be "pcie-rst0", "pcie-rst1", "pcie-rstN".. based on the
   number of root ports.
 
-Required properties for MT2712/MT7622:
+Required properties for MT2712/MT7622/MT7629:
 -interrupts: A list of interrupt outputs of the controller, must have one
 	     entry for each PCIe port
 
@@ -143,56 +143,73 @@ Examples for MT7623:
 
 Examples for MT2712:
 
-	pcie: pcie@11700000 {
+	pcie1: pcie@112ff000 {
 		compatible = "mediatek,mt2712-pcie";
 		device_type = "pci";
-		reg = <0 0x11700000 0 0x1000>,
-		      <0 0x112ff000 0 0x1000>;
-		reg-names = "port0", "port1";
+		reg = <0 0x112ff000 0 0x1000>;
+		reg-names = "port1";
 		#address-cells = <3>;
 		#size-cells = <2>;
-		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
-			 <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
-			 <&pericfg CLK_PERI_PCIE0>,
+		interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "pcie_irq";
+		clocks = <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
 			 <&pericfg CLK_PERI_PCIE1>;
-		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1";
-		phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
-		phy-names = "pcie-phy0", "pcie-phy1";
+		clock-names = "sys_ck1", "ahb_ck1";
+		phys = <&u3port1 PHY_TYPE_PCIE>;
+		phy-names = "pcie-phy1";
 		bus-range = <0x00 0xff>;
-		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
+		ranges = <0x82000000 0 0x11400000  0x0 0x11400000  0 0x300000>;
+		status = "disabled";
 
-		pcie0: pcie@0,0 {
-			reg = <0x0000 0 0 0 0>;
+		slot1: pcie@1,0 {
+			reg = <0x0800 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
 			ranges;
 			interrupt-map-mask = <0 0 0 7>;
-			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
-					<0 0 0 2 &pcie_intc0 1>,
-					<0 0 0 3 &pcie_intc0 2>,
-					<0 0 0 4 &pcie_intc0 3>;
-			pcie_intc0: interrupt-controller {
+			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
+					<0 0 0 2 &pcie_intc1 1>,
+					<0 0 0 3 &pcie_intc1 2>,
+					<0 0 0 4 &pcie_intc1 3>;
+			pcie_intc1: interrupt-controller {
 				interrupt-controller;
 				#address-cells = <0>;
 				#interrupt-cells = <1>;
 			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
-			reg = <0x0800 0 0 0 0>;
+	pcie0: pcie@11700000 {
+		compatible = "mediatek,mt2712-pcie";
+		device_type = "pci";
+		reg = <0 0x11700000 0 0x1000>;
+		reg-names = "port0";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "pcie_irq";
+		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
+			 <&pericfg CLK_PERI_PCIE0>;
+		clock-names = "sys_ck0", "ahb_ck0";
+		phys = <&u3port0 PHY_TYPE_PCIE>;
+		phy-names = "pcie-phy0";
+		bus-range = <0x00 0xff>;
+		ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+		status = "disabled";
+
+		slot0: pcie@0,0 {
+			reg = <0x0000 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
 			ranges;
 			interrupt-map-mask = <0 0 0 7>;
-			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
-					<0 0 0 2 &pcie_intc1 1>,
-					<0 0 0 3 &pcie_intc1 2>,
-					<0 0 0 4 &pcie_intc1 3>;
-			pcie_intc1: interrupt-controller {
+			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
+					<0 0 0 2 &pcie_intc0 1>,
+					<0 0 0 3 &pcie_intc0 2>,
+					<0 0 0 4 &pcie_intc0 3>;
+			pcie_intc0: interrupt-controller {
 				interrupt-controller;
 				#address-cells = <0>;
 				#interrupt-cells = <1>;
@@ -202,39 +219,30 @@ Examples for MT2712:
 
 Examples for MT7622:
 
-	pcie: pcie@1a140000 {
+	pcie0: pcie@1a143000 {
 		compatible = "mediatek,mt7622-pcie";
 		device_type = "pci";
-		reg = <0 0x1a140000 0 0x1000>,
-		      <0 0x1a143000 0 0x1000>,
-		      <0 0x1a145000 0 0x1000>;
-		reg-names = "subsys", "port0", "port1";
+		reg = <0 0x1a143000 0 0x1000>;
+		reg-names = "port0";
 		#address-cells = <3>;
 		#size-cells = <2>;
-		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "pcie_irq";
 		clocks = <&pciesys CLK_PCIE_P0_MAC_EN>,
-			 <&pciesys CLK_PCIE_P1_MAC_EN>,
 			 <&pciesys CLK_PCIE_P0_AHB_EN>,
-			 <&pciesys CLK_PCIE_P1_AHB_EN>,
 			 <&pciesys CLK_PCIE_P0_AUX_EN>,
-			 <&pciesys CLK_PCIE_P1_AUX_EN>,
 			 <&pciesys CLK_PCIE_P0_AXI_EN>,
-			 <&pciesys CLK_PCIE_P1_AXI_EN>,
 			 <&pciesys CLK_PCIE_P0_OBFF_EN>,
-			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
-			 <&pciesys CLK_PCIE_P0_PIPE_EN>,
-			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
-		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1",
-			      "aux_ck0", "aux_ck1", "axi_ck0", "axi_ck1",
-			      "obff_ck0", "obff_ck1", "pipe_ck0", "pipe_ck1";
-		phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
-		phy-names = "pcie-phy0", "pcie-phy1";
+			 <&pciesys CLK_PCIE_P0_PIPE_EN>;
+		clock-names = "sys_ck0", "ahb_ck0", "aux_ck0",
+			      "axi_ck0", "obff_ck0", "pipe_ck0";
+
 		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
 		bus-range = <0x00 0xff>;
-		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
+		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x8000000>;
+		status = "disabled";
 
-		pcie0: pcie@0,0 {
+		slot0: pcie@0,0 {
 			reg = <0x0000 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
@@ -251,8 +259,33 @@ Examples for MT7622:
 				#interrupt-cells = <1>;
 			};
 		};
+	};
+
+	pcie1: pcie@1a145000 {
+		compatible = "mediatek,mt7622-pcie";
+		device_type = "pci";
+		reg = <0 0x1a145000 0 0x1000>;
+		reg-names = "port1";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "pcie_irq";
+		clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
+			 /* designer has connect RC1 with p0_ahb clock */
+			 <&pciesys CLK_PCIE_P0_AHB_EN>,
+			 <&pciesys CLK_PCIE_P1_AUX_EN>,
+			 <&pciesys CLK_PCIE_P1_AXI_EN>,
+			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
+			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
+		clock-names = "sys_ck1", "ahb_ck1", "aux_ck1",
+			      "axi_ck1", "obff_ck1", "pipe_ck1";
+
+		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
+		bus-range = <0x00 0xff>;
+		ranges = <0x82000000 0 0x28000000  0x0 0x28000000  0 0x8000000>;
+		status = "disabled";
 
-		pcie1: pcie@1,0 {
+		slot1: pcie@1,0 {
 			reg = <0x0800 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
-- 
2.18.0

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

* [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq
  2020-09-14 11:26 [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Chuanjia Liu
  2020-09-14 11:26 ` [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings Chuanjia Liu
@ 2020-09-14 11:26 ` Chuanjia Liu
  2020-09-30 15:23   ` Rob Herring
  2020-09-14 11:26 ` [PATCH v6 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622 Chuanjia Liu
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-14 11:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger
  Cc: linux-pci, linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee, Chuanjia Liu

Add new method to get shared pcie-cfg base and pcie irq for
new dts format.

Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cf4c18f0c25a..5b915eb0cf1e 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -14,6 +14,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/msi.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
@@ -23,6 +24,7 @@
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/reset.h>
 
 #include "../pci.h"
@@ -205,6 +207,7 @@ struct mtk_pcie_port {
  * struct mtk_pcie - PCIe host information
  * @dev: pointer to PCIe device
  * @base: IO mapped register base
+ * @cfg: IO mapped register map for PCIe config
  * @free_ck: free-run reference clock
  * @mem: non-prefetchable memory resource
  * @ports: pointer to PCIe port information
@@ -213,6 +216,7 @@ struct mtk_pcie_port {
 struct mtk_pcie {
 	struct device *dev;
 	void __iomem *base;
+	struct regmap *cfg;
 	struct clk *free_ck;
 
 	struct list_head ports;
@@ -648,7 +652,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
 		return err;
 	}
 
-	port->irq = platform_get_irq(pdev, port->slot);
+	if (of_find_property(dev->of_node, "interrupt-names", NULL))
+		port->irq = platform_get_irq_byname(pdev, "pcie_irq");
+	else
+		port->irq = platform_get_irq(pdev, port->slot);
+
 	if (port->irq < 0)
 		return port->irq;
 
@@ -680,6 +688,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 		val |= PCIE_CSR_LTSSM_EN(port->slot) |
 		       PCIE_CSR_ASPM_L1_EN(port->slot);
 		writel(val, pcie->base + PCIE_SYS_CFG_V2);
+	} else if (pcie->cfg) {
+		val = PCIE_CSR_LTSSM_EN(port->slot) |
+		      PCIE_CSR_ASPM_L1_EN(port->slot);
+		regmap_update_bits(pcie->cfg, PCIE_SYS_CFG_V2, val, val);
 	}
 
 	/* Assert all reset signals */
@@ -983,6 +995,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct resource *regs;
+	struct device_node *cfg_node;
 	int err;
 
 	/* get shared registers, which are optional */
@@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
 		}
 	}
 
+	cfg_node = of_find_compatible_node(NULL, NULL,
+					   "mediatek,generic-pciecfg");
+	if (cfg_node) {
+		pcie->cfg = syscon_node_to_regmap(cfg_node);
+		if (IS_ERR(pcie->cfg))
+			return PTR_ERR(pcie->cfg);
+	}
+
 	pcie->free_ck = devm_clk_get(dev, "free_ck");
 	if (IS_ERR(pcie->free_ck)) {
 		if (PTR_ERR(pcie->free_ck) == -EPROBE_DEFER)
-- 
2.18.0

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

* [PATCH v6 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622
  2020-09-14 11:26 [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Chuanjia Liu
  2020-09-14 11:26 ` [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings Chuanjia Liu
  2020-09-14 11:26 ` [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq Chuanjia Liu
@ 2020-09-14 11:26 ` Chuanjia Liu
  2020-09-14 11:26 ` [PATCH v6 4/4] ARM: dts: mediatek: Modified MT7629 PCIe node Chuanjia Liu
  2020-09-14 11:39 ` Aw: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Frank Wunderlich
  4 siblings, 0 replies; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-14 11:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger
  Cc: linux-pci, linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee, Chuanjia Liu

There are two independent PCIe controllers in MT2712 and MT7622
platform. Each of them should contain an independent MSI domain.

In current architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.

Split the PCIe node for MT2712 and MT7622 platform to fix MSI issue
and comply with the hardware design.

Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi     | 75 +++++++++++--------
 .../dts/mediatek/mt7622-bananapi-bpi-r64.dts  | 16 ++--
 arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts  |  6 +-
 arch/arm64/boot/dts/mediatek/mt7622.dtsi      | 66 ++++++++++------
 4 files changed, 94 insertions(+), 69 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index db17d0a4ed57..337e56bdbd08 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -915,60 +915,73 @@
 		};
 	};
 
-	pcie: pcie@11700000 {
+	pcie1: pcie@112ff000 {
 		compatible = "mediatek,mt2712-pcie";
 		device_type = "pci";
-		reg = <0 0x11700000 0 0x1000>,
-		      <0 0x112ff000 0 0x1000>;
-		reg-names = "port0", "port1";
+		reg = <0 0x112ff000 0 0x1000>;
+		reg-names = "port1";
 		#address-cells = <3>;
 		#size-cells = <2>;
-		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
-			 <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
-			 <&pericfg CLK_PERI_PCIE0>,
+		interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "pcie_irq";
+		clocks = <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
 			 <&pericfg CLK_PERI_PCIE1>;
-		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1";
-		phys = <&u3port0 PHY_TYPE_PCIE>, <&u3port1 PHY_TYPE_PCIE>;
-		phy-names = "pcie-phy0", "pcie-phy1";
+		clock-names = "sys_ck1", "ahb_ck1";
+		phys = <&u3port1 PHY_TYPE_PCIE>;
+		phy-names = "pcie-phy1";
 		bus-range = <0x00 0xff>;
-		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
+		ranges = <0x82000000 0 0x11400000  0x0 0x11400000  0 0x300000>;
+		status = "disabled";
 
-		pcie0: pcie@0,0 {
-			device_type = "pci";
-			status = "disabled";
-			reg = <0x0000 0 0 0 0>;
+		slot1: pcie@1,0 {
+			reg = <0x0800 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
 			ranges;
 			interrupt-map-mask = <0 0 0 7>;
-			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
-					<0 0 0 2 &pcie_intc0 1>,
-					<0 0 0 3 &pcie_intc0 2>,
-					<0 0 0 4 &pcie_intc0 3>;
-			pcie_intc0: interrupt-controller {
+			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
+					<0 0 0 2 &pcie_intc1 1>,
+					<0 0 0 3 &pcie_intc1 2>,
+					<0 0 0 4 &pcie_intc1 3>;
+			pcie_intc1: interrupt-controller {
 				interrupt-controller;
 				#address-cells = <0>;
 				#interrupt-cells = <1>;
 			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
-			device_type = "pci";
-			status = "disabled";
-			reg = <0x0800 0 0 0 0>;
+	pcie0: pcie@11700000 {
+		compatible = "mediatek,mt2712-pcie";
+		device_type = "pci";
+		reg = <0 0x11700000 0 0x1000>;
+		reg-names = "port0";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "pcie_irq";
+		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
+			 <&pericfg CLK_PERI_PCIE0>;
+		clock-names = "sys_ck0", "ahb_ck0";
+		phys = <&u3port0 PHY_TYPE_PCIE>;
+		phy-names = "pcie-phy0";
+		bus-range = <0x00 0xff>;
+		ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+		status = "disabled";
+
+		slot0: pcie@0,0 {
+			reg = <0x0000 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
 			ranges;
 			interrupt-map-mask = <0 0 0 7>;
-			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
-					<0 0 0 2 &pcie_intc1 1>,
-					<0 0 0 3 &pcie_intc1 2>,
-					<0 0 0 4 &pcie_intc1 3>;
-			pcie_intc1: interrupt-controller {
+			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
+					<0 0 0 2 &pcie_intc0 1>,
+					<0 0 0 3 &pcie_intc0 2>,
+					<0 0 0 4 &pcie_intc0 3>;
+			pcie_intc0: interrupt-controller {
 				interrupt-controller;
 				#address-cells = <0>;
 				#interrupt-cells = <1>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index d174ad214857..83baab46f060 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -207,18 +207,16 @@
 	};
 };
 
-&pcie {
+&pcie0 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pcie0_pins>, <&pcie1_pins>;
+	pinctrl-0 = <&pcie0_pins>;
 	status = "okay";
+};
 
-	pcie@0,0 {
-		status = "okay";
-	};
-
-	pcie@1,0 {
-		status = "okay";
-	};
+&pcie1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie1_pins>;
+	status = "okay";
 };
 
 &pio {
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 0b4de627f96e..8e98b78ba232 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -183,14 +183,10 @@
 	};
 };
 
-&pcie {
+&pcie0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie0_pins>;
 	status = "okay";
-
-	pcie@0,0 {
-		status = "okay";
-	};
 };
 
 &pio {
diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index 1a39e0ef776b..539d912e3e1f 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -777,45 +777,40 @@
 		#reset-cells = <1>;
 	};
 
-	pcie: pcie@1a140000 {
+	pciecfg: pciecfg@1a140000 {
+		compatible = "mediatek,generic-pciecfg", "syscon";
+		reg = <0 0x1a140000 0 0x1000>;
+	};
+
+	pcie0: pcie@1a143000 {
 		compatible = "mediatek,mt7622-pcie";
 		device_type = "pci";
-		reg = <0 0x1a140000 0 0x1000>,
-		      <0 0x1a143000 0 0x1000>,
-		      <0 0x1a145000 0 0x1000>;
-		reg-names = "subsys", "port0", "port1";
+		reg = <0 0x1a143000 0 0x1000>;
+		reg-names = "port0";
 		#address-cells = <3>;
 		#size-cells = <2>;
-		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "pcie_irq";
 		clocks = <&pciesys CLK_PCIE_P0_MAC_EN>,
-			 <&pciesys CLK_PCIE_P1_MAC_EN>,
-			 <&pciesys CLK_PCIE_P0_AHB_EN>,
 			 <&pciesys CLK_PCIE_P0_AHB_EN>,
 			 <&pciesys CLK_PCIE_P0_AUX_EN>,
-			 <&pciesys CLK_PCIE_P1_AUX_EN>,
 			 <&pciesys CLK_PCIE_P0_AXI_EN>,
-			 <&pciesys CLK_PCIE_P1_AXI_EN>,
 			 <&pciesys CLK_PCIE_P0_OBFF_EN>,
-			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
-			 <&pciesys CLK_PCIE_P0_PIPE_EN>,
-			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
-		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1",
-			      "aux_ck0", "aux_ck1", "axi_ck0", "axi_ck1",
-			      "obff_ck0", "obff_ck1", "pipe_ck0", "pipe_ck1";
+			 <&pciesys CLK_PCIE_P0_PIPE_EN>;
+		clock-names = "sys_ck0", "ahb_ck0", "aux_ck0",
+			      "axi_ck0", "obff_ck0", "pipe_ck0";
+
 		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
 		bus-range = <0x00 0xff>;
-		ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x8000000>;
 		status = "disabled";
 
-		pcie0: pcie@0,0 {
+		slot0: pcie@0,0 {
 			reg = <0x0000 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
 			ranges;
-			status = "disabled";
-
 			interrupt-map-mask = <0 0 0 7>;
 			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
 					<0 0 0 2 &pcie_intc0 1>,
@@ -827,15 +822,38 @@
 				#interrupt-cells = <1>;
 			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
+	pcie1: pcie@1a145000 {
+		compatible = "mediatek,mt7622-pcie";
+		device_type = "pci";
+		reg = <0 0x1a145000 0 0x1000>;
+		reg-names = "port1";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "pcie_irq";
+		clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
+			 /* designer has connect RC1 with p0_ahb clock */
+			 <&pciesys CLK_PCIE_P0_AHB_EN>,
+			 <&pciesys CLK_PCIE_P1_AUX_EN>,
+			 <&pciesys CLK_PCIE_P1_AXI_EN>,
+			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
+			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
+		clock-names = "sys_ck1", "ahb_ck1", "aux_ck1",
+			      "axi_ck1", "obff_ck1", "pipe_ck1";
+
+		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
+		bus-range = <0x00 0xff>;
+		ranges = <0x82000000 0 0x28000000  0x0 0x28000000  0 0x8000000>;
+		status = "disabled";
+
+		slot1: pcie@1,0 {
 			reg = <0x0800 0 0 0 0>;
 			#address-cells = <3>;
 			#size-cells = <2>;
 			#interrupt-cells = <1>;
 			ranges;
-			status = "disabled";
-
 			interrupt-map-mask = <0 0 0 7>;
 			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
 					<0 0 0 2 &pcie_intc1 1>,
-- 
2.18.0

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

* [PATCH v6 4/4] ARM: dts: mediatek: Modified MT7629 PCIe node
  2020-09-14 11:26 [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Chuanjia Liu
                   ` (2 preceding siblings ...)
  2020-09-14 11:26 ` [PATCH v6 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622 Chuanjia Liu
@ 2020-09-14 11:26 ` Chuanjia Liu
  2020-09-14 11:39 ` Aw: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Frank Wunderlich
  4 siblings, 0 replies; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-14 11:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger
  Cc: linux-pci, linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee, Chuanjia Liu

Remove unused property and add pciecfg node.

Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
 arch/arm/boot/dts/mt7629-rfb.dts |  3 ++-
 arch/arm/boot/dts/mt7629.dtsi    | 22 ++++++++++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/mt7629-rfb.dts b/arch/arm/boot/dts/mt7629-rfb.dts
index 9980c10c6e29..eb536cbebd9b 100644
--- a/arch/arm/boot/dts/mt7629-rfb.dts
+++ b/arch/arm/boot/dts/mt7629-rfb.dts
@@ -140,9 +140,10 @@
 	};
 };
 
-&pcie {
+&pcie1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_pins>;
+	status = "okay";
 };
 
 &pciephy1 {
diff --git a/arch/arm/boot/dts/mt7629.dtsi b/arch/arm/boot/dts/mt7629.dtsi
index 5cbb3d244c75..6d6397f0c2fc 100644
--- a/arch/arm/boot/dts/mt7629.dtsi
+++ b/arch/arm/boot/dts/mt7629.dtsi
@@ -360,16 +360,20 @@
 			#reset-cells = <1>;
 		};
 
-		pcie: pcie@1a140000 {
+		pciecfg: pciecfg@1a140000 {
+			compatible = "mediatek,generic-pciecfg", "syscon";
+			reg = <0x1a140000 0x1000>;
+		};
+
+		pcie1: pcie@1a145000 {
 			compatible = "mediatek,mt7629-pcie";
 			device_type = "pci";
-			reg = <0x1a140000 0x1000>,
-			      <0x1a145000 0x1000>;
-			reg-names = "subsys","port1";
+			reg = <0x1a145000 0x1000>;
+			reg-names = "port1";
 			#address-cells = <3>;
 			#size-cells = <2>;
-			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_LOW>,
-				     <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+			interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-names = "pcie_irq";
 			clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
 				 <&pciesys CLK_PCIE_P0_AHB_EN>,
 				 <&pciesys CLK_PCIE_P1_AUX_EN>,
@@ -390,21 +394,19 @@
 			power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
 			bus-range = <0x00 0xff>;
 			ranges = <0x82000000 0 0x20000000 0x20000000 0 0x10000000>;
+			status = "disabled";
 
-			pcie1: pcie@1,0 {
-				device_type = "pci";
+			slot1: pcie@1,0 {
 				reg = <0x0800 0 0 0 0>;
 				#address-cells = <3>;
 				#size-cells = <2>;
 				#interrupt-cells = <1>;
 				ranges;
-				num-lanes = <1>;
 				interrupt-map-mask = <0 0 0 7>;
 				interrupt-map = <0 0 0 1 &pcie_intc1 0>,
 						<0 0 0 2 &pcie_intc1 1>,
 						<0 0 0 3 &pcie_intc1 2>,
 						<0 0 0 4 &pcie_intc1 3>;
-
 				pcie_intc1: interrupt-controller {
 					interrupt-controller;
 					#address-cells = <0>;
-- 
2.18.0

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

* Aw: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design
  2020-09-14 11:26 [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Chuanjia Liu
                   ` (3 preceding siblings ...)
  2020-09-14 11:26 ` [PATCH v6 4/4] ARM: dts: mediatek: Modified MT7629 PCIe node Chuanjia Liu
@ 2020-09-14 11:39 ` Frank Wunderlich
  2020-09-28  3:25   ` Chuanjia Liu
  4 siblings, 1 reply; 11+ messages in thread
From: Frank Wunderlich @ 2020-09-14 11:39 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger,
	devicetree, Ryder Lee, linux-pci, linux-mediatek,
	linux-arm-kernel, yong.wu

> Betreff: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design

just if you need to make another version (as it is only the cover-letter) you can fix the typo in subject ;)

regards Frank


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

* Re: [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings
  2020-09-14 11:26 ` [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings Chuanjia Liu
@ 2020-09-22 23:31   ` Rob Herring
  2020-09-28  3:18     ` Chuanjia Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-09-22 23:31 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Matthias Brugger, linux-pci,
	linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee

On Mon, Sep 14, 2020 at 07:26:56PM +0800, Chuanjia Liu wrote:
> Split the PCIe node and add pciecfg node to fix MSI issue.

What's the MSI issue?

This is not a compatible change. Please explain why that's okay.

> 
> Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  .../bindings/pci/mediatek-pcie-cfg.yaml       |  37 +++++
>  .../devicetree/bindings/pci/mediatek-pcie.txt | 139 +++++++++++-------
>  2 files changed, 123 insertions(+), 53 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> new file mode 100644
> index 000000000000..cd72973c99d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> @@ -0,0 +1,37 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/mediatek-pcie-cfg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek PCIECFG controller
> +
> +maintainers:
> +  - Chuanjia Liu <chuanjia.liu@mediatek.com>
> +  - Jianjun Wang <jianjun.wang@mediatek.com>
> +
> +description: |
> +  The MediaTek PCIECFG controller controls some feature about
> +  LTSSM, ASPM and so on.
> +
> +properties:
> +  compatible:
> +      items:
> +        - enum:
> +            - mediatek,generic-pciecfg
> +        - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg

additionalProperties: false

> +
> +examples:
> +  - |
> +    pciecfg: pciecfg@1a140000 {
> +        compatible = "mediatek,generic-pciecfg", "syscon";
> +        reg = <0x1a140000 0x1000>;
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> index 7468d666763a..f849703dfb17 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> @@ -8,7 +8,7 @@ Required properties:
>  	"mediatek,mt7623-pcie"
>  	"mediatek,mt7629-pcie"
>  - device_type: Must be "pci"
> -- reg: Base addresses and lengths of the PCIe subsys and root ports.
> +- reg: Base addresses and lengths of the root ports.
>  - reg-names: Names of the above areas to use during resource lookup.
>  - #address-cells: Address representation for root ports (must be 3)
>  - #size-cells: Size representation for root ports (must be 2)
> @@ -19,10 +19,10 @@ Required properties:
>     - sys_ckN :transaction layer and data link layer clock
>    Required entries for MT2701/MT7623:
>     - free_ck :for reference clock of PCIe subsys
> -  Required entries for MT2712/MT7622:
> +  Required entries for MT2712/MT7622/MT7629:

Seems like a unrelated change.

>     - ahb_ckN :AHB slave interface operating clock for CSR access and RC
>  	      initiated MMIO access
> -  Required entries for MT7622:
> +  Required entries for MT7622/MT7629:
>     - axi_ckN :application layer MMIO channel operating clock
>     - aux_ckN :pe2_mac_bridge and pe2_mac_core operating clock when
>  	      pcie_mac_ck/pcie_pipe_ck is turned off
> @@ -47,7 +47,7 @@ Required properties for MT7623/MT2701:
>  - reset-names: Must be "pcie-rst0", "pcie-rst1", "pcie-rstN".. based on the
>    number of root ports.
>  
> -Required properties for MT2712/MT7622:
> +Required properties for MT2712/MT7622/MT7629:
>  -interrupts: A list of interrupt outputs of the controller, must have one
>  	     entry for each PCIe port
>  
> @@ -143,56 +143,73 @@ Examples for MT7623:
>  
>  Examples for MT2712:
>  
> -	pcie: pcie@11700000 {
> +	pcie1: pcie@112ff000 {
>  		compatible = "mediatek,mt2712-pcie";
>  		device_type = "pci";
> -		reg = <0 0x11700000 0 0x1000>,
> -		      <0 0x112ff000 0 0x1000>;
> -		reg-names = "port0", "port1";
> +		reg = <0 0x112ff000 0 0x1000>;
> +		reg-names = "port1";
>  		#address-cells = <3>;
>  		#size-cells = <2>;
> -		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> -		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
> -			 <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
> -			 <&pericfg CLK_PERI_PCIE0>,
> +		interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "pcie_irq";
> +		clocks = <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
>  			 <&pericfg CLK_PERI_PCIE1>;
> -		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1";
> -		phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
> -		phy-names = "pcie-phy0", "pcie-phy1";
> +		clock-names = "sys_ck1", "ahb_ck1";
> +		phys = <&u3port1 PHY_TYPE_PCIE>;
> +		phy-names = "pcie-phy1";
>  		bus-range = <0x00 0xff>;
> -		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
> +		ranges = <0x82000000 0 0x11400000  0x0 0x11400000  0 0x300000>;
> +		status = "disabled";

Don't show status in examples.

>  
> -		pcie0: pcie@0,0 {
> -			reg = <0x0000 0 0 0 0>;
> +		slot1: pcie@1,0 {

Since you are breaking everything, you don't really need these child 
nodes. Just move interrupt-map and interrupt-controller up to the parent 
like other PCI host bindings.

> +			reg = <0x0800 0 0 0 0>;
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			#interrupt-cells = <1>;
>  			ranges;
>  			interrupt-map-mask = <0 0 0 7>;
> -			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
> -					<0 0 0 2 &pcie_intc0 1>,
> -					<0 0 0 3 &pcie_intc0 2>,
> -					<0 0 0 4 &pcie_intc0 3>;
> -			pcie_intc0: interrupt-controller {
> +			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
> +					<0 0 0 2 &pcie_intc1 1>,
> +					<0 0 0 3 &pcie_intc1 2>,
> +					<0 0 0 4 &pcie_intc1 3>;
> +			pcie_intc1: interrupt-controller {
>  				interrupt-controller;
>  				#address-cells = <0>;
>  				#interrupt-cells = <1>;
>  			};
>  		};
> +	};
>  
> -		pcie1: pcie@1,0 {
> -			reg = <0x0800 0 0 0 0>;
> +	pcie0: pcie@11700000 {
> +		compatible = "mediatek,mt2712-pcie";
> +		device_type = "pci";
> +		reg = <0 0x11700000 0 0x1000>;
> +		reg-names = "port0";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "pcie_irq";
> +		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
> +			 <&pericfg CLK_PERI_PCIE0>;
> +		clock-names = "sys_ck0", "ahb_ck0";
> +		phys = <&u3port0 PHY_TYPE_PCIE>;
> +		phy-names = "pcie-phy0";
> +		bus-range = <0x00 0xff>;
> +		ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
> +		status = "disabled";
> +
> +		slot0: pcie@0,0 {
> +			reg = <0x0000 0 0 0 0>;
>  			#address-cells = <3>;
>  			#size-cells = <2>;
>  			#interrupt-cells = <1>;
>  			ranges;
>  			interrupt-map-mask = <0 0 0 7>;
> -			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
> -					<0 0 0 2 &pcie_intc1 1>,
> -					<0 0 0 3 &pcie_intc1 2>,
> -					<0 0 0 4 &pcie_intc1 3>;
> -			pcie_intc1: interrupt-controller {
> +			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
> +					<0 0 0 2 &pcie_intc0 1>,
> +					<0 0 0 3 &pcie_intc0 2>,
> +					<0 0 0 4 &pcie_intc0 3>;
> +			pcie_intc0: interrupt-controller {
>  				interrupt-controller;
>  				#address-cells = <0>;
>  				#interrupt-cells = <1>;
> @@ -202,39 +219,30 @@ Examples for MT2712:
>  
>  Examples for MT7622:
>  
> -	pcie: pcie@1a140000 {
> +	pcie0: pcie@1a143000 {
>  		compatible = "mediatek,mt7622-pcie";
>  		device_type = "pci";
> -		reg = <0 0x1a140000 0 0x1000>,
> -		      <0 0x1a143000 0 0x1000>,
> -		      <0 0x1a145000 0 0x1000>;
> -		reg-names = "subsys", "port0", "port1";
> +		reg = <0 0x1a143000 0 0x1000>;
> +		reg-names = "port0";
>  		#address-cells = <3>;
>  		#size-cells = <2>;
> -		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>,
> -			     <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
> +		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-names = "pcie_irq";
>  		clocks = <&pciesys CLK_PCIE_P0_MAC_EN>,
> -			 <&pciesys CLK_PCIE_P1_MAC_EN>,
>  			 <&pciesys CLK_PCIE_P0_AHB_EN>,
> -			 <&pciesys CLK_PCIE_P1_AHB_EN>,
>  			 <&pciesys CLK_PCIE_P0_AUX_EN>,
> -			 <&pciesys CLK_PCIE_P1_AUX_EN>,
>  			 <&pciesys CLK_PCIE_P0_AXI_EN>,
> -			 <&pciesys CLK_PCIE_P1_AXI_EN>,
>  			 <&pciesys CLK_PCIE_P0_OBFF_EN>,
> -			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
> -			 <&pciesys CLK_PCIE_P0_PIPE_EN>,
> -			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
> -		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1",
> -			      "aux_ck0", "aux_ck1", "axi_ck0", "axi_ck1",
> -			      "obff_ck0", "obff_ck1", "pipe_ck0", "pipe_ck1";
> -		phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
> -		phy-names = "pcie-phy0", "pcie-phy1";
> +			 <&pciesys CLK_PCIE_P0_PIPE_EN>;
> +		clock-names = "sys_ck0", "ahb_ck0", "aux_ck0",
> +			      "axi_ck0", "obff_ck0", "pipe_ck0";
> +
>  		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
>  		bus-range = <0x00 0xff>;
> -		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
> +		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x8000000>;
> +		status = "disabled";
>  
> -		pcie0: pcie@0,0 {
> +		slot0: pcie@0,0 {
>  			reg = <0x0000 0 0 0 0>;
>  			#address-cells = <3>;
>  			#size-cells = <2>;
> @@ -251,8 +259,33 @@ Examples for MT7622:
>  				#interrupt-cells = <1>;
>  			};
>  		};
> +	};
> +
> +	pcie1: pcie@1a145000 {
> +		compatible = "mediatek,mt7622-pcie";
> +		device_type = "pci";
> +		reg = <0 0x1a145000 0 0x1000>;
> +		reg-names = "port1";
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-names = "pcie_irq";
> +		clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
> +			 /* designer has connect RC1 with p0_ahb clock */
> +			 <&pciesys CLK_PCIE_P0_AHB_EN>,
> +			 <&pciesys CLK_PCIE_P1_AUX_EN>,
> +			 <&pciesys CLK_PCIE_P1_AXI_EN>,
> +			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
> +			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
> +		clock-names = "sys_ck1", "ahb_ck1", "aux_ck1",
> +			      "axi_ck1", "obff_ck1", "pipe_ck1";
> +
> +		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
> +		bus-range = <0x00 0xff>;
> +		ranges = <0x82000000 0 0x28000000  0x0 0x28000000  0 0x8000000>;
> +		status = "disabled";
>  
> -		pcie1: pcie@1,0 {
> +		slot1: pcie@1,0 {
>  			reg = <0x0800 0 0 0 0>;
>  			#address-cells = <3>;
>  			#size-cells = <2>;
> -- 
> 2.18.0

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

* Re: [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings
  2020-09-22 23:31   ` Rob Herring
@ 2020-09-28  3:18     ` Chuanjia Liu
  0 siblings, 0 replies; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-28  3:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Lorenzo Pieralisi, Frank Wunderlich, linux-pci,
	Matthias Brugger, Ryder Lee, linux-mediatek, yong.wu,
	Bjorn Helgaas, linux-arm-kernel

On Tue, 2020-09-22 at 17:31 -0600, Rob Herring wrote:
> On Mon, Sep 14, 2020 at 07:26:56PM +0800, Chuanjia Liu wrote:
> > Split the PCIe node and add pciecfg node to fix MSI issue.
> 
> What's the MSI issue?
> 
> This is not a compatible change. Please explain why that's okay.
Hi, Rob

Thanks for your review!

In current architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.

Because of 2712/7622 is two independent PCIe controllers not one PCIe
controller two port,So we split PCIe node to comply with hardware
design and fix msi issue.
> 
> > 
> > Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  .../bindings/pci/mediatek-pcie-cfg.yaml       |  37 +++++
> >  .../devicetree/bindings/pci/mediatek-pcie.txt | 139 +++++++++++-------
> >  2 files changed, 123 insertions(+), 53 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> > new file mode 100644
> > index 000000000000..cd72973c99d5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> > @@ -0,0 +1,37 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/mediatek-pcie-cfg.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Mediatek PCIECFG controller
> > +
> > +maintainers:
> > +  - Chuanjia Liu <chuanjia.liu@mediatek.com>
> > +  - Jianjun Wang <jianjun.wang@mediatek.com>
> > +
> > +description: |
> > +  The MediaTek PCIECFG controller controls some feature about
> > +  LTSSM, ASPM and so on.
> > +
> > +properties:
> > +  compatible:
> > +      items:
> > +        - enum:
> > +            - mediatek,generic-pciecfg
> > +        - const: syscon
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> 
> additionalProperties: false

Thanks,I will add it in next version.
> 
> > +
> > +examples:
> > +  - |
> > +    pciecfg: pciecfg@1a140000 {
> > +        compatible = "mediatek,generic-pciecfg", "syscon";
> > +        reg = <0x1a140000 0x1000>;
> > +    };
> > +...
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> > index 7468d666763a..f849703dfb17 100644
> > --- a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> > @@ -8,7 +8,7 @@ Required properties:
> >  	"mediatek,mt7623-pcie"
> >  	"mediatek,mt7629-pcie"
> >  - device_type: Must be "pci"
> > -- reg: Base addresses and lengths of the PCIe subsys and root ports.
> > +- reg: Base addresses and lengths of the root ports.
> >  - reg-names: Names of the above areas to use during resource lookup.
> >  - #address-cells: Address representation for root ports (must be 3)
> >  - #size-cells: Size representation for root ports (must be 2)
> > @@ -19,10 +19,10 @@ Required properties:
> >     - sys_ckN :transaction layer and data link layer clock
> >    Required entries for MT2701/MT7623:
> >     - free_ck :for reference clock of PCIe subsys
> > -  Required entries for MT2712/MT7622:
> > +  Required entries for MT2712/MT7622/MT7629:
> 
> Seems like a unrelated change.

Yes,I will remove it in next version
> 
> >     - ahb_ckN :AHB slave interface operating clock for CSR access and RC
> >  	      initiated MMIO access
> > -  Required entries for MT7622:
> > +  Required entries for MT7622/MT7629:
> >     - axi_ckN :application layer MMIO channel operating clock
> >     - aux_ckN :pe2_mac_bridge and pe2_mac_core operating clock when
> >  	      pcie_mac_ck/pcie_pipe_ck is turned off
> > @@ -47,7 +47,7 @@ Required properties for MT7623/MT2701:
> >  - reset-names: Must be "pcie-rst0", "pcie-rst1", "pcie-rstN".. based on the
> >    number of root ports.
> >  
> > -Required properties for MT2712/MT7622:
> > +Required properties for MT2712/MT7622/MT7629:
> >  -interrupts: A list of interrupt outputs of the controller, must have one
> >  	     entry for each PCIe port
> >  
> > @@ -143,56 +143,73 @@ Examples for MT7623:
> >  
> >  Examples for MT2712:
> >  
> > -	pcie: pcie@11700000 {
> > +	pcie1: pcie@112ff000 {
> >  		compatible = "mediatek,mt2712-pcie";
> >  		device_type = "pci";
> > -		reg = <0 0x11700000 0 0x1000>,
> > -		      <0 0x112ff000 0 0x1000>;
> > -		reg-names = "port0", "port1";
> > +		reg = <0 0x112ff000 0 0x1000>;
> > +		reg-names = "port1";
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> > -		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
> > -			     <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> > -		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
> > -			 <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
> > -			 <&pericfg CLK_PERI_PCIE0>,
> > +		interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-names = "pcie_irq";
> > +		clocks = <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
> >  			 <&pericfg CLK_PERI_PCIE1>;
> > -		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1";
> > -		phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
> > -		phy-names = "pcie-phy0", "pcie-phy1";
> > +		clock-names = "sys_ck1", "ahb_ck1";
> > +		phys = <&u3port1 PHY_TYPE_PCIE>;
> > +		phy-names = "pcie-phy1";
> >  		bus-range = <0x00 0xff>;
> > -		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
> > +		ranges = <0x82000000 0 0x11400000  0x0 0x11400000  0 0x300000>;
> > +		status = "disabled";
> 
> Don't show status in examples.

Thanks,I will remove it in next version.
> 
> >  
> > -		pcie0: pcie@0,0 {
> > -			reg = <0x0000 0 0 0 0>;
> > +		slot1: pcie@1,0 {
> 
> Since you are breaking everything, you don't really need these child 
> nodes. Just move interrupt-map and interrupt-controller up to the parent 
> like other PCI host bindings.

In my opinion, interrupt-map and interrupt-controller can be placed in
parent or child node. I put it on child node for driver compatibility
with the old and new DTS format.
> 
> > +			reg = <0x0800 0 0 0 0>;
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			#interrupt-cells = <1>;
> >  			ranges;
> >  			interrupt-map-mask = <0 0 0 7>;
> > -			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
> > -					<0 0 0 2 &pcie_intc0 1>,
> > -					<0 0 0 3 &pcie_intc0 2>,
> > -					<0 0 0 4 &pcie_intc0 3>;
> > -			pcie_intc0: interrupt-controller {
> > +			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
> > +					<0 0 0 2 &pcie_intc1 1>,
> > +					<0 0 0 3 &pcie_intc1 2>,
> > +					<0 0 0 4 &pcie_intc1 3>;
> > +			pcie_intc1: interrupt-controller {
> >  				interrupt-controller;
> >  				#address-cells = <0>;
> >  				#interrupt-cells = <1>;
> >  			};
> >  		};
> > +	};
> >  
> > -		pcie1: pcie@1,0 {
> > -			reg = <0x0800 0 0 0 0>;
> > +	pcie0: pcie@11700000 {
> > +		compatible = "mediatek,mt2712-pcie";
> > +		device_type = "pci";
> > +		reg = <0 0x11700000 0 0x1000>;
> > +		reg-names = "port0";
> > +		#address-cells = <3>;
> > +		#size-cells = <2>;
> > +		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-names = "pcie_irq";
> > +		clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
> > +			 <&pericfg CLK_PERI_PCIE0>;
> > +		clock-names = "sys_ck0", "ahb_ck0";
> > +		phys = <&u3port0 PHY_TYPE_PCIE>;
> > +		phy-names = "pcie-phy0";
> > +		bus-range = <0x00 0xff>;
> > +		ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
> > +		status = "disabled";
> > +
> > +		slot0: pcie@0,0 {
> > +			reg = <0x0000 0 0 0 0>;
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> >  			#interrupt-cells = <1>;
> >  			ranges;
> >  			interrupt-map-mask = <0 0 0 7>;
> > -			interrupt-map = <0 0 0 1 &pcie_intc1 0>,
> > -					<0 0 0 2 &pcie_intc1 1>,
> > -					<0 0 0 3 &pcie_intc1 2>,
> > -					<0 0 0 4 &pcie_intc1 3>;
> > -			pcie_intc1: interrupt-controller {
> > +			interrupt-map = <0 0 0 1 &pcie_intc0 0>,
> > +					<0 0 0 2 &pcie_intc0 1>,
> > +					<0 0 0 3 &pcie_intc0 2>,
> > +					<0 0 0 4 &pcie_intc0 3>;
> > +			pcie_intc0: interrupt-controller {
> >  				interrupt-controller;
> >  				#address-cells = <0>;
> >  				#interrupt-cells = <1>;
> > @@ -202,39 +219,30 @@ Examples for MT2712:
> >  
> >  Examples for MT7622:
> >  
> > -	pcie: pcie@1a140000 {
> > +	pcie0: pcie@1a143000 {
> >  		compatible = "mediatek,mt7622-pcie";
> >  		device_type = "pci";
> > -		reg = <0 0x1a140000 0 0x1000>,
> > -		      <0 0x1a143000 0 0x1000>,
> > -		      <0 0x1a145000 0 0x1000>;
> > -		reg-names = "subsys", "port0", "port1";
> > +		reg = <0 0x1a143000 0 0x1000>;
> > +		reg-names = "port0";
> >  		#address-cells = <3>;
> >  		#size-cells = <2>;
> > -		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>,
> > -			     <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-names = "pcie_irq";
> >  		clocks = <&pciesys CLK_PCIE_P0_MAC_EN>,
> > -			 <&pciesys CLK_PCIE_P1_MAC_EN>,
> >  			 <&pciesys CLK_PCIE_P0_AHB_EN>,
> > -			 <&pciesys CLK_PCIE_P1_AHB_EN>,
> >  			 <&pciesys CLK_PCIE_P0_AUX_EN>,
> > -			 <&pciesys CLK_PCIE_P1_AUX_EN>,
> >  			 <&pciesys CLK_PCIE_P0_AXI_EN>,
> > -			 <&pciesys CLK_PCIE_P1_AXI_EN>,
> >  			 <&pciesys CLK_PCIE_P0_OBFF_EN>,
> > -			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
> > -			 <&pciesys CLK_PCIE_P0_PIPE_EN>,
> > -			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
> > -		clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1",
> > -			      "aux_ck0", "aux_ck1", "axi_ck0", "axi_ck1",
> > -			      "obff_ck0", "obff_ck1", "pipe_ck0", "pipe_ck1";
> > -		phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
> > -		phy-names = "pcie-phy0", "pcie-phy1";
> > +			 <&pciesys CLK_PCIE_P0_PIPE_EN>;
> > +		clock-names = "sys_ck0", "ahb_ck0", "aux_ck0",
> > +			      "axi_ck0", "obff_ck0", "pipe_ck0";
> > +
> >  		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
> >  		bus-range = <0x00 0xff>;
> > -		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x10000000>;
> > +		ranges = <0x82000000 0 0x20000000  0x0 0x20000000  0 0x8000000>;
> > +		status = "disabled";
> >  
> > -		pcie0: pcie@0,0 {
> > +		slot0: pcie@0,0 {
> >  			reg = <0x0000 0 0 0 0>;
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> > @@ -251,8 +259,33 @@ Examples for MT7622:
> >  				#interrupt-cells = <1>;
> >  			};
> >  		};
> > +	};
> > +
> > +	pcie1: pcie@1a145000 {
> > +		compatible = "mediatek,mt7622-pcie";
> > +		device_type = "pci";
> > +		reg = <0 0x1a145000 0 0x1000>;
> > +		reg-names = "port1";
> > +		#address-cells = <3>;
> > +		#size-cells = <2>;
> > +		interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-names = "pcie_irq";
> > +		clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
> > +			 /* designer has connect RC1 with p0_ahb clock */
> > +			 <&pciesys CLK_PCIE_P0_AHB_EN>,
> > +			 <&pciesys CLK_PCIE_P1_AUX_EN>,
> > +			 <&pciesys CLK_PCIE_P1_AXI_EN>,
> > +			 <&pciesys CLK_PCIE_P1_OBFF_EN>,
> > +			 <&pciesys CLK_PCIE_P1_PIPE_EN>;
> > +		clock-names = "sys_ck1", "ahb_ck1", "aux_ck1",
> > +			      "axi_ck1", "obff_ck1", "pipe_ck1";
> > +
> > +		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
> > +		bus-range = <0x00 0xff>;
> > +		ranges = <0x82000000 0 0x28000000  0x0 0x28000000  0 0x8000000>;
> > +		status = "disabled";
> >  
> > -		pcie1: pcie@1,0 {
> > +		slot1: pcie@1,0 {
> >  			reg = <0x0800 0 0 0 0>;
> >  			#address-cells = <3>;
> >  			#size-cells = <2>;
> > -- 
> > 2.18.0
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


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

* Re: Aw: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design
  2020-09-14 11:39 ` Aw: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Frank Wunderlich
@ 2020-09-28  3:25   ` Chuanjia Liu
  0 siblings, 0 replies; 11+ messages in thread
From: Chuanjia Liu @ 2020-09-28  3:25 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi, Matthias Brugger,
	devicetree, Ryder Lee, linux-pci, linux-mediatek,
	linux-arm-kernel, yong.wu

On Mon, 2020-09-14 at 13:39 +0200, Frank Wunderlich wrote:
> > Betreff: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design
> 
> just if you need to make another version (as it is only the cover-letter) you can fix the typo in subject ;)

Thanks for your reminding, I will change it in the next version.
> 
> regards Frank
> 


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

* Re: [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq
  2020-09-14 11:26 ` [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq Chuanjia Liu
@ 2020-09-30 15:23   ` Rob Herring
  2020-10-09 12:53     ` Chuanjia Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-09-30 15:23 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Matthias Brugger, linux-pci,
	linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee

On Mon, Sep 14, 2020 at 07:26:57PM +0800, Chuanjia Liu wrote:
> Add new method to get shared pcie-cfg base and pcie irq for
> new dts format.
> 
> Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index cf4c18f0c25a..5b915eb0cf1e 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -14,6 +14,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/msi.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
> @@ -23,6 +24,7 @@
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>  #include <linux/reset.h>
>  
>  #include "../pci.h"
> @@ -205,6 +207,7 @@ struct mtk_pcie_port {
>   * struct mtk_pcie - PCIe host information
>   * @dev: pointer to PCIe device
>   * @base: IO mapped register base
> + * @cfg: IO mapped register map for PCIe config
>   * @free_ck: free-run reference clock
>   * @mem: non-prefetchable memory resource
>   * @ports: pointer to PCIe port information
> @@ -213,6 +216,7 @@ struct mtk_pcie_port {
>  struct mtk_pcie {
>  	struct device *dev;
>  	void __iomem *base;
> +	struct regmap *cfg;
>  	struct clk *free_ck;
>  
>  	struct list_head ports;
> @@ -648,7 +652,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
>  		return err;
>  	}
>  
> -	port->irq = platform_get_irq(pdev, port->slot);
> +	if (of_find_property(dev->of_node, "interrupt-names", NULL))
> +		port->irq = platform_get_irq_byname(pdev, "pcie_irq");

Not really any point in having a name with a single interrupt.

> +	else
> +		port->irq = platform_get_irq(pdev, port->slot);

With the new binding, slot is always 0, right? Then you don't need any 
change here.

> +
>  	if (port->irq < 0)
>  		return port->irq;
>  
> @@ -680,6 +688,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		val |= PCIE_CSR_LTSSM_EN(port->slot) |
>  		       PCIE_CSR_ASPM_L1_EN(port->slot);
>  		writel(val, pcie->base + PCIE_SYS_CFG_V2);
> +	} else if (pcie->cfg) {
> +		val = PCIE_CSR_LTSSM_EN(port->slot) |
> +		      PCIE_CSR_ASPM_L1_EN(port->slot);
> +		regmap_update_bits(pcie->cfg, PCIE_SYS_CFG_V2, val, val);
>  	}
>  
>  	/* Assert all reset signals */
> @@ -983,6 +995,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
>  	struct device *dev = pcie->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct resource *regs;
> +	struct device_node *cfg_node;
>  	int err;
>  
>  	/* get shared registers, which are optional */
> @@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
>  		}
>  	}
>  
> +	cfg_node = of_find_compatible_node(NULL, NULL,
> +					   "mediatek,generic-pciecfg");
> +	if (cfg_node) {
> +		pcie->cfg = syscon_node_to_regmap(cfg_node);
> +		if (IS_ERR(pcie->cfg))
> +			return PTR_ERR(pcie->cfg);
> +	}
> +
>  	pcie->free_ck = devm_clk_get(dev, "free_ck");
>  	if (IS_ERR(pcie->free_ck)) {
>  		if (PTR_ERR(pcie->free_ck) == -EPROBE_DEFER)
> -- 
> 2.18.0

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

* Re: [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq
  2020-09-30 15:23   ` Rob Herring
@ 2020-10-09 12:53     ` Chuanjia Liu
  0 siblings, 0 replies; 11+ messages in thread
From: Chuanjia Liu @ 2020-10-09 12:53 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Matthias Brugger, linux-pci,
	linux-mediatek, devicetree, linux-arm-kernel, yong.wu,
	Frank Wunderlich, Ryder Lee

On Wed, 2020-09-30 at 10:23 -0500, Rob Herring wrote:
> On Mon, Sep 14, 2020 at 07:26:57PM +0800, Chuanjia Liu wrote:
> > Add new method to get shared pcie-cfg base and pcie irq for
> > new dts format.
> > 
> > Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index cf4c18f0c25a..5b915eb0cf1e 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/irqchip/chained_irq.h>
> >  #include <linux/irqdomain.h>
> >  #include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/msi.h>
> >  #include <linux/module.h>
> >  #include <linux/of_address.h>
> > @@ -23,6 +24,7 @@
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> >  #include <linux/reset.h>
> >  
> >  #include "../pci.h"
> > @@ -205,6 +207,7 @@ struct mtk_pcie_port {
> >   * struct mtk_pcie - PCIe host information
> >   * @dev: pointer to PCIe device
> >   * @base: IO mapped register base
> > + * @cfg: IO mapped register map for PCIe config
> >   * @free_ck: free-run reference clock
> >   * @mem: non-prefetchable memory resource
> >   * @ports: pointer to PCIe port information
> > @@ -213,6 +216,7 @@ struct mtk_pcie_port {
> >  struct mtk_pcie {
> >  	struct device *dev;
> >  	void __iomem *base;
> > +	struct regmap *cfg;
> >  	struct clk *free_ck;
> >  
> >  	struct list_head ports;
> > @@ -648,7 +652,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
> >  		return err;
> >  	}
> >  
> > -	port->irq = platform_get_irq(pdev, port->slot);
> > +	if (of_find_property(dev->of_node, "interrupt-names", NULL))
> > +		port->irq = platform_get_irq_byname(pdev, "pcie_irq");
> 
> Not really any point in having a name with a single interrupt.
> 
> > +	else
> > +		port->irq = platform_get_irq(pdev, port->slot);
> 
> With the new binding, slot is always 0, right? Then you don't need any 
> change here.
In the new binding, PCIe1 slot number is 1.
Because some setting in the driver is based on slot number to determine
offset, this is to reduce driver changes and be compatible with new and
old DTS format.
> 
> > +
> >  	if (port->irq < 0)
> >  		return port->irq;
> >  
> > @@ -680,6 +688,10 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		val |= PCIE_CSR_LTSSM_EN(port->slot) |
> >  		       PCIE_CSR_ASPM_L1_EN(port->slot);
> >  		writel(val, pcie->base + PCIE_SYS_CFG_V2);
> > +	} else if (pcie->cfg) {
> > +		val = PCIE_CSR_LTSSM_EN(port->slot) |
> > +		      PCIE_CSR_ASPM_L1_EN(port->slot);
> > +		regmap_update_bits(pcie->cfg, PCIE_SYS_CFG_V2, val, val);
> >  	}
> >  
> >  	/* Assert all reset signals */
> > @@ -983,6 +995,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
> >  	struct device *dev = pcie->dev;
> >  	struct platform_device *pdev = to_platform_device(dev);
> >  	struct resource *regs;
> > +	struct device_node *cfg_node;
> >  	int err;
> >  
> >  	/* get shared registers, which are optional */
> > @@ -995,6 +1008,14 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
> >  		}
> >  	}
> >  
> > +	cfg_node = of_find_compatible_node(NULL, NULL,
> > +					   "mediatek,generic-pciecfg");
> > +	if (cfg_node) {
> > +		pcie->cfg = syscon_node_to_regmap(cfg_node);
> > +		if (IS_ERR(pcie->cfg))
> > +			return PTR_ERR(pcie->cfg);
> > +	}
> > +
> >  	pcie->free_ck = devm_clk_get(dev, "free_ck");
> >  	if (IS_ERR(pcie->free_ck)) {
> >  		if (PTR_ERR(pcie->free_ck) == -EPROBE_DEFER)
> > -- 
> > 2.18.0


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

end of thread, other threads:[~2020-10-09 12:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 11:26 [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Chuanjia Liu
2020-09-14 11:26 ` [PATCH v6 1/4] dt-bindings: pci: mediatek: Modified the Device tree bindings Chuanjia Liu
2020-09-22 23:31   ` Rob Herring
2020-09-28  3:18     ` Chuanjia Liu
2020-09-14 11:26 ` [PATCH v6 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base and irq Chuanjia Liu
2020-09-30 15:23   ` Rob Herring
2020-10-09 12:53     ` Chuanjia Liu
2020-09-14 11:26 ` [PATCH v6 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622 Chuanjia Liu
2020-09-14 11:26 ` [PATCH v6 4/4] ARM: dts: mediatek: Modified MT7629 PCIe node Chuanjia Liu
2020-09-14 11:39 ` Aw: [PATCH v6 0/4] Spilt PCIe node to comply with hardware design Frank Wunderlich
2020-09-28  3:25   ` Chuanjia Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).