[PATCH V2 0/4] Add Tegra186 PCIe support

[PATCH V2 0/4] Add Tegra186 PCIe support

[Manikanta Maddireddy]

Tegra186 has three PCIe controllers which can be operated
in 401, 211 or 111 lane configurations. Tegra TX2 platform
has x4 and M.2 Key E PCIe slots, these patches enables
x4 slot. BPMP programs UPHY lane0 ownership to USB,
so M.2 Key E PCIe will not work.

Testing: x4 slot is verified with PCIe based USB3.1 card.
PCIe link up, usb flash drive mounting and file copy are
verified. These patches are also verified by
Mikko Perttunen <mperttunen@nvidia.com> with an Intel 82574L
ethernet card.

Patch V2 adds soc->program_uphy check for phy_exit call.

Pasting PCIe link up logs below.

[    1.452512] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
[    1.452723] tegra-pcie 10003000.pcie: Failed to get supply 'dvdd-pex': -517
[    1.561824] ehci-pci: EHCI PCI platform driver
[    1.591587] ohci-pci: OHCI PCI platform driver
[    3.072464] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
[    3.078989] tegra-pcie 10003000.pcie: probing port 0, using 4 lanes
[    3.087272] tegra-pcie 10003000.pcie: Slot present pin change, signature: 00000018
[    3.128818] tegra-pcie 10003000.pcie: PCI host bridge to bus 0000:00
[    3.135174] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    3.141352] pci_bus 0000:00: root bus resource [mem 0x50100000-0x57ffffff]
[    3.148221] pci_bus 0000:00: root bus resource [mem 0x58000000-0x7fffffff pref]
[    3.155542] pci_bus 0000:00: root bus resource [bus 00-ff]
[    3.161173] pci 0000:00:01.0: [10de:10e5] type 01 class 0x060400
[    3.161188] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
[    3.170838] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
[    3.180532] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x52 may corrupt adjacent RW1C bits
[    3.190305] pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot D3cold
[    3.190312] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4c may corrupt adjacent RW1C bits
[    3.200158] pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    3.208158] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
[    3.217918] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
[    3.227649] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
[    3.237382] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x6 may corrupt adjacent RW1C bits
[    3.247135] pci 0000:01:00.0: [1b21:1242] type 00 class 0x0c0330
[    3.247157] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
[    3.256807] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
[    3.266477] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00007fff 64bit]
[    3.266544] pci 0000:01:00.0: enabling Extended Tags
[    3.271622] pci 0000:01:00.0: PME# supported from D3hot D3cold
[    3.271775] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[    3.271798] pci 0000:00:01.0: BAR 14: assigned [mem 0x50100000-0x501fffff]
[    3.278671] pci 0000:01:00.0: BAR 0: assigned [mem 0x50100000-0x50107fff 64bit]
[    3.285993] pci 0000:00:01.0: PCI bridge to [bus 01]
[    3.290959] pci 0000:00:01.0:   bridge window [mem 0x50100000-0x501fffff]
[    3.297763] pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge
[    3.304784] pcieport 0000:00:01.0: enabling device (0000 -> 0002)
[    3.311014] pcieport 0000:00:01.0: Signaling PME with IRQ 57
[    3.316768] pci 0000:01:00.0: enabling device (0000 -> 0002)


Manikanta Maddireddy (4):
  dt-bindings: pci: tegra: Document Tegra186 PCIe DT
  PCI: tegra: Add Tegra186 PCIe support
  arm64: tegra: Add PCIe node for Tegra186
  arm64: tegra: Enable PCIe on Jetson TX2

 .../bindings/pci/nvidia,tegra20-pcie.txt           | 134 ++++++++++++++++++++-
 arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi     |  24 ++++
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  82 +++++++++++++
 drivers/pci/host/pci-tegra.c                       | 132 ++++++++++++++++----
 4 files changed, 344 insertions(+), 28 deletions(-)

-- 
2.1.4

[PATCH V2 1/4] dt-bindings: pci: tegra: Document Tegra186 PCIe DT

[Manikanta Maddireddy]

Tegra186 PCIe controller DT properties has couple of differences
wrt Tegra210 PCIe, rest of the DT properties are same.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
---
V2: No change in this patch
 .../bindings/pci/nvidia,tegra20-pcie.txt           | 134 ++++++++++++++++++++-
 1 file changed, 130 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index 982a74ea6df9..753b67327373 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -1,10 +1,15 @@
 NVIDIA Tegra PCIe controller
 
 Required properties:
-- compatible: For Tegra20, must contain "nvidia,tegra20-pcie".  For Tegra30,
-  "nvidia,tegra30-pcie".  For Tegra124, must contain "nvidia,tegra124-pcie".
-  Otherwise, must contain "nvidia,<chip>-pcie", plus one of the above, where
-  <chip> is tegra132 or tegra210.
+- compatible: Must be:
+  - "nvidia,tegra20-pcie": for Tegra20
+  - "nvidia,tegra30-pcie": for Tegra30
+  - "nvidia,tegra124-pcie": for Tegra124 and Tegra132
+  - "nvidia,tegra210-pcie": for Tegra210
+  - "nvidia,tegra186-pcie": for Tegra186
+- power-domains: To ungate power partition by BPMP powergate driver. Must
+contain BPMP phandle and PCIe power partition ID. This is required only
+for Tegra186.
 - device_type: Must be "pci"
 - reg: A list of physical base address and length for each set of controller
   registers. Must contain an entry for each entry in the reg-names property.
@@ -124,6 +129,16 @@ Power supplies for Tegra210:
   - vddio-pex-ctl-supply: Power supply for PCIe control I/O partition. Must
     supply 1.8 V.
 
+Power supplies for Tegra186:
+- Required:
+  - dvdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V.
+  - hvdd-pex-pll-supply: High-voltage supply for PLLE (shared with USB3). Must
+    supply 1.8 V.
+  - hvdd-pex-supply: High-voltage supply for PCIe I/O and PCIe output clocks.
+    Must supply 1.8 V.
+  - vddio-pexctl-aud-supply: Power supply for PCIe side band signals. Must
+    supply 1.8 V.
+
 Root ports are defined as subnodes of the PCIe controller node.
 
 Required properties:
@@ -546,3 +561,114 @@ Board DTS:
 			status = "okay";
 		};
 	};
+
+Tegra186:
+---------
+
+SoC DTSI:
+
+	pcie@10003000 {
+		compatible = "nvidia,tegra186-pcie";
+		power-domains = <&bpmp TEGRA186_POWER_DOMAIN_PCX>;
+		device_type = "pci";
+		reg = <0x0 0x10003000 0x0 0x00000800   /* PADS registers */
+		       0x0 0x10003800 0x0 0x00000800   /* AFI registers */
+		       0x0 0x40000000 0x0 0x10000000>; /* configuration space */
+		reg-names = "pads", "afi", "cs";
+
+		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */
+			     <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */
+		interrupt-names = "intr", "msi";
+
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+
+		bus-range = <0x00 0xff>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		ranges = <0x82000000 0 0x10000000 0x0 0x10000000 0 0x00001000   /* port 0 configuration space */
+			  0x82000000 0 0x10001000 0x0 0x10001000 0 0x00001000   /* port 1 configuration space */
+			  0x82000000 0 0x10004000 0x0 0x10004000 0 0x00001000   /* port 2 configuration space */
+			  0x81000000 0 0x0        0x0 0x50000000 0 0x00010000   /* downstream I/O (64 KiB) */
+			  0x82000000 0 0x50100000 0x0 0x50100000 0 0x07F00000   /* non-prefetchable memory (127 MiB) */
+			  0xc2000000 0 0x58000000 0x0 0x58000000 0 0x28000000>; /* prefetchable memory (640 MiB) */
+
+		clocks = <&bpmp TEGRA186_CLK_AFI>,
+			 <&bpmp TEGRA186_CLK_PCIE>,
+			 <&bpmp TEGRA186_CLK_PLLE>;
+		clock-names = "afi", "pex", "pll_e";
+
+		resets = <&bpmp TEGRA186_RESET_AFI>,
+			 <&bpmp TEGRA186_RESET_PCIE>,
+			 <&bpmp TEGRA186_RESET_PCIEXCLK>;
+		reset-names = "afi", "pex", "pcie_x";
+
+		status = "disabled";
+
+		pci@1,0 {
+			device_type = "pci";
+			assigned-addresses = <0x82000800 0 0x10000000 0 0x1000>;
+			reg = <0x000800 0 0 0 0>;
+			status = "disabled";
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			nvidia,num-lanes = <2>;
+		};
+
+		pci@2,0 {
+			device_type = "pci";
+			assigned-addresses = <0x82001000 0 0x10001000 0 0x1000>;
+			reg = <0x001000 0 0 0 0>;
+			status = "disabled";
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			nvidia,num-lanes = <1>;
+		};
+
+		pci@3,0 {
+			device_type = "pci";
+			assigned-addresses = <0x82001800 0 0x10004000 0 0x1000>;
+			reg = <0x001800 0 0 0 0>;
+			status = "disabled";
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			nvidia,num-lanes = <1>;
+		};
+	};
+
+Board DTS:
+
+	pcie@10003000 {
+		status = "okay";
+
+		dvdd-pex-supply = <&vdd_pex>;
+		hvdd-pex-pll-supply = <&vdd_1v8>;
+		hvdd-pex-supply = <&vdd_1v8>;
+		vddio-pexctl-aud-supply = <&vdd_1v8>;
+
+		pci@1,0 {
+			nvidia,num-lanes = <4>;
+			status = "okay";
+		};
+
+		pci@2,0 {
+			nvidia,num-lanes = <0>;
+			status = "disabled";
+		};
+
+		pci@3,0 {
+			nvidia,num-lanes = <1>;
+			status = "disabled";
+		};
+	};
-- 
2.1.4

[PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Manikanta Maddireddy]

UPHY programming is performed by BPMP, PHY enable calls are
not required for Tegra186 PCIe. Power partition ungate is
done by BPMP powergate driver.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
---
V2: Add soc->program_uphy check for phy_exit call
 drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 108 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 9c40da54f88a..96e8038c3019 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -159,10 +159,13 @@
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_SINGLE	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_420	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_X2_X1	(0x0 << 20)
+#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_401	(0x0 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_DUAL	(0x1 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_222	(0x1 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_X4_X1	(0x1 << 20)
+#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_211	(0x1 << 20)
 #define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_411	(0x2 << 20)
+#define  AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_111	(0x2 << 20)
 
 #define AFI_FUSE			0x104
 #define  AFI_FUSE_PCIE_T0_GEN2_DIS	(1 << 2)
@@ -252,6 +255,7 @@ struct tegra_pcie_soc {
 	bool has_cml_clk;
 	bool has_gen2;
 	bool force_pca_enable;
+	bool program_uphy;
 };
 
 static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
@@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 		afi_writel(pcie, value, AFI_FUSE);
 	}
 
-	err = tegra_pcie_phy_power_on(pcie);
-	if (err < 0) {
-		dev_err(dev, "failed to power on PHY(s): %d\n", err);
-		return err;
+	if (soc->program_uphy) {
+		err = tegra_pcie_phy_power_on(pcie);
+		if (err < 0) {
+			dev_err(dev, "failed to power on PHY(s): %d\n", err);
+			return err;
+		}
 	}
 
 	/* take the PCIe interface module out of reset */
@@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
 static void tegra_pcie_power_off(struct tegra_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
+	const struct tegra_pcie_soc *soc = pcie->soc;
 	int err;
 
 	/* TODO: disable and unprepare clocks? */
 
-	err = tegra_pcie_phy_power_off(pcie);
-	if (err < 0)
-		dev_err(dev, "failed to power off PHY(s): %d\n", err);
+	if (soc->program_uphy) {
+		err = tegra_pcie_phy_power_off(pcie);
+		if (err < 0)
+			dev_err(dev, "failed to power off PHY(s): %d\n", err);
+	}
 
 	reset_control_assert(pcie->pcie_xrst);
 	reset_control_assert(pcie->afi_rst);
 	reset_control_assert(pcie->pex_rst);
 
-	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
+	if (!dev->pm_domain)
+		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
 
 	err = regulator_bulk_disable(pcie->num_supplies, pcie->supplies);
 	if (err < 0)
@@ -1077,19 +1087,29 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
 	reset_control_assert(pcie->afi_rst);
 	reset_control_assert(pcie->pex_rst);
 
-	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
+	if (!dev->pm_domain)
+		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
 
 	/* enable regulators */
 	err = regulator_bulk_enable(pcie->num_supplies, pcie->supplies);
 	if (err < 0)
 		dev_err(dev, "failed to enable regulators: %d\n", err);
 
-	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
-						pcie->pex_clk,
-						pcie->pex_rst);
-	if (err) {
-		dev_err(dev, "powerup sequence failed: %d\n", err);
-		return err;
+	if (dev->pm_domain) {
+		err = clk_prepare_enable(pcie->pex_clk);
+		if (err) {
+			dev_err(dev, "failed to enable PEX clock: %d\n", err);
+			return err;
+		}
+		reset_control_deassert(pcie->pex_rst);
+	} else {
+		err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_PCIE,
+							pcie->pex_clk,
+							pcie->pex_rst);
+		if (err) {
+			dev_err(dev, "powerup sequence failed: %d\n", err);
+			return err;
+		}
 	}
 
 	reset_control_deassert(pcie->afi_rst);
@@ -1262,6 +1282,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct resource *pads, *afi, *res;
+	const struct tegra_pcie_soc *soc = pcie->soc;
 	int err;
 
 	err = tegra_pcie_clocks_get(pcie);
@@ -1276,10 +1297,12 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
 		return err;
 	}
 
-	err = tegra_pcie_phys_get(pcie);
-	if (err < 0) {
-		dev_err(dev, "failed to get PHYs: %d\n", err);
-		return err;
+	if (soc->program_uphy) {
+		err = tegra_pcie_phys_get(pcie);
+		if (err < 0) {
+			dev_err(dev, "failed to get PHYs: %d\n", err);
+			return err;
+		}
 	}
 
 	err = tegra_pcie_power_on(pcie);
@@ -1341,6 +1364,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
 static int tegra_pcie_put_resources(struct tegra_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
+	const struct tegra_pcie_soc *soc = pcie->soc;
 	int err;
 
 	if (pcie->irq > 0)
@@ -1348,9 +1372,11 @@ static int tegra_pcie_put_resources(struct tegra_pcie *pcie)
 
 	tegra_pcie_power_off(pcie);
 
-	err = phy_exit(pcie->phy);
-	if (err < 0)
-		dev_err(dev, "failed to teardown PHY: %d\n", err);
+	if (soc->program_uphy) {
+		err = phy_exit(pcie->phy);
+		if (err < 0)
+			dev_err(dev, "failed to teardown PHY: %d\n", err);
+	}
 
 	return 0;
 }
@@ -1616,7 +1642,31 @@ static int tegra_pcie_get_xbar_config(struct tegra_pcie *pcie, u32 lanes,
 	struct device *dev = pcie->dev;
 	struct device_node *np = dev->of_node;
 
-	if (of_device_is_compatible(np, "nvidia,tegra124-pcie") ||
+	if (of_device_is_compatible(np, "nvidia,tegra186-pcie")) {
+		switch (lanes) {
+		case 0x010004:
+			dev_info(dev, "4x1, 1x1 configuration\n");
+			*xbar = AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_401;
+			return 0;
+
+		case 0x010102:
+			dev_info(dev, "2x1, 1X1, 1x1 configuration\n");
+			*xbar = AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_211;
+			return 0;
+
+		case 0x010101:
+			dev_info(dev, "1x1, 1x1, 1x1 configuration\n");
+			*xbar = AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_111;
+			return 0;
+
+		default:
+			dev_info(dev, "wrong configuration updated in DT, "
+				 "switching to default 2x1, 1x1, 1x1 "
+				 "configuration\n");
+			*xbar = AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_211;
+			return 0;
+		}
+	} else if (of_device_is_compatible(np, "nvidia,tegra124-pcie") ||
 	    of_device_is_compatible(np, "nvidia,tegra210-pcie")) {
 		switch (lanes) {
 		case 0x0000104:
@@ -1737,7 +1787,20 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask)
 	struct device_node *np = dev->of_node;
 	unsigned int i = 0;
 
-	if (of_device_is_compatible(np, "nvidia,tegra210-pcie")) {
+	if (of_device_is_compatible(np, "nvidia,tegra186-pcie")) {
+		pcie->num_supplies = 4;
+
+		pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies,
+					      sizeof(*pcie->supplies),
+					      GFP_KERNEL);
+		if (!pcie->supplies)
+			return -ENOMEM;
+
+		pcie->supplies[i++].supply = "dvdd-pex";
+		pcie->supplies[i++].supply = "hvdd-pex-pll";
+		pcie->supplies[i++].supply = "hvdd-pex";
+		pcie->supplies[i++].supply = "vddio-pexctl-aud";
+	} else if (of_device_is_compatible(np, "nvidia,tegra210-pcie")) {
 		pcie->num_supplies = 6;
 
 		pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies,
@@ -2076,6 +2139,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
 	.has_cml_clk = false,
 	.has_gen2 = false,
 	.force_pca_enable = false,
+	.program_uphy = true,
 };
 
 static const struct tegra_pcie_soc tegra30_pcie = {
@@ -2091,6 +2155,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
 	.has_cml_clk = true,
 	.has_gen2 = false,
 	.force_pca_enable = false,
+	.program_uphy = true,
 };
 
 static const struct tegra_pcie_soc tegra124_pcie = {
@@ -2105,6 +2170,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
 	.has_cml_clk = true,
 	.has_gen2 = true,
 	.force_pca_enable = false,
+	.program_uphy = true,
 };
 
 static const struct tegra_pcie_soc tegra210_pcie = {
@@ -2119,9 +2185,27 @@ static const struct tegra_pcie_soc tegra210_pcie = {
 	.has_cml_clk = true,
 	.has_gen2 = true,
 	.force_pca_enable = true,
+	.program_uphy = true,
+};
+
+static const struct tegra_pcie_soc tegra186_pcie = {
+	.num_ports = 3,
+	.msi_base_shift = 8,
+	.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
+	.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+	.pads_refclk_cfg0 = 0x80b880b8,
+	.pads_refclk_cfg1 = 0x000480b8,
+	.has_pex_clkreq_en = true,
+	.has_pex_bias_ctrl = true,
+	.has_intr_prsnt_sense = true,
+	.has_cml_clk = false,
+	.has_gen2 = true,
+	.force_pca_enable = false,
+	.program_uphy = false,
 };
 
 static const struct of_device_id tegra_pcie_of_match[] = {
+	{ .compatible = "nvidia,tegra186-pcie", .data = &tegra186_pcie },
 	{ .compatible = "nvidia,tegra210-pcie", .data = &tegra210_pcie },
 	{ .compatible = "nvidia,tegra124-pcie", .data = &tegra124_pcie },
 	{ .compatible = "nvidia,tegra30-pcie", .data = &tegra30_pcie },
-- 
2.1.4

[PATCH V2 3/4] arm64: tegra: Add PCIe node for Tegra186

[Manikanta Maddireddy]

Tegra186 has three PCIe controllers, which can be operated
in 401, 211 or 111 lane combinations. Add DT support for
PCIe controllers.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
---
V2: No change in this patch
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 82 ++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 0b0552c9f7dd..9edf2a839e5d 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -443,6 +443,7 @@
 		shmem = <&cpu_bpmp_tx &cpu_bpmp_rx>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
+		#power-domain-cells = <1>;
 
 		bpmp_i2c: i2c {
 			compatible = "nvidia,tegra186-bpmp-i2c";
@@ -465,4 +466,85 @@
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
 		interrupt-parent = <&gic>;
 	};
+
+	pcie@10003000 {
+		compatible = "nvidia,tegra186-pcie";
+		power-domains = <&bpmp TEGRA186_POWER_DOMAIN_PCX>;
+		device_type = "pci";
+		reg = <0x0 0x10003000 0x0 0x00000800   /* PADS registers */
+		       0x0 0x10003800 0x0 0x00000800   /* AFI registers */
+		       0x0 0x40000000 0x0 0x10000000>; /* configuration space */
+		reg-names = "pads", "afi", "cs";
+
+		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */
+			     <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */
+		interrupt-names = "intr", "msi";
+
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+
+		bus-range = <0x00 0xff>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		ranges = <0x82000000 0 0x10000000 0x0 0x10000000 0 0x00001000   /* port 0 configuration space */
+			  0x82000000 0 0x10001000 0x0 0x10001000 0 0x00001000   /* port 1 configuration space */
+			  0x82000000 0 0x10004000 0x0 0x10004000 0 0x00001000   /* port 2 configuration space */
+			  0x81000000 0 0x0        0x0 0x50000000 0 0x00010000   /* downstream I/O (64 KiB) */
+			  0x82000000 0 0x50100000 0x0 0x50100000 0 0x07F00000   /* non-prefetchable memory (127 MiB) */
+			  0xc2000000 0 0x58000000 0x0 0x58000000 0 0x28000000>; /* prefetchable memory (640 MiB) */
+
+		clocks = <&bpmp TEGRA186_CLK_AFI>,
+			 <&bpmp TEGRA186_CLK_PCIE>,
+			 <&bpmp TEGRA186_CLK_PLLE>;
+		clock-names = "afi", "pex", "pll_e";
+
+		resets = <&bpmp TEGRA186_RESET_AFI>,
+			 <&bpmp TEGRA186_RESET_PCIE>,
+			 <&bpmp TEGRA186_RESET_PCIEXCLK>;
+		reset-names = "afi", "pex", "pcie_x";
+
+		status = "disabled";
+
+		pci@1,0 {
+			device_type = "pci";
+			assigned-addresses = <0x82000800 0 0x10000000 0 0x1000>;
+			reg = <0x000800 0 0 0 0>;
+			status = "disabled";
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			nvidia,num-lanes = <2>;
+		};
+
+		pci@2,0 {
+			device_type = "pci";
+			assigned-addresses = <0x82001000 0 0x10001000 0 0x1000>;
+			reg = <0x001000 0 0 0 0>;
+			status = "disabled";
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			nvidia,num-lanes = <1>;
+		};
+
+		pci@3,0 {
+			device_type = "pci";
+			assigned-addresses = <0x82001800 0 0x10004000 0 0x1000>;
+			reg = <0x001800 0 0 0 0>;
+			status = "disabled";
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			nvidia,num-lanes = <1>;
+		};
+
+	};
 };
-- 
2.1.4

[PATCH V2 4/4] arm64: tegra: Enable PCIe on Jetson TX2

[Manikanta Maddireddy]

Enable x4 PCIe slot on Jetson TX2.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
---
V2: No change in this patch
 arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
index cf84d7046ad5..a4d96b2a23b4 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
@@ -378,4 +378,28 @@
 			vin-supply = <&vdd_1v8>;
 		};
 	};
+
+	pcie@10003000 {
+		status = "okay";
+
+		dvdd-pex-supply = <&vdd_pex>;
+		hvdd-pex-pll-supply = <&vdd_1v8>;
+		hvdd-pex-supply = <&vdd_1v8>;
+		vddio-pexctl-aud-supply = <&vdd_1v8>;
+
+		pci@1,0 {
+			nvidia,num-lanes = <4>;
+			status = "okay";
+		};
+
+		pci@2,0 {
+			nvidia,num-lanes = <0>;
+			status = "disabled";
+		};
+
+		pci@3,0 {
+			nvidia,num-lanes = <1>;
+			status = "disabled";
+		};
+	};
 };
-- 
2.1.4

Re: [PATCH V2 1/4] dt-bindings: pci: tegra: Document Tegra186 PCIe DT

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 6318 bytes --]

On Wed, Sep 27, 2017 at 05:28:34PM +0530, Manikanta Maddireddy wrote:
> Tegra186 PCIe controller DT properties has couple of differences
> wrt Tegra210 PCIe, rest of the DT properties are same.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> V2: No change in this patch
>  .../bindings/pci/nvidia,tegra20-pcie.txt           | 134 ++++++++++++++++++++-
>  1 file changed, 130 insertions(+), 4 deletions(-)

Hi Rob,

Manikanta forgot to add you on Cc on this one. Can you take a look or
should Manikanta resend the series to include you and the device tree
mailing list?

FWIW, this looks good to me, so:

Acked-by: Thierry Reding <treding@nvidia.com>

Bjorn,

I take it that you'd pull this into the PCI tree along with the host
controller driver changes? I can take patches 3 and 4 through the Tegra
tree.

Thierry

> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> index 982a74ea6df9..753b67327373 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> @@ -1,10 +1,15 @@
>  NVIDIA Tegra PCIe controller
>  
>  Required properties:
> -- compatible: For Tegra20, must contain "nvidia,tegra20-pcie".  For Tegra30,
> -  "nvidia,tegra30-pcie".  For Tegra124, must contain "nvidia,tegra124-pcie".
> -  Otherwise, must contain "nvidia,<chip>-pcie", plus one of the above, where
> -  <chip> is tegra132 or tegra210.
> +- compatible: Must be:
> +  - "nvidia,tegra20-pcie": for Tegra20
> +  - "nvidia,tegra30-pcie": for Tegra30
> +  - "nvidia,tegra124-pcie": for Tegra124 and Tegra132
> +  - "nvidia,tegra210-pcie": for Tegra210
> +  - "nvidia,tegra186-pcie": for Tegra186
> +- power-domains: To ungate power partition by BPMP powergate driver. Must
> +contain BPMP phandle and PCIe power partition ID. This is required only
> +for Tegra186.
>  - device_type: Must be "pci"
>  - reg: A list of physical base address and length for each set of controller
>    registers. Must contain an entry for each entry in the reg-names property.
> @@ -124,6 +129,16 @@ Power supplies for Tegra210:
>    - vddio-pex-ctl-supply: Power supply for PCIe control I/O partition. Must
>      supply 1.8 V.
>  
> +Power supplies for Tegra186:
> +- Required:
> +  - dvdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V.
> +  - hvdd-pex-pll-supply: High-voltage supply for PLLE (shared with USB3). Must
> +    supply 1.8 V.
> +  - hvdd-pex-supply: High-voltage supply for PCIe I/O and PCIe output clocks.
> +    Must supply 1.8 V.
> +  - vddio-pexctl-aud-supply: Power supply for PCIe side band signals. Must
> +    supply 1.8 V.
> +
>  Root ports are defined as subnodes of the PCIe controller node.
>  
>  Required properties:
> @@ -546,3 +561,114 @@ Board DTS:
>  			status = "okay";
>  		};
>  	};
> +
> +Tegra186:
> +---------
> +
> +SoC DTSI:
> +
> +	pcie@10003000 {
> +		compatible = "nvidia,tegra186-pcie";
> +		power-domains = <&bpmp TEGRA186_POWER_DOMAIN_PCX>;
> +		device_type = "pci";
> +		reg = <0x0 0x10003000 0x0 0x00000800   /* PADS registers */
> +		       0x0 0x10003800 0x0 0x00000800   /* AFI registers */
> +		       0x0 0x40000000 0x0 0x10000000>; /* configuration space */
> +		reg-names = "pads", "afi", "cs";
> +
> +		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */
> +			     <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */
> +		interrupt-names = "intr", "msi";
> +
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		bus-range = <0x00 0xff>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +		ranges = <0x82000000 0 0x10000000 0x0 0x10000000 0 0x00001000   /* port 0 configuration space */
> +			  0x82000000 0 0x10001000 0x0 0x10001000 0 0x00001000   /* port 1 configuration space */
> +			  0x82000000 0 0x10004000 0x0 0x10004000 0 0x00001000   /* port 2 configuration space */
> +			  0x81000000 0 0x0        0x0 0x50000000 0 0x00010000   /* downstream I/O (64 KiB) */
> +			  0x82000000 0 0x50100000 0x0 0x50100000 0 0x07F00000   /* non-prefetchable memory (127 MiB) */
> +			  0xc2000000 0 0x58000000 0x0 0x58000000 0 0x28000000>; /* prefetchable memory (640 MiB) */
> +
> +		clocks = <&bpmp TEGRA186_CLK_AFI>,
> +			 <&bpmp TEGRA186_CLK_PCIE>,
> +			 <&bpmp TEGRA186_CLK_PLLE>;
> +		clock-names = "afi", "pex", "pll_e";
> +
> +		resets = <&bpmp TEGRA186_RESET_AFI>,
> +			 <&bpmp TEGRA186_RESET_PCIE>,
> +			 <&bpmp TEGRA186_RESET_PCIEXCLK>;
> +		reset-names = "afi", "pex", "pcie_x";
> +
> +		status = "disabled";
> +
> +		pci@1,0 {
> +			device_type = "pci";
> +			assigned-addresses = <0x82000800 0 0x10000000 0 0x1000>;
> +			reg = <0x000800 0 0 0 0>;
> +			status = "disabled";
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			nvidia,num-lanes = <2>;
> +		};
> +
> +		pci@2,0 {
> +			device_type = "pci";
> +			assigned-addresses = <0x82001000 0 0x10001000 0 0x1000>;
> +			reg = <0x001000 0 0 0 0>;
> +			status = "disabled";
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			nvidia,num-lanes = <1>;
> +		};
> +
> +		pci@3,0 {
> +			device_type = "pci";
> +			assigned-addresses = <0x82001800 0 0x10004000 0 0x1000>;
> +			reg = <0x001800 0 0 0 0>;
> +			status = "disabled";
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			nvidia,num-lanes = <1>;
> +		};
> +	};
> +
> +Board DTS:
> +
> +	pcie@10003000 {
> +		status = "okay";
> +
> +		dvdd-pex-supply = <&vdd_pex>;
> +		hvdd-pex-pll-supply = <&vdd_1v8>;
> +		hvdd-pex-supply = <&vdd_1v8>;
> +		vddio-pexctl-aud-supply = <&vdd_1v8>;
> +
> +		pci@1,0 {
> +			nvidia,num-lanes = <4>;
> +			status = "okay";
> +		};
> +
> +		pci@2,0 {
> +			nvidia,num-lanes = <0>;
> +			status = "disabled";
> +		};
> +
> +		pci@3,0 {
> +			nvidia,num-lanes = <1>;
> +			status = "disabled";
> +		};
> +	};
> -- 
> 2.1.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
> UPHY programming is performed by BPMP, PHY enable calls are
> not required for Tegra186 PCIe. Power partition ungate is
> done by BPMP powergate driver.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> V2: Add soc->program_uphy check for phy_exit call
>  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 108 insertions(+), 24 deletions(-)

This works, excellent!

Tested-by: Thierry Reding <treding@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Bjorn Helgaas]

On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
> UPHY programming is performed by BPMP, PHY enable calls are
> not required for Tegra186 PCIe. Power partition ungate is
> done by BPMP powergate driver.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> V2: Add soc->program_uphy check for phy_exit call
>  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 108 insertions(+), 24 deletions(-)

> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
>  		afi_writel(pcie, value, AFI_FUSE);
>  	}
>  
> -	err = tegra_pcie_phy_power_on(pcie);
> -	if (err < 0) {
> -		dev_err(dev, "failed to power on PHY(s): %d\n", err);
> -		return err;
> +	if (soc->program_uphy) {
> +		err = tegra_pcie_phy_power_on(pcie);
> +		if (err < 0) {
> +			dev_err(dev, "failed to power on PHY(s): %d\n", err);
> +			return err;
> +		}

This looks good: it's obvious that the previously-supported devices
continue to work the same way because you set ".program_uphy = true"
for them.  (It would be even more obvious if you changed the sense so
that only the new device had to add this initializaer, but in general
I prefer the positive logic as you have here, and I did verify that
you added the initializer for all tegra_pcie_soc variants.)

> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
>  static void tegra_pcie_power_off(struct tegra_pcie *pcie)
>  {
>  	struct device *dev = pcie->dev;
> +	const struct tegra_pcie_soc *soc = pcie->soc;
>  	int err;
>  
>  	/* TODO: disable and unprepare clocks? */
>  
> -	err = tegra_pcie_phy_power_off(pcie);
> -	if (err < 0)
> -		dev_err(dev, "failed to power off PHY(s): %d\n", err);
> +	if (soc->program_uphy) {
> +		err = tegra_pcie_phy_power_off(pcie);
> +		if (err < 0)
> +			dev_err(dev, "failed to power off PHY(s): %d\n", err);
> +	}
>  
>  	reset_control_assert(pcie->pcie_xrst);
>  	reset_control_assert(pcie->afi_rst);
>  	reset_control_assert(pcie->pex_rst);
>  
> -	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> +	if (!dev->pm_domain)
> +		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);

But this one isn't obvious because nothing in your patch obviously
affects dev->pm_domain, so I can't tell whether this is safe.  It's
worth a changelog comment to help justify this.

Bjorn

Re: [PATCH V2 0/4] Add Tegra186 PCIe support

[Bjorn Helgaas]

On Wed, Sep 27, 2017 at 05:28:33PM +0530, Manikanta Maddireddy wrote:
> Tegra186 has three PCIe controllers which can be operated
> in 401, 211 or 111 lane configurations. Tegra TX2 platform
> has x4 and M.2 Key E PCIe slots, these patches enables
> x4 slot. BPMP programs UPHY lane0 ownership to USB,
> so M.2 Key E PCIe will not work.
> 
> Testing: x4 slot is verified with PCIe based USB3.1 card.
> PCIe link up, usb flash drive mounting and file copy are
> verified. These patches are also verified by
> Mikko Perttunen <mperttunen@nvidia.com> with an Intel 82574L
> ethernet card.
> 
> Patch V2 adds soc->program_uphy check for phy_exit call.
> 
> Pasting PCIe link up logs below.
> 
> [    1.452512] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
> [    1.452723] tegra-pcie 10003000.pcie: Failed to get supply 'dvdd-pex': -517
> [    1.561824] ehci-pci: EHCI PCI platform driver
> [    1.591587] ohci-pci: OHCI PCI platform driver
> [    3.072464] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
> [    3.078989] tegra-pcie 10003000.pcie: probing port 0, using 4 lanes
> [    3.087272] tegra-pcie 10003000.pcie: Slot present pin change, signature: 00000018
> [    3.128818] tegra-pcie 10003000.pcie: PCI host bridge to bus 0000:00
> [    3.135174] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> [    3.141352] pci_bus 0000:00: root bus resource [mem 0x50100000-0x57ffffff]
> [    3.148221] pci_bus 0000:00: root bus resource [mem 0x58000000-0x7fffffff pref]
> [    3.155542] pci_bus 0000:00: root bus resource [bus 00-ff]
> [    3.161173] pci 0000:00:01.0: [10de:10e5] type 01 class 0x060400
> [    3.161188] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
> [    3.170838] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
> [    3.180532] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x52 may corrupt adjacent RW1C bits
> [    3.190305] pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot D3cold
> [    3.190312] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4c may corrupt adjacent RW1C bits
> [    3.200158] pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> [    3.208158] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
> [    3.217918] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
> [    3.227649] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
> [    3.237382] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x6 may corrupt adjacent RW1C bits
> [    3.247135] pci 0000:01:00.0: [1b21:1242] type 00 class 0x0c0330
> [    3.247157] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
> [    3.256807] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
> [    3.266477] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00007fff 64bit]
> [    3.266544] pci 0000:01:00.0: enabling Extended Tags
> [    3.271622] pci 0000:01:00.0: PME# supported from D3hot D3cold
> [    3.271775] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> [    3.271798] pci 0000:00:01.0: BAR 14: assigned [mem 0x50100000-0x501fffff]
> [    3.278671] pci 0000:01:00.0: BAR 0: assigned [mem 0x50100000-0x50107fff 64bit]
> [    3.285993] pci 0000:00:01.0: PCI bridge to [bus 01]
> [    3.290959] pci 0000:00:01.0:   bridge window [mem 0x50100000-0x501fffff]
> [    3.297763] pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge
> [    3.304784] pcieport 0000:00:01.0: enabling device (0000 -> 0002)
> [    3.311014] pcieport 0000:00:01.0: Signaling PME with IRQ 57
> [    3.316768] pci 0000:01:00.0: enabling device (0000 -> 0002)
> 
> 
> Manikanta Maddireddy (4):
>   dt-bindings: pci: tegra: Document Tegra186 PCIe DT
>   PCI: tegra: Add Tegra186 PCIe support

I applied these two with Thierry's tested-by and acks to
pci/host-tegra for v4.15, thanks!  If you have a changelog update for
the pm_domain question, I can fold that in.

>   arm64: tegra: Add PCIe node for Tegra186
>   arm64: tegra: Enable PCIe on Jetson TX2
> 
>  .../bindings/pci/nvidia,tegra20-pcie.txt           | 134 ++++++++++++++++++++-
>  arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi     |  24 ++++
>  arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  82 +++++++++++++
>  drivers/pci/host/pci-tegra.c                       | 132 ++++++++++++++++----
>  4 files changed, 344 insertions(+), 28 deletions(-)

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3056 bytes --]

On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
> > UPHY programming is performed by BPMP, PHY enable calls are
> > not required for Tegra186 PCIe. Power partition ungate is
> > done by BPMP powergate driver.
> > 
> > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> > Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> > Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> > V2: Add soc->program_uphy check for phy_exit call
> >  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 108 insertions(+), 24 deletions(-)
> 
> > @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
> >  		afi_writel(pcie, value, AFI_FUSE);
> >  	}
> >  
> > -	err = tegra_pcie_phy_power_on(pcie);
> > -	if (err < 0) {
> > -		dev_err(dev, "failed to power on PHY(s): %d\n", err);
> > -		return err;
> > +	if (soc->program_uphy) {
> > +		err = tegra_pcie_phy_power_on(pcie);
> > +		if (err < 0) {
> > +			dev_err(dev, "failed to power on PHY(s): %d\n", err);
> > +			return err;
> > +		}
> 
> This looks good: it's obvious that the previously-supported devices
> continue to work the same way because you set ".program_uphy = true"
> for them.  (It would be even more obvious if you changed the sense so
> that only the new device had to add this initializaer, but in general
> I prefer the positive logic as you have here, and I did verify that
> you added the initializer for all tegra_pcie_soc variants.)
> 
> > @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
> >  static void tegra_pcie_power_off(struct tegra_pcie *pcie)
> >  {
> >  	struct device *dev = pcie->dev;
> > +	const struct tegra_pcie_soc *soc = pcie->soc;
> >  	int err;
> >  
> >  	/* TODO: disable and unprepare clocks? */
> >  
> > -	err = tegra_pcie_phy_power_off(pcie);
> > -	if (err < 0)
> > -		dev_err(dev, "failed to power off PHY(s): %d\n", err);
> > +	if (soc->program_uphy) {
> > +		err = tegra_pcie_phy_power_off(pcie);
> > +		if (err < 0)
> > +			dev_err(dev, "failed to power off PHY(s): %d\n", err);
> > +	}
> >  
> >  	reset_control_assert(pcie->pcie_xrst);
> >  	reset_control_assert(pcie->afi_rst);
> >  	reset_control_assert(pcie->pex_rst);
> >  
> > -	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> > +	if (!dev->pm_domain)
> > +		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> 
> But this one isn't obvious because nothing in your patch obviously
> affects dev->pm_domain, so I can't tell whether this is safe.  It's
> worth a changelog comment to help justify this.

The last sentence in the commit message is what this is about. Maybe it
should be more verbose:

	Since the BPMP controls the powergates on Tegra186, there is no
	need to manually power on and off the PCIe power partition. The
	BPMP generic power domain driver takes care of it instead.

?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 0/4] Add Tegra186 PCIe support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4426 bytes --]

On Tue, Oct 17, 2017 at 12:37:40PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 27, 2017 at 05:28:33PM +0530, Manikanta Maddireddy wrote:
> > Tegra186 has three PCIe controllers which can be operated
> > in 401, 211 or 111 lane configurations. Tegra TX2 platform
> > has x4 and M.2 Key E PCIe slots, these patches enables
> > x4 slot. BPMP programs UPHY lane0 ownership to USB,
> > so M.2 Key E PCIe will not work.
> > 
> > Testing: x4 slot is verified with PCIe based USB3.1 card.
> > PCIe link up, usb flash drive mounting and file copy are
> > verified. These patches are also verified by
> > Mikko Perttunen <mperttunen@nvidia.com> with an Intel 82574L
> > ethernet card.
> > 
> > Patch V2 adds soc->program_uphy check for phy_exit call.
> > 
> > Pasting PCIe link up logs below.
> > 
> > [    1.452512] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
> > [    1.452723] tegra-pcie 10003000.pcie: Failed to get supply 'dvdd-pex': -517
> > [    1.561824] ehci-pci: EHCI PCI platform driver
> > [    1.591587] ohci-pci: OHCI PCI platform driver
> > [    3.072464] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
> > [    3.078989] tegra-pcie 10003000.pcie: probing port 0, using 4 lanes
> > [    3.087272] tegra-pcie 10003000.pcie: Slot present pin change, signature: 00000018
> > [    3.128818] tegra-pcie 10003000.pcie: PCI host bridge to bus 0000:00
> > [    3.135174] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> > [    3.141352] pci_bus 0000:00: root bus resource [mem 0x50100000-0x57ffffff]
> > [    3.148221] pci_bus 0000:00: root bus resource [mem 0x58000000-0x7fffffff pref]
> > [    3.155542] pci_bus 0000:00: root bus resource [bus 00-ff]
> > [    3.161173] pci 0000:00:01.0: [10de:10e5] type 01 class 0x060400
> > [    3.161188] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
> > [    3.170838] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
> > [    3.180532] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x52 may corrupt adjacent RW1C bits
> > [    3.190305] pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot D3cold
> > [    3.190312] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4c may corrupt adjacent RW1C bits
> > [    3.200158] pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> > [    3.208158] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
> > [    3.217918] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
> > [    3.227649] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
> > [    3.237382] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x6 may corrupt adjacent RW1C bits
> > [    3.247135] pci 0000:01:00.0: [1b21:1242] type 00 class 0x0c0330
> > [    3.247157] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
> > [    3.256807] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
> > [    3.266477] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00007fff 64bit]
> > [    3.266544] pci 0000:01:00.0: enabling Extended Tags
> > [    3.271622] pci 0000:01:00.0: PME# supported from D3hot D3cold
> > [    3.271775] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> > [    3.271798] pci 0000:00:01.0: BAR 14: assigned [mem 0x50100000-0x501fffff]
> > [    3.278671] pci 0000:01:00.0: BAR 0: assigned [mem 0x50100000-0x50107fff 64bit]
> > [    3.285993] pci 0000:00:01.0: PCI bridge to [bus 01]
> > [    3.290959] pci 0000:00:01.0:   bridge window [mem 0x50100000-0x501fffff]
> > [    3.297763] pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge
> > [    3.304784] pcieport 0000:00:01.0: enabling device (0000 -> 0002)
> > [    3.311014] pcieport 0000:00:01.0: Signaling PME with IRQ 57
> > [    3.316768] pci 0000:01:00.0: enabling device (0000 -> 0002)
> > 
> > 
> > Manikanta Maddireddy (4):
> >   dt-bindings: pci: tegra: Document Tegra186 PCIe DT
> >   PCI: tegra: Add Tegra186 PCIe support
> 
> I applied these two with Thierry's tested-by and acks to
> pci/host-tegra for v4.15, thanks!  If you have a changelog update for
> the pm_domain question, I can fold that in.

Thanks!

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 0/4] Add Tegra186 PCIe support

[Manikanta Maddireddy]

On 18-Oct-17 1:58 AM, Thierry Reding wrote:
> On Tue, Oct 17, 2017 at 12:37:40PM -0500, Bjorn Helgaas wrote:
>> On Wed, Sep 27, 2017 at 05:28:33PM +0530, Manikanta Maddireddy wrote:
>>> Tegra186 has three PCIe controllers which can be operated
>>> in 401, 211 or 111 lane configurations. Tegra TX2 platform
>>> has x4 and M.2 Key E PCIe slots, these patches enables
>>> x4 slot. BPMP programs UPHY lane0 ownership to USB,
>>> so M.2 Key E PCIe will not work.
>>>
>>> Testing: x4 slot is verified with PCIe based USB3.1 card.
>>> PCIe link up, usb flash drive mounting and file copy are
>>> verified. These patches are also verified by
>>> Mikko Perttunen <mperttunen@nvidia.com> with an Intel 82574L
>>> ethernet card.
>>>
>>> Patch V2 adds soc->program_uphy check for phy_exit call.
>>>
>>> Pasting PCIe link up logs below.
>>>
>>> [    1.452512] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
>>> [    1.452723] tegra-pcie 10003000.pcie: Failed to get supply 'dvdd-pex': -517
>>> [    1.561824] ehci-pci: EHCI PCI platform driver
>>> [    1.591587] ohci-pci: OHCI PCI platform driver
>>> [    3.072464] tegra-pcie 10003000.pcie: 4x1, 1x1 configuration
>>> [    3.078989] tegra-pcie 10003000.pcie: probing port 0, using 4 lanes
>>> [    3.087272] tegra-pcie 10003000.pcie: Slot present pin change, signature: 00000018
>>> [    3.128818] tegra-pcie 10003000.pcie: PCI host bridge to bus 0000:00
>>> [    3.135174] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
>>> [    3.141352] pci_bus 0000:00: root bus resource [mem 0x50100000-0x57ffffff]
>>> [    3.148221] pci_bus 0000:00: root bus resource [mem 0x58000000-0x7fffffff pref]
>>> [    3.155542] pci_bus 0000:00: root bus resource [bus 00-ff]
>>> [    3.161173] pci 0000:00:01.0: [10de:10e5] type 01 class 0x060400
>>> [    3.161188] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
>>> [    3.170838] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4 may corrupt adjacent RW1C bits
>>> [    3.180532] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x52 may corrupt adjacent RW1C bits
>>> [    3.190305] pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot D3cold
>>> [    3.190312] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x4c may corrupt adjacent RW1C bits
>>> [    3.200158] pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>>> [    3.208158] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
>>> [    3.217918] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
>>> [    3.227649] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x3e may corrupt adjacent RW1C bits
>>> [    3.237382] pci_bus 0000:00: 2-byte config write to 0000:00:01.0 offset 0x6 may corrupt adjacent RW1C bits
>>> [    3.247135] pci 0000:01:00.0: [1b21:1242] type 00 class 0x0c0330
>>> [    3.247157] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
>>> [    3.256807] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset 0x4 may corrupt adjacent RW1C bits
>>> [    3.266477] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00007fff 64bit]
>>> [    3.266544] pci 0000:01:00.0: enabling Extended Tags
>>> [    3.271622] pci 0000:01:00.0: PME# supported from D3hot D3cold
>>> [    3.271775] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
>>> [    3.271798] pci 0000:00:01.0: BAR 14: assigned [mem 0x50100000-0x501fffff]
>>> [    3.278671] pci 0000:01:00.0: BAR 0: assigned [mem 0x50100000-0x50107fff 64bit]
>>> [    3.285993] pci 0000:00:01.0: PCI bridge to [bus 01]
>>> [    3.290959] pci 0000:00:01.0:   bridge window [mem 0x50100000-0x501fffff]
>>> [    3.297763] pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge
>>> [    3.304784] pcieport 0000:00:01.0: enabling device (0000 -> 0002)
>>> [    3.311014] pcieport 0000:00:01.0: Signaling PME with IRQ 57
>>> [    3.316768] pci 0000:01:00.0: enabling device (0000 -> 0002)
>>>
>>>
>>> Manikanta Maddireddy (4):
>>>   dt-bindings: pci: tegra: Document Tegra186 PCIe DT
>>>   PCI: tegra: Add Tegra186 PCIe support
>>
>> I applied these two with Thierry's tested-by and acks to
>> pci/host-tegra for v4.15, thanks!  If you have a changelog update for
>> the pm_domain question, I can fold that in.
> 
> Thanks!
> 
> Thierry
> 

Thank you Mikko, Thierry and Bjorn for testing and reviewing these patches.

Bjorn,

Please let me know if Thierry's elaborate changelog update in [PATCH V2 2/4] clarified the pm_domain question or not?
If that helps, please take that into changelog.

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Bjorn Helgaas]

On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:
> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
> > > UPHY programming is performed by BPMP, PHY enable calls are
> > > not required for Tegra186 PCIe. Power partition ungate is
> > > done by BPMP powergate driver.
> > > 
> > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> > > Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> > > Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> > > ---
> > > V2: Add soc->program_uphy check for phy_exit call
> > >  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
> > >  1 file changed, 108 insertions(+), 24 deletions(-)
> > 
> > > @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
> > >  		afi_writel(pcie, value, AFI_FUSE);
> > >  	}
> > >  
> > > -	err = tegra_pcie_phy_power_on(pcie);
> > > -	if (err < 0) {
> > > -		dev_err(dev, "failed to power on PHY(s): %d\n", err);
> > > -		return err;
> > > +	if (soc->program_uphy) {
> > > +		err = tegra_pcie_phy_power_on(pcie);
> > > +		if (err < 0) {
> > > +			dev_err(dev, "failed to power on PHY(s): %d\n", err);
> > > +			return err;
> > > +		}
> > 
> > This looks good: it's obvious that the previously-supported devices
> > continue to work the same way because you set ".program_uphy = true"
> > for them.  (It would be even more obvious if you changed the sense so
> > that only the new device had to add this initializaer, but in general
> > I prefer the positive logic as you have here, and I did verify that
> > you added the initializer for all tegra_pcie_soc variants.)
> > 
> > > @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
> > >  static void tegra_pcie_power_off(struct tegra_pcie *pcie)
> > >  {
> > >  	struct device *dev = pcie->dev;
> > > +	const struct tegra_pcie_soc *soc = pcie->soc;
> > >  	int err;
> > >  
> > >  	/* TODO: disable and unprepare clocks? */
> > >  
> > > -	err = tegra_pcie_phy_power_off(pcie);
> > > -	if (err < 0)
> > > -		dev_err(dev, "failed to power off PHY(s): %d\n", err);
> > > +	if (soc->program_uphy) {
> > > +		err = tegra_pcie_phy_power_off(pcie);
> > > +		if (err < 0)
> > > +			dev_err(dev, "failed to power off PHY(s): %d\n", err);
> > > +	}
> > >  
> > >  	reset_control_assert(pcie->pcie_xrst);
> > >  	reset_control_assert(pcie->afi_rst);
> > >  	reset_control_assert(pcie->pex_rst);
> > >  
> > > -	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> > > +	if (!dev->pm_domain)
> > > +		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> > 
> > But this one isn't obvious because nothing in your patch obviously
> > affects dev->pm_domain, so I can't tell whether this is safe.  It's
> > worth a changelog comment to help justify this.
> 
> The last sentence in the commit message is what this is about. Maybe it
> should be more verbose:
> 
> 	Since the BPMP controls the powergates on Tegra186, there is no
> 	need to manually power on and off the PCIe power partition. The
> 	BPMP generic power domain driver takes care of it instead.

If you know Tegra intimately, maybe that clarifies it.  I don't, so my
problem is that there's nothing in the patch that helps me verify
this.  I infer that maybe there's something different in the DT that
means dev->pm_domain will be set for Tegra186, but not for other
variants?

There should be something that helps the reader connect the dots.

Bjorn

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Manikanta Maddireddy]

On 18-Oct-17 6:57 PM, Bjorn Helgaas wrote:
> On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:
>> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:
>>> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
>>>> UPHY programming is performed by BPMP, PHY enable calls are
>>>> not required for Tegra186 PCIe. Power partition ungate is
>>>> done by BPMP powergate driver.
>>>>
>>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>>>> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>> ---
>>>> V2: Add soc->program_uphy check for phy_exit call
>>>>  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
>>>>  1 file changed, 108 insertions(+), 24 deletions(-)
>>>
>>>> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
>>>>  		afi_writel(pcie, value, AFI_FUSE);
>>>>  	}
>>>>  
>>>> -	err = tegra_pcie_phy_power_on(pcie);
>>>> -	if (err < 0) {
>>>> -		dev_err(dev, "failed to power on PHY(s): %d\n", err);
>>>> -		return err;
>>>> +	if (soc->program_uphy) {
>>>> +		err = tegra_pcie_phy_power_on(pcie);
>>>> +		if (err < 0) {
>>>> +			dev_err(dev, "failed to power on PHY(s): %d\n", err);
>>>> +			return err;
>>>> +		}
>>>
>>> This looks good: it's obvious that the previously-supported devices
>>> continue to work the same way because you set ".program_uphy = true"
>>> for them.  (It would be even more obvious if you changed the sense so
>>> that only the new device had to add this initializaer, but in general
>>> I prefer the positive logic as you have here, and I did verify that
>>> you added the initializer for all tegra_pcie_soc variants.)
>>>
>>>> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
>>>>  static void tegra_pcie_power_off(struct tegra_pcie *pcie)
>>>>  {
>>>>  	struct device *dev = pcie->dev;
>>>> +	const struct tegra_pcie_soc *soc = pcie->soc;
>>>>  	int err;
>>>>  
>>>>  	/* TODO: disable and unprepare clocks? */
>>>>  
>>>> -	err = tegra_pcie_phy_power_off(pcie);
>>>> -	if (err < 0)
>>>> -		dev_err(dev, "failed to power off PHY(s): %d\n", err);
>>>> +	if (soc->program_uphy) {
>>>> +		err = tegra_pcie_phy_power_off(pcie);
>>>> +		if (err < 0)
>>>> +			dev_err(dev, "failed to power off PHY(s): %d\n", err);
>>>> +	}
>>>>  
>>>>  	reset_control_assert(pcie->pcie_xrst);
>>>>  	reset_control_assert(pcie->afi_rst);
>>>>  	reset_control_assert(pcie->pex_rst);
>>>>  
>>>> -	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
>>>> +	if (!dev->pm_domain)
>>>> +		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
>>>
>>> But this one isn't obvious because nothing in your patch obviously
>>> affects dev->pm_domain, so I can't tell whether this is safe.  It's
>>> worth a changelog comment to help justify this.
>>
>> The last sentence in the commit message is what this is about. Maybe it
>> should be more verbose:
>>
>> 	Since the BPMP controls the powergates on Tegra186, there is no
>> 	need to manually power on and off the PCIe power partition. The
>> 	BPMP generic power domain driver takes care of it instead.
> 
> If you know Tegra intimately, maybe that clarifies it.  I don't, so my
> problem is that there's nothing in the patch that helps me verify
> this.  I infer that maybe there's something different in the DT that
> means dev->pm_domain will be set for Tegra186, but not for other
> variants?
> 
> There should be something that helps the reader connect the dots.
> 
> Bjorn
> 
Hi Bjorn,

Till tegra210 pmc driver provides direct powergate power ON/OFF functions.

In tegra186.dtsi power-domains property is added in pcie@10003000 DT node. reference: https://patchwork.ozlabs.org/patch/819113/
powergate-bpmp.c is the pm domain provider for pcie powergate. 
Platform driver will look for pm domain provider of Tegra pcie host driver and sets dev->pm_domains.
powergate-bpmp.c driver registers powergate power ON/OFF functions to generic PD power_on/powerr_off callback functions.
Generic power domain will take care of calling power_on before Tegra PCIe probe.

So in this patch I used dev->pm_domain to skip pmc driver calls.

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Bjorn Helgaas]

On Wed, Oct 18, 2017 at 07:44:12PM +0530, Manikanta Maddireddy wrote:
> 
> 
> On 18-Oct-17 6:57 PM, Bjorn Helgaas wrote:
> > On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:
> >> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:
> >>> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
> >>>> UPHY programming is performed by BPMP, PHY enable calls are
> >>>> not required for Tegra186 PCIe. Power partition ungate is
> >>>> done by BPMP powergate driver.
> >>>>
> >>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> >>>> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> >>>> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> >>>> ---
> >>>> V2: Add soc->program_uphy check for phy_exit call
> >>>>  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
> >>>>  1 file changed, 108 insertions(+), 24 deletions(-)
> >>>
> >>>> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
> >>>>  		afi_writel(pcie, value, AFI_FUSE);
> >>>>  	}
> >>>>  
> >>>> -	err = tegra_pcie_phy_power_on(pcie);
> >>>> -	if (err < 0) {
> >>>> -		dev_err(dev, "failed to power on PHY(s): %d\n", err);
> >>>> -		return err;
> >>>> +	if (soc->program_uphy) {
> >>>> +		err = tegra_pcie_phy_power_on(pcie);
> >>>> +		if (err < 0) {
> >>>> +			dev_err(dev, "failed to power on PHY(s): %d\n", err);
> >>>> +			return err;
> >>>> +		}
> >>>
> >>> This looks good: it's obvious that the previously-supported devices
> >>> continue to work the same way because you set ".program_uphy = true"
> >>> for them.  (It would be even more obvious if you changed the sense so
> >>> that only the new device had to add this initializaer, but in general
> >>> I prefer the positive logic as you have here, and I did verify that
> >>> you added the initializer for all tegra_pcie_soc variants.)
> >>>
> >>>> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
> >>>>  static void tegra_pcie_power_off(struct tegra_pcie *pcie)
> >>>>  {
> >>>>  	struct device *dev = pcie->dev;
> >>>> +	const struct tegra_pcie_soc *soc = pcie->soc;
> >>>>  	int err;
> >>>>  
> >>>>  	/* TODO: disable and unprepare clocks? */
> >>>>  
> >>>> -	err = tegra_pcie_phy_power_off(pcie);
> >>>> -	if (err < 0)
> >>>> -		dev_err(dev, "failed to power off PHY(s): %d\n", err);
> >>>> +	if (soc->program_uphy) {
> >>>> +		err = tegra_pcie_phy_power_off(pcie);
> >>>> +		if (err < 0)
> >>>> +			dev_err(dev, "failed to power off PHY(s): %d\n", err);
> >>>> +	}
> >>>>  
> >>>>  	reset_control_assert(pcie->pcie_xrst);
> >>>>  	reset_control_assert(pcie->afi_rst);
> >>>>  	reset_control_assert(pcie->pex_rst);
> >>>>  
> >>>> -	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> >>>> +	if (!dev->pm_domain)
> >>>> +		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> >>>
> >>> But this one isn't obvious because nothing in your patch obviously
> >>> affects dev->pm_domain, so I can't tell whether this is safe.  It's
> >>> worth a changelog comment to help justify this.
> >>
> >> The last sentence in the commit message is what this is about. Maybe it
> >> should be more verbose:
> >>
> >> 	Since the BPMP controls the powergates on Tegra186, there is no
> >> 	need to manually power on and off the PCIe power partition. The
> >> 	BPMP generic power domain driver takes care of it instead.
> > 
> > If you know Tegra intimately, maybe that clarifies it.  I don't, so my
> > problem is that there's nothing in the patch that helps me verify
> > this.  I infer that maybe there's something different in the DT that
> > means dev->pm_domain will be set for Tegra186, but not for other
> > variants?
> > 
> > There should be something that helps the reader connect the dots.
> 
> Till tegra210 pmc driver provides direct powergate power ON/OFF functions.
> 
> In tegra186.dtsi power-domains property is added in pcie@10003000 DT node. reference: https://patchwork.ozlabs.org/patch/819113/
> powergate-bpmp.c is the pm domain provider for pcie powergate. 
> Platform driver will look for pm domain provider of Tegra pcie host driver and sets dev->pm_domains.
> powergate-bpmp.c driver registers powergate power ON/OFF functions to generic PD power_on/powerr_off callback functions.
> Generic power domain will take care of calling power_on before Tegra PCIe probe.
> 
> So in this patch I used dev->pm_domain to skip pmc driver calls.

Thanks, I updated the changelog as follows:


commit 9cea513d8cbc75ee26327d3d8971fe7b58288d8f
Author: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Date:   Wed Sep 27 17:28:35 2017 +0530

    PCI: tegra: Add Tegra186 PCIe support
    
    Add Tegra186 PCIe support.  UPHY programming is performed by BPMP; PHY
    enable calls are not required for Tegra186 PCIe.
    
    Power partition ungate is done by BPMP powergate driver.  The Tegra186
    DT description must include a "power-domains" property, which results in
    dev->pm_domain being set.

Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support

[Manikanta Maddireddy]

On 18-Oct-17 9:59 PM, Bjorn Helgaas wrote:
> On Wed, Oct 18, 2017 at 07:44:12PM +0530, Manikanta Maddireddy wrote:
>>
>>
>> On 18-Oct-17 6:57 PM, Bjorn Helgaas wrote:
>>> On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:
>>>> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:
>>>>> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:
>>>>>> UPHY programming is performed by BPMP, PHY enable calls are
>>>>>> not required for Tegra186 PCIe. Power partition ungate is
>>>>>> done by BPMP powergate driver.
>>>>>>
>>>>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>>>>>> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>>>> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
>>>>>> ---
>>>>>> V2: Add soc->program_uphy check for phy_exit call
>>>>>>  drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------
>>>>>>  1 file changed, 108 insertions(+), 24 deletions(-)
>>>>>
>>>>>> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
>>>>>>  		afi_writel(pcie, value, AFI_FUSE);
>>>>>>  	}
>>>>>>  
>>>>>> -	err = tegra_pcie_phy_power_on(pcie);
>>>>>> -	if (err < 0) {
>>>>>> -		dev_err(dev, "failed to power on PHY(s): %d\n", err);
>>>>>> -		return err;
>>>>>> +	if (soc->program_uphy) {
>>>>>> +		err = tegra_pcie_phy_power_on(pcie);
>>>>>> +		if (err < 0) {
>>>>>> +			dev_err(dev, "failed to power on PHY(s): %d\n", err);
>>>>>> +			return err;
>>>>>> +		}
>>>>>
>>>>> This looks good: it's obvious that the previously-supported devices
>>>>> continue to work the same way because you set ".program_uphy = true"
>>>>> for them.  (It would be even more obvious if you changed the sense so
>>>>> that only the new device had to add this initializaer, but in general
>>>>> I prefer the positive logic as you have here, and I did verify that
>>>>> you added the initializer for all tegra_pcie_soc variants.)
>>>>>
>>>>>> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)
>>>>>>  static void tegra_pcie_power_off(struct tegra_pcie *pcie)
>>>>>>  {
>>>>>>  	struct device *dev = pcie->dev;
>>>>>> +	const struct tegra_pcie_soc *soc = pcie->soc;
>>>>>>  	int err;
>>>>>>  
>>>>>>  	/* TODO: disable and unprepare clocks? */
>>>>>>  
>>>>>> -	err = tegra_pcie_phy_power_off(pcie);
>>>>>> -	if (err < 0)
>>>>>> -		dev_err(dev, "failed to power off PHY(s): %d\n", err);
>>>>>> +	if (soc->program_uphy) {
>>>>>> +		err = tegra_pcie_phy_power_off(pcie);
>>>>>> +		if (err < 0)
>>>>>> +			dev_err(dev, "failed to power off PHY(s): %d\n", err);
>>>>>> +	}
>>>>>>  
>>>>>>  	reset_control_assert(pcie->pcie_xrst);
>>>>>>  	reset_control_assert(pcie->afi_rst);
>>>>>>  	reset_control_assert(pcie->pex_rst);
>>>>>>  
>>>>>> -	tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
>>>>>> +	if (!dev->pm_domain)
>>>>>> +		tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
>>>>>
>>>>> But this one isn't obvious because nothing in your patch obviously
>>>>> affects dev->pm_domain, so I can't tell whether this is safe.  It's
>>>>> worth a changelog comment to help justify this.
>>>>
>>>> The last sentence in the commit message is what this is about. Maybe it
>>>> should be more verbose:
>>>>
>>>> 	Since the BPMP controls the powergates on Tegra186, there is no
>>>> 	need to manually power on and off the PCIe power partition. The
>>>> 	BPMP generic power domain driver takes care of it instead.
>>>
>>> If you know Tegra intimately, maybe that clarifies it.  I don't, so my
>>> problem is that there's nothing in the patch that helps me verify
>>> this.  I infer that maybe there's something different in the DT that
>>> means dev->pm_domain will be set for Tegra186, but not for other
>>> variants?
>>>
>>> There should be something that helps the reader connect the dots.
>>
>> Till tegra210 pmc driver provides direct powergate power ON/OFF functions.
>>
>> In tegra186.dtsi power-domains property is added in pcie@10003000 DT node. reference: https://patchwork.ozlabs.org/patch/819113/
>> powergate-bpmp.c is the pm domain provider for pcie powergate. 
>> Platform driver will look for pm domain provider of Tegra pcie host driver and sets dev->pm_domains.
>> powergate-bpmp.c driver registers powergate power ON/OFF functions to generic PD power_on/powerr_off callback functions.
>> Generic power domain will take care of calling power_on before Tegra PCIe probe.
>>
>> So in this patch I used dev->pm_domain to skip pmc driver calls.
> 
> Thanks, I updated the changelog as follows:
> 
> 
> commit 9cea513d8cbc75ee26327d3d8971fe7b58288d8f
> Author: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Date:   Wed Sep 27 17:28:35 2017 +0530
> 
>     PCI: tegra: Add Tegra186 PCIe support
>     
>     Add Tegra186 PCIe support.  UPHY programming is performed by BPMP; PHY
>     enable calls are not required for Tegra186 PCIe.
>     
>     Power partition ungate is done by BPMP powergate driver.  The Tegra186
>     DT description must include a "power-domains" property, which results in
>     dev->pm_domain being set.
> 
Thank you Bjorn

Re: [PATCH V2 3/4] arm64: tegra: Add PCIe node for Tegra186

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On Wed, Sep 27, 2017 at 05:28:36PM +0530, Manikanta Maddireddy wrote:
> Tegra186 has three PCIe controllers, which can be operated
> in 401, 211 or 111 lane combinations. Add DT support for
> PCIe controllers.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> V2: No change in this patch
>  arch/arm64/boot/dts/nvidia/tegra186.dtsi | 82 ++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> index 0b0552c9f7dd..9edf2a839e5d 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> @@ -443,6 +443,7 @@
>  		shmem = <&cpu_bpmp_tx &cpu_bpmp_rx>;
>  		#clock-cells = <1>;
>  		#reset-cells = <1>;
> +		#power-domain-cells = <1>;
>  
>  		bpmp_i2c: i2c {
>  			compatible = "nvidia,tegra186-bpmp-i2c";
> @@ -465,4 +466,85 @@
>  				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>  		interrupt-parent = <&gic>;
>  	};
> +
> +	pcie@10003000 {

I sorted this in correctly. For the future, please sort nodes by unit
address. If a node doesn't have a unit address, sort it by name and
after those with a unit address.

Also removed a gratuituous blank line.

Applied to for-4.15/arm64/dt.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 4/4] arm64: tegra: Enable PCIe on Jetson TX2

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Wed, Sep 27, 2017 at 05:28:37PM +0530, Manikanta Maddireddy wrote:
> Enable x4 PCIe slot on Jetson TX2.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
> Tested-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> V2: No change in this patch
>  arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
> index cf84d7046ad5..a4d96b2a23b4 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi
> @@ -378,4 +378,28 @@
>  			vin-supply = <&vdd_1v8>;
>  		};
>  	};
> +
> +	pcie@10003000 {
> +		status = "okay";
> +
> +		dvdd-pex-supply = <&vdd_pex>;
> +		hvdd-pex-pll-supply = <&vdd_1v8>;
> +		hvdd-pex-supply = <&vdd_1v8>;
> +		vddio-pexctl-aud-supply = <&vdd_1v8>;
> +
> +		pci@1,0 {
> +			nvidia,num-lanes = <4>;
> +			status = "okay";
> +		};
> +
> +		pci@2,0 {
> +			nvidia,num-lanes = <0>;
> +			status = "disabled";
> +		};
> +
> +		pci@3,0 {
> +			nvidia,num-lanes = <1>;
> +			status = "disabled";
> +		};
> +	};

I moved this to tegra186-p2771-0000.dts because it exposes connectors
that are on the carrier board rather than the processor module.

Applied to for-4.15/arm64/dt, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]