LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design
@ 2021-07-19  7:34 Chuanjia Liu
  2021-07-19  7:34 ` [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings Chuanjia Liu
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-19  7:34 UTC (permalink / raw)
  To: robh+dt, bhelgaas, matthias.bgg, lorenzo.pieralisi
  Cc: ryder.lee, jianjun.wang, yong.wu, Frank Wunderlich, chuanjia.liu,
	linux-pci, linux-mediatek, devicetree, linux-arm-kernel,
	linux-kernel

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

In old dts 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 comply with 
the hardware design and fix MSI issue.

change note:
  v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain" 
      description in binding file. No code change.
  v10:Rebase for 5.13-rc1, no code change. 
  v9:fix kernel-ci bot warning. In the scene of using new dts format,
     when mtk_pcie_parse_port fails, of_node_put don't need to be called.
  v8:remove slot node and fix yaml warning.
  v7:dt-bindings file was modified as suggested by Rob, other file no
     change.
  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: Update the Device tree bindings
  PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622
  ARM: dts: mediatek: Update MT7629 PCIe node for new format

  .../bindings/pci/mediatek-pcie-cfg.yaml       |  39 ++++
  .../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
  arch/arm/boot/dts/mt7629-rfb.dts              |   3 +-
  arch/arm/boot/dts/mt7629.dtsi                 |  45 ++--
  arch/arm64/boot/dts/mediatek/mt2712e.dtsi     |  97 +++++----
  .../dts/mediatek/mt7622-bananapi-bpi-r64.dts  |  16 +-
  arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts  |   6 +-
  arch/arm64/boot/dts/mediatek/mt7622.dtsi      | 112 +++++-----
  drivers/pci/controller/pcie-mediatek.c        |  52 +++--
  9 files changed, 330 insertions(+), 246 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml

--
2.18.0



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

* [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings
  2021-07-19  7:34 [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Chuanjia Liu
@ 2021-07-19  7:34 ` Chuanjia Liu
  2021-07-19 22:47   ` Rob Herring
  2021-07-19  7:34 ` [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node Chuanjia Liu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-19  7:34 UTC (permalink / raw)
  To: robh+dt, bhelgaas, matthias.bgg, lorenzo.pieralisi
  Cc: ryder.lee, jianjun.wang, yong.wu, Frank Wunderlich, chuanjia.liu,
	linux-pci, linux-mediatek, devicetree, linux-arm-kernel,
	linux-kernel

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

In old dts 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 comply with
the 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       |  39 ++++
 .../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
 2 files changed, 150 insertions(+), 95 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..841a3d284bbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
@@ -0,0 +1,39 @@
+# 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..57ae73462272 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)
@@ -47,9 +47,12 @@ 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
+- interrupt-names: Must include the following entries:
+	- "pcie_irq": The interrupt that is asserted when an MSI/INTX is received
+- linux,pci-domain: PCI domain ID. Should be unique for each host controller
 
 In addition, the device tree node must have sub-nodes describing each
 PCIe port interface, having the following mandatory properties:
@@ -143,130 +146,143 @@ 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";
+		linux,pci-domain = <1>;
 		#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>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+		#interrupt-cells = <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>;
 			#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-controller;
-				#address-cells = <0>;
-				#interrupt-cells = <1>;
-			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
-			reg = <0x0800 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+	pcie0: pcie@11700000 {
+		compatible = "mediatek,mt2712-pcie";
+		device_type = "pci";
+		reg = <0 0x11700000 0 0x1000>;
+		reg-names = "port0";
+		linux,pci-domain = <0>;
+		#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";
+
+		#interrupt-cells = <1>;
+		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-controller;
+			#address-cells = <0>;
 			#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-controller;
-				#address-cells = <0>;
-				#interrupt-cells = <1>;
-			};
 		};
 	};
 
 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";
+		linux,pci-domain = <0>;
 		#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 {
-			reg = <0x0000 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+		#interrupt-cells = <1>;
+		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-controller;
+			#address-cells = <0>;
 			#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-controller;
-				#address-cells = <0>;
-				#interrupt-cells = <1>;
-			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
-			reg = <0x0800 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+	pcie1: pcie@1a145000 {
+		compatible = "mediatek,mt7622-pcie";
+		device_type = "pci";
+		reg = <0 0x1a145000 0 0x1000>;
+		reg-names = "port1";
+		linux,pci-domain = <1>;
+		#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";
+
+		#interrupt-cells = <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>;
 			#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-controller;
-				#address-cells = <0>;
-				#interrupt-cells = <1>;
-			};
 		};
 	};
-- 
2.18.0


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

* [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-07-19  7:34 [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Chuanjia Liu
  2021-07-19  7:34 ` [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings Chuanjia Liu
@ 2021-07-19  7:34 ` Chuanjia Liu
  2021-07-20  2:59   ` Chuanjia Liu
                     ` (2 more replies)
  2021-07-19  7:34 ` [PATCH v11 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622 Chuanjia Liu
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-19  7:34 UTC (permalink / raw)
  To: robh+dt, bhelgaas, matthias.bgg, lorenzo.pieralisi
  Cc: ryder.lee, jianjun.wang, yong.wu, Frank Wunderlich, chuanjia.liu,
	linux-pci, linux-mediatek, devicetree, linux-arm-kernel,
	linux-kernel

For the new dts format, add a new method to get
shared pcie-cfg base address and parse node.

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

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 25bee693834f..928e0983a900 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"
@@ -207,6 +209,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
@@ -215,6 +218,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;
@@ -650,7 +654,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;
 
@@ -682,6 +690,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 */
@@ -985,6 +997,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)
 			return PTR_ERR(pcie->base);
 	}
 
+	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)
@@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
 	struct device *dev = pcie->dev;
 	struct device_node *node = dev->of_node, *child;
 	struct mtk_pcie_port *port, *tmp;
-	int err;
+	int err, slot;
+
+	slot = of_get_pci_domain_nr(dev->of_node);
+	if (slot < 0) {
+		for_each_available_child_of_node(node, child) {
+			err = of_pci_get_devfn(child);
+			if (err < 0) {
+				dev_err(dev, "failed to get devfn: %d\n", err);
+				goto error_put_node;
+			}
 
-	for_each_available_child_of_node(node, child) {
-		int slot;
+			slot = PCI_SLOT(err);
 
-		err = of_pci_get_devfn(child);
-		if (err < 0) {
-			dev_err(dev, "failed to parse devfn: %d\n", err);
-			goto error_put_node;
+			err = mtk_pcie_parse_port(pcie, child, slot);
+			if (err)
+				goto error_put_node;
 		}
-
-		slot = PCI_SLOT(err);
-
-		err = mtk_pcie_parse_port(pcie, child, slot);
+	} else {
+		err = mtk_pcie_parse_port(pcie, node, slot);
 		if (err)
-			goto error_put_node;
+			return err;
 	}
 
 	err = mtk_pcie_subsys_powerup(pcie);
-- 
2.18.0


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

* [PATCH v11 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622
  2021-07-19  7:34 [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Chuanjia Liu
  2021-07-19  7:34 ` [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings Chuanjia Liu
  2021-07-19  7:34 ` [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node Chuanjia Liu
@ 2021-07-19  7:34 ` Chuanjia Liu
  2021-07-19  7:34 ` [PATCH v11 4/4] ARM: dts: mediatek: Update MT7629 PCIe node for new format Chuanjia Liu
  2021-08-06  9:39 ` [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Lorenzo Pieralisi
  4 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-19  7:34 UTC (permalink / raw)
  To: robh+dt, bhelgaas, matthias.bgg, lorenzo.pieralisi
  Cc: ryder.lee, jianjun.wang, yong.wu, Frank Wunderlich, chuanjia.liu,
	linux-pci, linux-mediatek, devicetree, linux-arm-kernel,
	linux-kernel

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

In old dts 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 comply with
the hardware design and fix MSI issue.

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

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index a9cca9c146fd..de16c0d80c30 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -915,64 +915,67 @@
 		};
 	};
 
-	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";
+		linux,pci-domain = <1>;
 		#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>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+		#interrupt-cells = <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>;
 			#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-controller;
-				#address-cells = <0>;
-				#interrupt-cells = <1>;
-			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
-			device_type = "pci";
-			status = "disabled";
-			reg = <0x0800 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+	pcie0: pcie@11700000 {
+		compatible = "mediatek,mt2712-pcie";
+		device_type = "pci";
+		reg = <0 0x11700000 0 0x1000>;
+		reg-names = "port0";
+		linux,pci-domain = <0>;
+		#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";
+
+		#interrupt-cells = <1>;
+		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-controller;
+			#address-cells = <0>;
 			#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-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 2f77dc40b9b8..2b9bf8dd14ec 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -257,18 +257,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 f2dc850010f1..596c073d8b05 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -234,14 +234,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 890a942ec608..6f8cb3ad1e84 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -781,75 +781,83 @@
 		#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";
+		linux,pci-domain = <0>;
 		#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 {
-			reg = <0x0000 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+		#interrupt-cells = <1>;
+		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-controller;
+			#address-cells = <0>;
 			#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>,
-					<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>;
-			};
 		};
+	};
 
-		pcie1: pcie@1,0 {
-			reg = <0x0800 0 0 0 0>;
-			#address-cells = <3>;
-			#size-cells = <2>;
+	pcie1: pcie@1a145000 {
+		compatible = "mediatek,mt7622-pcie";
+		device_type = "pci";
+		reg = <0 0x1a145000 0 0x1000>;
+		reg-names = "port1";
+		linux,pci-domain = <1>;
+		#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";
+
+		#interrupt-cells = <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>;
 			#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>,
-					<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>;
-			};
 		};
 	};
 
-- 
2.18.0


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

* [PATCH v11 4/4] ARM: dts: mediatek: Update MT7629 PCIe node for new format
  2021-07-19  7:34 [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Chuanjia Liu
                   ` (2 preceding siblings ...)
  2021-07-19  7:34 ` [PATCH v11 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622 Chuanjia Liu
@ 2021-07-19  7:34 ` Chuanjia Liu
  2021-08-06  9:39 ` [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Lorenzo Pieralisi
  4 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-19  7:34 UTC (permalink / raw)
  To: robh+dt, bhelgaas, matthias.bgg, lorenzo.pieralisi
  Cc: ryder.lee, jianjun.wang, yong.wu, Frank Wunderlich, chuanjia.liu,
	linux-pci, linux-mediatek, devicetree, linux-arm-kernel,
	linux-kernel

To match the new dts binding. Remove "subsys",unused
interrupt and slot node.Add "interrupt-names",
"linux,pci-domain" and 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    | 45 +++++++++++++++-----------------
 2 files changed, 23 insertions(+), 25 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 874043f0490d..46fc236e1b89 100644
--- a/arch/arm/boot/dts/mt7629.dtsi
+++ b/arch/arm/boot/dts/mt7629.dtsi
@@ -361,16 +361,21 @@
 			#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";
+			linux,pci-domain = <1>;
 			#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>,
@@ -391,26 +396,18 @@
 			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";
-				reg = <0x0800 0 0 0 0>;
-				#address-cells = <3>;
-				#size-cells = <2>;
+			#interrupt-cells = <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>;
 				#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>;
-					#interrupt-cells = <1>;
-				};
 			};
 		};
 
-- 
2.18.0


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

* Re: [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings
  2021-07-19  7:34 ` [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings Chuanjia Liu
@ 2021-07-19 22:47   ` Rob Herring
  2021-07-20  2:07     ` Chuanjia Liu
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2021-07-19 22:47 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: linux-kernel, lorenzo.pieralisi, jianjun.wang, linux-mediatek,
	linux-arm-kernel, bhelgaas, ryder.lee, robh+dt, matthias.bgg,
	linux-pci, Frank Wunderlich, devicetree, yong.wu

On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
> There are two independent PCIe controllers in MT2712 and MT7622
> platform. Each of them should contain an independent MSI domain.
> 
> In old dts 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 comply with
> the 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       |  39 ++++
>  .../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
>  2 files changed, 150 insertions(+), 95 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.


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

* Re: [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings
  2021-07-19 22:47   ` Rob Herring
@ 2021-07-20  2:07     ` Chuanjia Liu
  2021-07-20 16:26       ` Rob Herring
  0 siblings, 1 reply; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-20  2:07 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger, Lorenzo Pieralisi, Bjorn Helgaas
  Cc: linux-kernel, jianjun.wang, linux-mediatek, linux-arm-kernel,
	ryder.lee, linux-pci, Frank Wunderlich, devicetree, yong.wu,
	Rob Herring

On Mon, 2021-07-19 at 16:47 -0600, Rob Herring wrote:
> On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
> > There are two independent PCIe controllers in MT2712 and MT7622
> > platform. Each of them should contain an independent MSI domain.
> > 
> > In old dts 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 comply with
> > the 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       |  39 ++++
> >  .../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
> >  2 files changed, 150 insertions(+), 95 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> > 
> 
> 
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
> 
> If a tag was not added on purpose, please state why and what changed.
> 
Hi,Rob
I have described in the cover letter:
v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain" 
    description in binding file. No code change.
if you still ok for this, I will add R-b in next version.

Best regards
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek


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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-07-19  7:34 ` [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node Chuanjia Liu
@ 2021-07-20  2:59   ` Chuanjia Liu
  2021-08-03 22:18     ` Rob Herring
  2021-08-02  7:07   ` Chuanjia Liu
  2021-08-10 19:42   ` Bjorn Helgaas
  2 siblings, 1 reply; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-20  2:59 UTC (permalink / raw)
  To: robh+dt, Bjorn Helgaas, Matthias Brugger, Lorenzo Pieralisi
  Cc: bhelgaas, matthias.bgg, lorenzo.pieralisi, ryder.lee,
	jianjun.wang, yong.wu, Frank Wunderlich, linux-pci,
	linux-mediatek, devicetree, linux-arm-kernel, linux-kernel,
	Chuanjia Liu

On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
> For the new dts format, add a new method to get
> shared pcie-cfg base address and parse node.
> 
> Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 25bee693834f..928e0983a900 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"
> @@ -207,6 +209,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
> @@ -215,6 +218,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;
> @@ -650,7 +654,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;
>  
> @@ -682,6 +690,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 */
> @@ -985,6 +997,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)
>  			return PTR_ERR(pcie->base);
>  	}
>  
> +	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)
> @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
>  	struct device *dev = pcie->dev;
>  	struct device_node *node = dev->of_node, *child;
>  	struct mtk_pcie_port *port, *tmp;
> -	int err;
> +	int err, slot;
> +
> +	slot = of_get_pci_domain_nr(dev->of_node);
> +	if (slot < 0) {
> +		for_each_available_child_of_node(node, child) {
> +			err = of_pci_get_devfn(child);
> +			if (err < 0) {
> +				dev_err(dev, "failed to get devfn: %d\n", err);
> +				goto error_put_node;
> +			}
>  
> -	for_each_available_child_of_node(node, child) {
> -		int slot;
> +			slot = PCI_SLOT(err);
>  
> -		err = of_pci_get_devfn(child);
> -		if (err < 0) {
> -			dev_err(dev, "failed to parse devfn: %d\n", err);
> -			goto error_put_node;
> +			err = mtk_pcie_parse_port(pcie, child, slot);
> +			if (err)
> +				goto error_put_node;
>  		}
> -
> -		slot = PCI_SLOT(err);
> -
> -		err = mtk_pcie_parse_port(pcie, child, slot);
> +	} else {
> +		err = mtk_pcie_parse_port(pcie, node, slot);
>  		if (err)
> -			goto error_put_node;
> +			return err;

Hi,Rob
I changed this in the v9 version:
When the new dts format is used, of_node_get() is not called.
So when mtk_pcie_parse_port fails, of_node_put don't need to be called.
if you still ok for this, I will add R-b in next version.

Best Regards
>  	}
>  
>  	err = mtk_pcie_subsys_powerup(pcie);


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

* Re: [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings
  2021-07-20  2:07     ` Chuanjia Liu
@ 2021-07-20 16:26       ` Rob Herring
  2021-07-23  7:17         ` Chuanjia Liu
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2021-07-20 16:26 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: Matthias Brugger, Lorenzo Pieralisi, Bjorn Helgaas, linux-kernel,
	Jianjun Wang, moderated list:ARM/Mediatek SoC support,
	linux-arm-kernel, Ryder Lee, PCI, Frank Wunderlich, devicetree,
	Yong Wu

On Mon, Jul 19, 2021 at 8:07 PM Chuanjia Liu <chuanjia.liu@mediatek.com> wrote:
>
> On Mon, 2021-07-19 at 16:47 -0600, Rob Herring wrote:
> > On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
> > > There are two independent PCIe controllers in MT2712 and MT7622
> > > platform. Each of them should contain an independent MSI domain.
> > >
> > > In old dts 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 comply with
> > > the 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       |  39 ++++
> > >  .../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
> > >  2 files changed, 150 insertions(+), 95 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> > >
> >
> >
> > Please add Acked-by/Reviewed-by tags when posting new versions. However,
> > there's no need to repost patches *only* to add the tags. The upstream
> > maintainer will do that for acks received on the version they apply.
> >
> > If a tag was not added on purpose, please state why and what changed.
> >
> Hi,Rob
> I have described in the cover letter:
> v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain"
>     description in binding file. No code change.
> if you still ok for this, I will add R-b in next version.

Yes, it's fine.

In the future, put the changelog for a patch in the patch.

Rob

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

* Re: [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings
  2021-07-20 16:26       ` Rob Herring
@ 2021-07-23  7:17         ` Chuanjia Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu @ 2021-07-23  7:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: Matthias Brugger, Lorenzo Pieralisi, Bjorn Helgaas, linux-kernel,
	Jianjun Wang, moderated list:ARM/Mediatek SoC support,
	linux-arm-kernel, Ryder Lee, PCI, Frank Wunderlich, devicetree,
	Yong Wu

On Tue, 2021-07-20 at 10:26 -0600, Rob Herring wrote:
> On Mon, Jul 19, 2021 at 8:07 PM Chuanjia Liu <chuanjia.liu@mediatek.com> wrote:
> >
> > On Mon, 2021-07-19 at 16:47 -0600, Rob Herring wrote:
> > > On Mon, 19 Jul 2021 15:34:53 +0800, Chuanjia Liu wrote:
> > > > There are two independent PCIe controllers in MT2712 and MT7622
> > > > platform. Each of them should contain an independent MSI domain.
> > > >
> > > > In old dts 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 comply with
> > > > the 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       |  39 ++++
> > > >  .../devicetree/bindings/pci/mediatek-pcie.txt | 206 ++++++++++--------
> > > >  2 files changed, 150 insertions(+), 95 deletions(-)
> > > >  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
> > > >
> > >
> > >
> > > Please add Acked-by/Reviewed-by tags when posting new versions. However,
> > > there's no need to repost patches *only* to add the tags. The upstream
> > > maintainer will do that for acks received on the version they apply.
> > >
> > > If a tag was not added on purpose, please state why and what changed.
> > >
> > Hi,Rob
> > I have described in the cover letter:
> > v11:Rebase for 5.14-rc1 and add "interrupt-names", "linux,pci-domain"
> >     description in binding file. No code change.
> > if you still ok for this, I will add R-b in next version.
> 
> Yes, it's fine.
> 
> In the future, put the changelog for a patch in the patch.
Hi, Rob
 Thanks for you suggestion, I will do this in future versions.
Best regards



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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-07-19  7:34 ` [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node Chuanjia Liu
  2021-07-20  2:59   ` Chuanjia Liu
@ 2021-08-02  7:07   ` Chuanjia Liu
  2021-08-10 19:42   ` Bjorn Helgaas
  2 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu @ 2021-08-02  7:07 UTC (permalink / raw)
  To: robh+dt, Bjorn Helgaas, Matthias Brugger, Lorenzo Pieralisi
  Cc: ryder.lee, jianjun.wang, yong.wu, Frank Wunderlich, linux-pci,
	linux-mediatek, devicetree, linux-arm-kernel, linux-kernel,
	chuanjia.liu

On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:

Gently ping...
> For the new dts format, add a new method to get
> shared pcie-cfg base address and parse node.
> 
> Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 25bee693834f..928e0983a900 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"
> @@ -207,6 +209,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
> @@ -215,6 +218,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;
> @@ -650,7 +654,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;
>  
> @@ -682,6 +690,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 */
> @@ -985,6 +997,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)
>  			return PTR_ERR(pcie->base);
>  	}
>  
> +	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)
> @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
>  	struct device *dev = pcie->dev;
>  	struct device_node *node = dev->of_node, *child;
>  	struct mtk_pcie_port *port, *tmp;
> -	int err;
> +	int err, slot;
> +
> +	slot = of_get_pci_domain_nr(dev->of_node);
> +	if (slot < 0) {
> +		for_each_available_child_of_node(node, child) {
> +			err = of_pci_get_devfn(child);
> +			if (err < 0) {
> +				dev_err(dev, "failed to get devfn: %d\n", err);
> +				goto error_put_node;
> +			}
>  
> -	for_each_available_child_of_node(node, child) {
> -		int slot;
> +			slot = PCI_SLOT(err);
>  
> -		err = of_pci_get_devfn(child);
> -		if (err < 0) {
> -			dev_err(dev, "failed to parse devfn: %d\n", err);
> -			goto error_put_node;
> +			err = mtk_pcie_parse_port(pcie, child, slot);
> +			if (err)
> +				goto error_put_node;
>  		}
> -
> -		slot = PCI_SLOT(err);
> -
> -		err = mtk_pcie_parse_port(pcie, child, slot);
> +	} else {
> +		err = mtk_pcie_parse_port(pcie, node, slot);
>  		if (err)
> -			goto error_put_node;
> +			return err;
>  	}
>  
>  	err = mtk_pcie_subsys_powerup(pcie);


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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-07-20  2:59   ` Chuanjia Liu
@ 2021-08-03 22:18     ` Rob Herring
  2021-08-06  7:37       ` Chuanjia Liu (柳传嘉)
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2021-08-03 22:18 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: Bjorn Helgaas, Matthias Brugger, Lorenzo Pieralisi, Ryder Lee,
	Jianjun Wang, Yong Wu, Frank Wunderlich, PCI,
	moderated list:ARM/Mediatek SoC support, devicetree,
	linux-arm-kernel, linux-kernel

On Mon, Jul 19, 2021 at 8:59 PM Chuanjia Liu <chuanjia.liu@mediatek.com> wrote:
>
> On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
> > For the new dts format, add a new method to get
> > shared pcie-cfg base address and parse node.
> >
> > Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
> >  1 file changed, 39 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 25bee693834f..928e0983a900 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"
> > @@ -207,6 +209,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
> > @@ -215,6 +218,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;
> > @@ -650,7 +654,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;
> >
> > @@ -682,6 +690,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 */
> > @@ -985,6 +997,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)
> >                       return PTR_ERR(pcie->base);
> >       }
> >
> > +     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)
> > @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
> >       struct device *dev = pcie->dev;
> >       struct device_node *node = dev->of_node, *child;
> >       struct mtk_pcie_port *port, *tmp;
> > -     int err;
> > +     int err, slot;
> > +
> > +     slot = of_get_pci_domain_nr(dev->of_node);
> > +     if (slot < 0) {
> > +             for_each_available_child_of_node(node, child) {
> > +                     err = of_pci_get_devfn(child);
> > +                     if (err < 0) {
> > +                             dev_err(dev, "failed to get devfn: %d\n", err);
> > +                             goto error_put_node;
> > +                     }
> >
> > -     for_each_available_child_of_node(node, child) {
> > -             int slot;
> > +                     slot = PCI_SLOT(err);
> >
> > -             err = of_pci_get_devfn(child);
> > -             if (err < 0) {
> > -                     dev_err(dev, "failed to parse devfn: %d\n", err);
> > -                     goto error_put_node;
> > +                     err = mtk_pcie_parse_port(pcie, child, slot);
> > +                     if (err)
> > +                             goto error_put_node;
> >               }
> > -
> > -             slot = PCI_SLOT(err);
> > -
> > -             err = mtk_pcie_parse_port(pcie, child, slot);
> > +     } else {
> > +             err = mtk_pcie_parse_port(pcie, node, slot);
> >               if (err)
> > -                     goto error_put_node;
> > +                     return err;
>
> Hi,Rob
> I changed this in the v9 version:
> When the new dts format is used, of_node_get() is not called.
> So when mtk_pcie_parse_port fails, of_node_put don't need to be called.
> if you still ok for this, I will add R-b in next version.

Yes, and that's small enough change to keep my R-b.

Rob

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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-08-03 22:18     ` Rob Herring
@ 2021-08-06  7:37       ` Chuanjia Liu (柳传嘉)
  0 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu (柳传嘉) @ 2021-08-06  7:37 UTC (permalink / raw)
  To: robh+dt
  Cc: linux-mediatek, linux-kernel, devicetree, lorenzo.pieralisi,
	Chuanjia Liu (柳传嘉),
	frank-w, Yong Wu (吴勇),
	Jianjun Wang (王建军),
	linux-arm-kernel, linux-pci, matthias.bgg, bhelgaas,
	Ryder Lee (李庚諺)

On Tue, 2021-08-03 at 16:18 -0600, Rob Herring wrote:
> On Mon, Jul 19, 2021 at 8:59 PM Chuanjia Liu <
> chuanjia.liu@mediatek.com> wrote:
> > 
> > On Mon, 2021-07-19 at 15:34 +0800, Chuanjia Liu wrote:
> > > For the new dts format, add a new method to get
> > > shared pcie-cfg base address and parse node.
> > > 
> > > Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> > > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-
> > > ------
> > >  1 file changed, 39 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c
> > > b/drivers/pci/controller/pcie-mediatek.c
> > > index 25bee693834f..928e0983a900 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"
> > > @@ -207,6 +209,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
> > > @@ -215,6 +218,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;
> > > @@ -650,7 +654,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;
> > > 
> > > @@ -682,6 +690,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 */
> > > @@ -985,6 +997,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)
> > >                       return PTR_ERR(pcie->base);
> > >       }
> > > 
> > > +     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)
> > > @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie
> > > *pcie)
> > >       struct device *dev = pcie->dev;
> > >       struct device_node *node = dev->of_node, *child;
> > >       struct mtk_pcie_port *port, *tmp;
> > > -     int err;
> > > +     int err, slot;
> > > +
> > > +     slot = of_get_pci_domain_nr(dev->of_node);
> > > +     if (slot < 0) {
> > > +             for_each_available_child_of_node(node, child) {
> > > +                     err = of_pci_get_devfn(child);
> > > +                     if (err < 0) {
> > > +                             dev_err(dev, "failed to get devfn:
> > > %d\n", err);
> > > +                             goto error_put_node;
> > > +                     }
> > > 
> > > -     for_each_available_child_of_node(node, child) {
> > > -             int slot;
> > > +                     slot = PCI_SLOT(err);
> > > 
> > > -             err = of_pci_get_devfn(child);
> > > -             if (err < 0) {
> > > -                     dev_err(dev, "failed to parse devfn: %d\n",
> > > err);
> > > -                     goto error_put_node;
> > > +                     err = mtk_pcie_parse_port(pcie, child,
> > > slot);
> > > +                     if (err)
> > > +                             goto error_put_node;
> > >               }
> > > -
> > > -             slot = PCI_SLOT(err);
> > > -
> > > -             err = mtk_pcie_parse_port(pcie, child, slot);
> > > +     } else {
> > > +             err = mtk_pcie_parse_port(pcie, node, slot);
> > >               if (err)
> > > -                     goto error_put_node;
> > > +                     return err;
> > 
> > Hi,Rob
> > I changed this in the v9 version:
> > When the new dts format is used, of_node_get() is not called.
> > So when mtk_pcie_parse_port fails, of_node_put don't need to be
> > called.
> > if you still ok for this, I will add R-b in next version.
> 
> Yes, and that's small enough change to keep my R-b.
> 
> Rob

Hi, Rob
Thanks for your review.

Hi lorenzo
If there are no more comments, can this series be merged?

Best Regards

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

* Re: [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design
  2021-07-19  7:34 [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Chuanjia Liu
                   ` (3 preceding siblings ...)
  2021-07-19  7:34 ` [PATCH v11 4/4] ARM: dts: mediatek: Update MT7629 PCIe node for new format Chuanjia Liu
@ 2021-08-06  9:39 ` Lorenzo Pieralisi
  2021-08-08  4:50   ` Chuanjia Liu (柳传嘉)
  4 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Pieralisi @ 2021-08-06  9:39 UTC (permalink / raw)
  To: Chuanjia Liu, matthias.bgg, bhelgaas, robh+dt
  Cc: Lorenzo Pieralisi, linux-pci, linux-mediatek, devicetree,
	yong.wu, linux-kernel, ryder.lee, Frank Wunderlich,
	linux-arm-kernel, jianjun.wang

On Mon, 19 Jul 2021 15:34:52 +0800, Chuanjia Liu wrote:
> There are two independent PCIe controllers in MT2712 and MT7622 platform.
> Each of them should contain an independent MSI domain.
> 
> In old dts 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.
> 
> [...]

Applied patches 1-2 to pci/mediatek (we don't merge dts changes), thanks!

[1/2] dt-bindings: PCI: mediatek: Update the Device tree bindings
      https://git.kernel.org/lpieralisi/pci/c/9c23251640
[2/2] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
      https://git.kernel.org/lpieralisi/pci/c/302e503e08

Thanks,
Lorenzo

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

* Re: [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design
  2021-08-06  9:39 ` [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Lorenzo Pieralisi
@ 2021-08-08  4:50   ` Chuanjia Liu (柳传嘉)
  0 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu (柳传嘉) @ 2021-08-08  4:50 UTC (permalink / raw)
  To: matthias.bgg, lorenzo.pieralisi, bhelgaas, robh+dt
  Cc: linux-kernel, linux-mediatek, devicetree,
	Chuanjia Liu (柳传嘉),
	frank-w, Yong Wu (吴勇),
	Jianjun Wang (王建军),
	linux-arm-kernel, linux-pci, Ryder Lee (李庚諺)

On Fri, 2021-08-06 at 10:39 +0100, Lorenzo Pieralisi wrote:
> On Mon, 19 Jul 2021 15:34:52 +0800, Chuanjia Liu wrote:
> > There are two independent PCIe controllers in MT2712 and MT7622
> > platform.
> > Each of them should contain an independent MSI domain.
> > 
> > In old dts 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.
> > 
> > [...]
> 
> Applied patches 1-2 to pci/mediatek (we don't merge dts changes),
> thanks!
> 
> [1/2] dt-bindings: PCI: mediatek: Update the Device tree bindings
>       https://git.kernel.org/lpieralisi/pci/c/9c23251640
> [2/2] PCI: mediatek: Add new method to get shared pcie-cfg base
> address and parse node
>       https://git.kernel.org/lpieralisi/pci/c/302e503e08
> 
> Thanks,
> Lorenzo

Hi, matthias
Could you help apply dts changes(patch 3-4)?
Lorenzo helped to apply the driver part, and dts part
hopes to apply together.

Best Regards

> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-07-19  7:34 ` [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node Chuanjia Liu
  2021-07-20  2:59   ` Chuanjia Liu
  2021-08-02  7:07   ` Chuanjia Liu
@ 2021-08-10 19:42   ` Bjorn Helgaas
  2021-08-13 15:22     ` Lorenzo Pieralisi
  2 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2021-08-10 19:42 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: robh+dt, bhelgaas, matthias.bgg, lorenzo.pieralisi, ryder.lee,
	jianjun.wang, yong.wu, Frank Wunderlich, linux-pci,
	linux-mediatek, devicetree, linux-arm-kernel, linux-kernel

On Mon, Jul 19, 2021 at 03:34:54PM +0800, Chuanjia Liu wrote:
> For the new dts format, add a new method to get
> shared pcie-cfg base address and parse node.

This commit log doesn't seem to really cover what's going on here.  It
looks like:

  - You added a check for "mediatek,generic-pciecfg" (I guess this is
    the "shared pcie-cfg base address" part).  Probably could have
    been its own patch.

  - You added checks for "interrupt-names" and "pcie_irq".  Not
    explained in commit log; probably could have been its own patch,
    too.

  - You now look for "linux,pci-domain" (via of_get_pci_domain_nr()).
    If present, you parse only one port instead of looking for all the
    children of the node.

    That's sort of weird behavior -- why should the presence of
    "linux,pci-domain" determine whether the node can have children?
    Is that really what you intend?

    Should be explained in the commit log and could have been its own
    patch, too.

> Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 25bee693834f..928e0983a900 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"
> @@ -207,6 +209,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
> @@ -215,6 +218,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;
> @@ -650,7 +654,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;
>  
> @@ -682,6 +690,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 */
> @@ -985,6 +997,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)
>  			return PTR_ERR(pcie->base);
>  	}
>  
> +	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)
> @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
>  	struct device *dev = pcie->dev;
>  	struct device_node *node = dev->of_node, *child;
>  	struct mtk_pcie_port *port, *tmp;
> -	int err;
> +	int err, slot;
> +
> +	slot = of_get_pci_domain_nr(dev->of_node);
> +	if (slot < 0) {
> +		for_each_available_child_of_node(node, child) {
> +			err = of_pci_get_devfn(child);
> +			if (err < 0) {
> +				dev_err(dev, "failed to get devfn: %d\n", err);
> +				goto error_put_node;
> +			}
>  
> -	for_each_available_child_of_node(node, child) {
> -		int slot;
> +			slot = PCI_SLOT(err);
>  
> -		err = of_pci_get_devfn(child);
> -		if (err < 0) {
> -			dev_err(dev, "failed to parse devfn: %d\n", err);
> -			goto error_put_node;
> +			err = mtk_pcie_parse_port(pcie, child, slot);
> +			if (err)
> +				goto error_put_node;
>  		}
> -
> -		slot = PCI_SLOT(err);
> -
> -		err = mtk_pcie_parse_port(pcie, child, slot);
> +	} else {
> +		err = mtk_pcie_parse_port(pcie, node, slot);
>  		if (err)
> -			goto error_put_node;
> +			return err;
>  	}
>  
>  	err = mtk_pcie_subsys_powerup(pcie);
> -- 
> 2.18.0
> 

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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-08-10 19:42   ` Bjorn Helgaas
@ 2021-08-13 15:22     ` Lorenzo Pieralisi
  2021-08-17 11:18       ` Chuanjia Liu (柳传嘉)
  0 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Pieralisi @ 2021-08-13 15:22 UTC (permalink / raw)
  To: Bjorn Helgaas, Chuanjia Liu
  Cc: robh+dt, bhelgaas, matthias.bgg, ryder.lee, jianjun.wang,
	yong.wu, Frank Wunderlich, linux-pci, linux-mediatek, devicetree,
	linux-arm-kernel, linux-kernel

On Tue, Aug 10, 2021 at 02:42:50PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 19, 2021 at 03:34:54PM +0800, Chuanjia Liu wrote:
> > For the new dts format, add a new method to get
> > shared pcie-cfg base address and parse node.
> 
> This commit log doesn't seem to really cover what's going on here.  It
> looks like:
> 
>   - You added a check for "mediatek,generic-pciecfg" (I guess this is
>     the "shared pcie-cfg base address" part).  Probably could have
>     been its own patch.
> 
>   - You added checks for "interrupt-names" and "pcie_irq".  Not
>     explained in commit log; probably could have been its own patch,
>     too.
> 
>   - You now look for "linux,pci-domain" (via of_get_pci_domain_nr()).
>     If present, you parse only one port instead of looking for all the
>     children of the node.
> 
>     That's sort of weird behavior -- why should the presence of
>     "linux,pci-domain" determine whether the node can have children?
>     Is that really what you intend?
> 
>     Should be explained in the commit log and could have been its own
>     patch, too.

I agree with Bjorn, this patch should be split (and commit logs
rewritten). I will drop it from my tree, waiting for a v12.

Lorenzo

> > Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
> >  1 file changed, 39 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 25bee693834f..928e0983a900 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"
> > @@ -207,6 +209,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
> > @@ -215,6 +218,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;
> > @@ -650,7 +654,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;
> >  
> > @@ -682,6 +690,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 */
> > @@ -985,6 +997,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)
> >  			return PTR_ERR(pcie->base);
> >  	}
> >  
> > +	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)
> > @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie *pcie)
> >  	struct device *dev = pcie->dev;
> >  	struct device_node *node = dev->of_node, *child;
> >  	struct mtk_pcie_port *port, *tmp;
> > -	int err;
> > +	int err, slot;
> > +
> > +	slot = of_get_pci_domain_nr(dev->of_node);
> > +	if (slot < 0) {
> > +		for_each_available_child_of_node(node, child) {
> > +			err = of_pci_get_devfn(child);
> > +			if (err < 0) {
> > +				dev_err(dev, "failed to get devfn: %d\n", err);
> > +				goto error_put_node;
> > +			}
> >  
> > -	for_each_available_child_of_node(node, child) {
> > -		int slot;
> > +			slot = PCI_SLOT(err);
> >  
> > -		err = of_pci_get_devfn(child);
> > -		if (err < 0) {
> > -			dev_err(dev, "failed to parse devfn: %d\n", err);
> > -			goto error_put_node;
> > +			err = mtk_pcie_parse_port(pcie, child, slot);
> > +			if (err)
> > +				goto error_put_node;
> >  		}
> > -
> > -		slot = PCI_SLOT(err);
> > -
> > -		err = mtk_pcie_parse_port(pcie, child, slot);
> > +	} else {
> > +		err = mtk_pcie_parse_port(pcie, node, slot);
> >  		if (err)
> > -			goto error_put_node;
> > +			return err;
> >  	}
> >  
> >  	err = mtk_pcie_subsys_powerup(pcie);
> > -- 
> > 2.18.0
> > 

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

* Re: [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node
  2021-08-13 15:22     ` Lorenzo Pieralisi
@ 2021-08-17 11:18       ` Chuanjia Liu (柳传嘉)
  0 siblings, 0 replies; 18+ messages in thread
From: Chuanjia Liu (柳传嘉) @ 2021-08-17 11:18 UTC (permalink / raw)
  To: lorenzo.pieralisi, helgaas
  Cc: linux-mediatek, robh+dt, linux-kernel, devicetree, frank-w,
	Yong Wu (吴勇),
	Jianjun Wang (王建军),
	linux-arm-kernel, linux-pci, matthias.bgg, bhelgaas,
	Ryder Lee (李庚諺)

On Fri, 2021-08-13 at 16:22 +0100, Lorenzo Pieralisi wrote:
> On Tue, Aug 10, 2021 at 02:42:50PM -0500, Bjorn Helgaas wrote:
> > On Mon, Jul 19, 2021 at 03:34:54PM +0800, Chuanjia Liu wrote:
> > > For the new dts format, add a new method to get
> > > shared pcie-cfg base address and parse node.
> > 
> > This commit log doesn't seem to really cover what's going on
> > here.  It
> > looks like:
> > 
> >   - You added a check for "mediatek,generic-pciecfg" (I guess this
> > is
> >     the "shared pcie-cfg base address" part).  Probably could have
> >     been its own patch.
> > 
> >   - You added checks for "interrupt-names" and "pcie_irq".  Not
> >     explained in commit log; probably could have been its own
> > patch,
> >     too.
> > 
> >   - You now look for "linux,pci-domain" (via
> > of_get_pci_domain_nr()).
> >     If present, you parse only one port instead of looking for all
> > the
> >     children of the node.
> > 
> >     That's sort of weird behavior -- why should the presence of
> >     "linux,pci-domain" determine whether the node can have
> > children?
> >     Is that really what you intend?
> > 
> >     Should be explained in the commit log and could have been its
> > own
> >     patch, too.
hi, Bjorn,
Yes, this is my intention,"Linux,pci-domain" has two purposes
1)Distinguish the old and new dts formats, then parse them
2)Determine the current pcie slot num in new dts format, because the
offset of some regs needs to be based on slot num

I will split this patch into three patches, and then explain them in
commit log, thanks for your comment.
> 
> I agree with Bjorn, this patch should be split (and commit logs
> rewritten). I will drop it from my tree, waiting for a v12.
> 
> Lorenzo

hi, Lorenzo
Thanks for your confirmation, I will send the V12 version as soon as
possible.

Chuanjia
> 
> > > Signed-off-by: Chuanjia Liu <chuanjia.liu@mediatek.com>
> > > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-
> > > ------
> > >  1 file changed, 39 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c
> > > b/drivers/pci/controller/pcie-mediatek.c
> > > index 25bee693834f..928e0983a900 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"
> > > @@ -207,6 +209,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
> > > @@ -215,6 +218,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;
> > > @@ -650,7 +654,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;
> > >  
> > > @@ -682,6 +690,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 */
> > > @@ -985,6 +997,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)
> > >  			return PTR_ERR(pcie->base);
> > >  	}
> > >  
> > > +	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)
> > > @@ -1027,22 +1048,27 @@ static int mtk_pcie_setup(struct mtk_pcie
> > > *pcie)
> > >  	struct device *dev = pcie->dev;
> > >  	struct device_node *node = dev->of_node, *child;
> > >  	struct mtk_pcie_port *port, *tmp;
> > > -	int err;
> > > +	int err, slot;
> > > +
> > > +	slot = of_get_pci_domain_nr(dev->of_node);
> > > +	if (slot < 0) {
> > > +		for_each_available_child_of_node(node, child) {
> > > +			err = of_pci_get_devfn(child);
> > > +			if (err < 0) {
> > > +				dev_err(dev, "failed to get devfn:
> > > %d\n", err);
> > > +				goto error_put_node;
> > > +			}
> > >  
> > > -	for_each_available_child_of_node(node, child) {
> > > -		int slot;
> > > +			slot = PCI_SLOT(err);
> > >  
> > > -		err = of_pci_get_devfn(child);
> > > -		if (err < 0) {
> > > -			dev_err(dev, "failed to parse devfn: %d\n",
> > > err);
> > > -			goto error_put_node;
> > > +			err = mtk_pcie_parse_port(pcie, child, slot);
> > > +			if (err)
> > > +				goto error_put_node;
> > >  		}
> > > -
> > > -		slot = PCI_SLOT(err);
> > > -
> > > -		err = mtk_pcie_parse_port(pcie, child, slot);
> > > +	} else {
> > > +		err = mtk_pcie_parse_port(pcie, node, slot);
> > >  		if (err)
> > > -			goto error_put_node;
> > > +			return err;
> > >  	}
> > >  
> > >  	err = mtk_pcie_subsys_powerup(pcie);
> > > -- 
> > > 2.18.0
> > > 

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

end of thread, other threads:[~2021-08-17 11:19 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19  7:34 [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Chuanjia Liu
2021-07-19  7:34 ` [PATCH v11 1/4] dt-bindings: PCI: mediatek: Update the Device tree bindings Chuanjia Liu
2021-07-19 22:47   ` Rob Herring
2021-07-20  2:07     ` Chuanjia Liu
2021-07-20 16:26       ` Rob Herring
2021-07-23  7:17         ` Chuanjia Liu
2021-07-19  7:34 ` [PATCH v11 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node Chuanjia Liu
2021-07-20  2:59   ` Chuanjia Liu
2021-08-03 22:18     ` Rob Herring
2021-08-06  7:37       ` Chuanjia Liu (柳传嘉)
2021-08-02  7:07   ` Chuanjia Liu
2021-08-10 19:42   ` Bjorn Helgaas
2021-08-13 15:22     ` Lorenzo Pieralisi
2021-08-17 11:18       ` Chuanjia Liu (柳传嘉)
2021-07-19  7:34 ` [PATCH v11 3/4] arm64: dts: mediatek: Split PCIe node for MT2712 and MT7622 Chuanjia Liu
2021-07-19  7:34 ` [PATCH v11 4/4] ARM: dts: mediatek: Update MT7629 PCIe node for new format Chuanjia Liu
2021-08-06  9:39 ` [PATCH v11 0/4] PCI: mediatek: Spilt PCIe node to comply with hardware design Lorenzo Pieralisi
2021-08-08  4:50   ` 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).