[PATCH v5 0/4] PCI: tegra: Add Tegra264 support

[PATCH v5 0/4] PCI: tegra: Add Tegra264 support

[Thierry Reding]

Hi,

this series adds support for the PCIe controllers found on the Tegra264
SoC. There are six instances, one of which is for internal purposes only
and the other five are general purpose.

The first patch tweaks the DT bindings slightly to avoid new DT compiler
warnings that slipped through because they are now disabled by default
(-Wno-unit_address_vs_reg).

Before adding the driver in patch 3, patch 2 introduces some new common
wait times for PCIe and unifies the way that drivers use them. Finally,
patch 4 reorders the reg and reg-names property entries to match the
bindings changes from patch 1.

All of the prerequisites were merged in v7.1-rc1, so this can be applied
to the PCI tree directly. Optionally I can also pick up patch 4 into the
Tegra tree, but there should be no conflicts, so feel free to pick this
up with the rest.

Thanks,
Thierry

Changes in v5:
- address review comments for the PCI driver patch
- Link to v4: https://patch.msgid.link/20260402-tegra264-pcie-v4-0-21e2e19987e8@nvidia.com

Changes in v4:
- strip out dependencies that are going in through the ARM SoC tree
- revert bindings to oneOf construct so that we don't produce new DTC
  warnings
- Link to v3: https://patch.msgid.link/20260326135855.2795149-1-thierry.reding@kernel.org

Changes in v3:
- integrate PCI standard wait times patch into the series to maintain
  bisectability
- fix review comments from Mikko
- Link to v2: https://patch.msgid.link/20260320225443.2571920-1-thierry.reding@kernel.org

Changes in v2:
- fix an issue with sanity-checking disabled BARs
- address review comments
- Link to v1: https://patch.msgid.link/20260319160110.2131954-1-thierry.reding@kernel.org

Thanks,
Thierry

---
Thierry Reding (4):
      dt-bindings: pci: Strictly distinguish C0 from C1-C5
      PCI: Use standard wait times for PCIe link monitoring
      PCI: tegra: Add Tegra264 support
      arm64: tegra: Reorder reg and reg-names to match bindings

 .../bindings/pci/nvidia,tegra264-pcie.yaml         |  75 ++-
 arch/arm64/boot/dts/nvidia/tegra264.dtsi           |  48 +-
 drivers/pci/controller/Kconfig                     |   9 +-
 drivers/pci/controller/Makefile                    |   1 +
 .../controller/cadence/pcie-cadence-host-common.c  |   6 +-
 .../pci/controller/cadence/pcie-cadence-lga-regs.h |   5 -
 drivers/pci/controller/mobiveil/pcie-mobiveil.c    |   4 +-
 drivers/pci/controller/mobiveil/pcie-mobiveil.h    |   5 -
 drivers/pci/controller/pci-aardvark.c              |   7 +-
 drivers/pci/controller/pcie-tegra264.c             | 544 +++++++++++++++++++++
 drivers/pci/controller/pcie-xilinx-nwl.c           |   9 +-
 drivers/pci/controller/plda/pcie-starfive.c        |   9 +-
 drivers/pci/pci.h                                  |   2 +
 13 files changed, 641 insertions(+), 83 deletions(-)
---
base-commit: 550604d6c9b9efc8d068aff94dc301694a7afdee
change-id: 20260402-tegra264-pcie-e30abe23da07

Best regards,
--  
Thierry Reding <treding@nvidia.com>

[PATCH v5 1/4] dt-bindings: pci: Strictly distinguish C0 from C1-C5

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Instead of using the ECAM registers as the first entry, strictly make a
distinction between C0 and C1-C5. This is needed because otherwise the
unit address doesn't match the first "reg" entry. We also cannot change
the ordering of these nodes to follow the ECAM addresses because that
would put them outside of their "control bus" hierarchy since the ECAM
address space is a global one outside of any of the control busses.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- rebase on top of v7.1-rc1, make it into a fix

Changes in v4:
- ECAM is outside of the controller's region, so it cannot be the first
  reg entry, otherwise we get warnings because it doesn't match the
  unit-address, so revert back to oneOf construct

Changes in v2:
- move ECAM region first and unify C0 vs. C1-C5
- move unevaluatedProperties to right before the examples
- add description to clarify the two types of controllers
- add examples for C0 and C1-C5
---
 .../bindings/pci/nvidia,tegra264-pcie.yaml         | 75 ++++++++++++++--------
 1 file changed, 50 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
index dc4f8725c9f5..acb677d477fb 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
@@ -10,32 +10,23 @@ maintainers:
   - Thierry Reding <thierry.reding@gmail.com>
   - Jon Hunter <jonathanh@nvidia.com>
 
+description: |
+  Of the six PCIe controllers found on Tegra264, one (C0) is used for the
+  internal GPU and the other five (C1-C5) are routed to connectors such as
+  PCI or M.2 slots. Therefore the UPHY registers (XPL) exist only for C1
+  through C5, but not for C0.
+
 properties:
   compatible:
     const: nvidia,tegra264-pcie
 
   reg:
-    description: |
-      Of the six PCIe controllers found on Tegra264, one (C0) is used for the
-      internal GPU and the other five (C1-C5) are routed to connectors such as
-      PCI or M.2 slots. Therefore the UPHY registers (XPL) exist only for C1
-      through C5, but not for C0.
     minItems: 4
-    items:
-      - description: ECAM-compatible configuration space
-      - description: application layer registers
-      - description: transaction layer registers
-      - description: privileged transaction layer registers
-      - description: data link/physical layer registers (not available on C0)
+    maxItems: 5
 
   reg-names:
     minItems: 4
-    items:
-      - const: ecam
-      - const: xal
-      - const: xtl
-      - const: xtl-pri
-      - const: xpl
+    maxItems: 5
 
   interrupts:
     minItems: 1
@@ -70,6 +61,40 @@ required:
 
 allOf:
   - $ref: /schemas/pci/pci-host-bridge.yaml#
+  - oneOf:
+      - description: C0 controller (no UPHY)
+        properties:
+          reg:
+            items:
+              - description: application layer registers
+              - description: transaction layer registers
+              - description: privileged transaction layer registers
+              - description: ECAM compatible configuration space
+
+          reg-names:
+            items:
+              - const: xal
+              - const: xtl
+              - const: xtl-pri
+              - const: ecam
+
+      - description: C1-C5 controllers (with UPHY)
+        properties:
+          reg:
+            items:
+              - description: application layer registers
+              - description: transaction layer registers
+              - description: privileged transaction layer registers
+              - description: data link/physical layer registers
+              - description: ECAM compatible configuration space
+
+          reg-names:
+            items:
+              - const: xal
+              - const: xtl
+              - const: xtl-pri
+              - const: xpl
+              - const: ecam
 
 unevaluatedProperties: false
 
@@ -81,11 +106,11 @@ examples:
 
       pci@c000000 {
         compatible = "nvidia,tegra264-pcie";
-        reg = <0xd0 0xb0000000 0x0 0x10000000>,
-              <0x00 0x0c000000 0x0 0x00004000>,
+        reg = <0x00 0x0c000000 0x0 0x00004000>,
               <0x00 0x0c004000 0x0 0x00001000>,
-              <0x00 0x0c005000 0x0 0x00001000>;
-        reg-names = "ecam", "xal", "xtl", "xtl-pri";
+              <0x00 0x0c005000 0x0 0x00001000>,
+              <0xd0 0xb0000000 0x0 0x10000000>;
+        reg-names = "xal", "xtl", "xtl-pri", "ecam";
         #address-cells = <3>;
         #size-cells = <2>;
         device_type = "pci";
@@ -118,12 +143,12 @@ examples:
 
       pci@8400000 {
         compatible = "nvidia,tegra264-pcie";
-        reg = <0xa8 0xb0000000 0x0 0x10000000>,
-              <0x00 0x08400000 0x0 0x00004000>,
+        reg = <0x00 0x08400000 0x0 0x00004000>,
               <0x00 0x08404000 0x0 0x00001000>,
               <0x00 0x08405000 0x0 0x00001000>,
-              <0x00 0x08410000 0x0 0x00010000>;
-        reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+              <0x00 0x08410000 0x0 0x00010000>,
+              <0xa8 0xb0000000 0x0 0x10000000>;
+        reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
         #address-cells = <3>;
         #size-cells = <2>;
         device_type = "pci";

-- 
2.52.0

[PATCH v5 2/4] PCI: Use standard wait times for PCIe link monitoring

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Instead of defining the wait values for each driver, use common values
defined in the core pci.h header file. Note that most drivers don't use
the millisecond waits, but rather usleep_range(), so add these commonly
used values to the header so that all drivers can use them.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- fix build for Cadence
---
 drivers/pci/controller/cadence/pcie-cadence-host-common.c | 6 ++++--
 drivers/pci/controller/cadence/pcie-cadence-lga-regs.h    | 5 -----
 drivers/pci/controller/mobiveil/pcie-mobiveil.c           | 4 ++--
 drivers/pci/controller/mobiveil/pcie-mobiveil.h           | 5 -----
 drivers/pci/controller/pci-aardvark.c                     | 7 ++-----
 drivers/pci/controller/pcie-xilinx-nwl.c                  | 9 ++-------
 drivers/pci/controller/plda/pcie-starfive.c               | 9 ++-------
 drivers/pci/pci.h                                         | 2 ++
 8 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
index 2b0211870f02..72b36c70f389 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
@@ -15,6 +15,8 @@
 #include "pcie-cadence.h"
 #include "pcie-cadence-host-common.h"
 
+#include "../../pci.h"
+
 #define LINK_RETRAIN_TIMEOUT HZ
 
 u64 bar_max_size[] = {
@@ -53,12 +55,12 @@ int cdns_pcie_host_wait_for_link(struct cdns_pcie *pcie,
 	int retries;
 
 	/* Check if the link is up or not */
-	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+	for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
 		if (pcie_link_up(pcie)) {
 			dev_info(dev, "Link up\n");
 			return 0;
 		}
-		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+		usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
 	}
 
 	return -ETIMEDOUT;
diff --git a/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h b/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h
index 857b2140c5d2..15dc4fcaf45d 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h
+++ b/drivers/pci/controller/cadence/pcie-cadence-lga-regs.h
@@ -10,11 +10,6 @@
 
 #include <linux/bitfield.h>
 
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES	10
-#define LINK_WAIT_USLEEP_MIN	90000
-#define LINK_WAIT_USLEEP_MAX	100000
-
 /* Local Management Registers */
 #define CDNS_PCIE_LM_BASE	0x00100000
 
diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.c b/drivers/pci/controller/mobiveil/pcie-mobiveil.c
index 62ecbaeb0a60..cc102032c1e6 100644
--- a/drivers/pci/controller/mobiveil/pcie-mobiveil.c
+++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.c
@@ -218,11 +218,11 @@ int mobiveil_bringup_link(struct mobiveil_pcie *pcie)
 	int retries;
 
 	/* check if the link is up or not */
-	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+	for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
 		if (mobiveil_pcie_link_up(pcie))
 			return 0;
 
-		usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX);
+		usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
 	}
 
 	dev_err(&pcie->pdev->dev, "link never came up\n");
diff --git a/drivers/pci/controller/mobiveil/pcie-mobiveil.h b/drivers/pci/controller/mobiveil/pcie-mobiveil.h
index 7246de6a7176..11010a99e27c 100644
--- a/drivers/pci/controller/mobiveil/pcie-mobiveil.h
+++ b/drivers/pci/controller/mobiveil/pcie-mobiveil.h
@@ -122,11 +122,6 @@
 #define IB_WIN_SIZE			((u64)256 * 1024 * 1024 * 1024)
 #define MAX_PIO_WINDOWS			8
 
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES		10
-#define LINK_WAIT_MIN			90000
-#define LINK_WAIT_MAX			100000
-
 #define PAGED_ADDR_BNDRY		0xc00
 #define OFFSET_TO_PAGE_ADDR(off)	\
 	((off & PAGE_LO_MASK) | PAGED_ADDR_BNDRY)
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index e34bea1ff0ac..506323a6c72b 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -255,9 +255,6 @@ enum {
 #define PIO_RETRY_CNT			750000 /* 1.5 s */
 #define PIO_RETRY_DELAY			2 /* 2 us*/
 
-#define LINK_WAIT_MAX_RETRIES		10
-#define LINK_WAIT_USLEEP_MIN		90000
-#define LINK_WAIT_USLEEP_MAX		100000
 #define RETRAIN_WAIT_MAX_RETRIES	10
 #define RETRAIN_WAIT_USLEEP_US		2000
 
@@ -349,11 +346,11 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
 	int retries;
 
 	/* check if the link is up or not */
-	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+	for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
 		if (advk_pcie_link_up(pcie))
 			return 0;
 
-		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+		usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
 	}
 
 	return -ETIMEDOUT;
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 7db2c96c6cec..fc65e9fdddb3 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -140,11 +140,6 @@
 #define PCIE_PHY_LINKUP_BIT		BIT(0)
 #define PHY_RDY_LINKUP_BIT		BIT(1)
 
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES          10
-#define LINK_WAIT_USLEEP_MIN           90000
-#define LINK_WAIT_USLEEP_MAX           100000
-
 struct nwl_msi {			/* MSI information */
 	DECLARE_BITMAP(bitmap, INT_PCI_MSI_NR);
 	struct irq_domain *dev_domain;
@@ -203,10 +198,10 @@ static int nwl_wait_for_link(struct nwl_pcie *pcie)
 	int retries;
 
 	/* check if the link is up or not */
-	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+	for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
 		if (nwl_phy_link_up(pcie))
 			return 0;
-		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+		usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
 	}
 
 	dev_err(dev, "PHY link never came up\n");
diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
index 298036c3e7f9..542a751b6f4d 100644
--- a/drivers/pci/controller/plda/pcie-starfive.c
+++ b/drivers/pci/controller/plda/pcie-starfive.c
@@ -45,11 +45,6 @@
 #define STG_SYSCON_LNKSTA_OFFSET		0x170
 #define DATA_LINK_ACTIVE			BIT(5)
 
-/* Parameters for the waiting for link up routine */
-#define LINK_WAIT_MAX_RETRIES	10
-#define LINK_WAIT_USLEEP_MIN	90000
-#define LINK_WAIT_USLEEP_MAX	100000
-
 struct starfive_jh7110_pcie {
 	struct plda_pcie_rp plda;
 	struct reset_control *resets;
@@ -217,12 +212,12 @@ static int starfive_pcie_host_wait_for_link(struct starfive_jh7110_pcie *pcie)
 	int retries;
 
 	/* Check if the link is up or not */
-	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+	for (retries = 0; retries < PCIE_LINK_WAIT_MAX_RETRIES; retries++) {
 		if (starfive_pcie_link_up(&pcie->plda)) {
 			dev_info(pcie->plda.dev, "port link up\n");
 			return 0;
 		}
-		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+		usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
 	}
 
 	return -ETIMEDOUT;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index b36667969ad5..54a9e06e85f8 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -63,6 +63,8 @@ struct pcie_tlp_log;
 /* Parameters for the waiting for link up routine */
 #define PCIE_LINK_WAIT_MAX_RETRIES	10
 #define PCIE_LINK_WAIT_SLEEP_MS		90
+#define PCIE_LINK_WAIT_US_MIN		90000
+#define PCIE_LINK_WAIT_US_MAX		100000
 
 /* Format of TLP; PCIe r7.0, sec 2.2.1 */
 #define PCIE_TLP_FMT_3DW_NO_DATA	0x00 /* 3DW header, no data */

-- 
2.52.0

[PATCH v5 3/4] PCI: tegra: Add Tegra264 support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add a driver for the PCIe controller found on NVIDIA Tegra264 SoCs. The
driver is very small, with its main purpose being to set up the address
translation registers and then creating a standard PCI host using ECAM.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- make PCIE_TEGRA264 symbol tristate
- drop dependency on PCI_MSI
- reorganize tegra264_pcie struct
- use standard wake-gpios property
- rename tegra264_pcie_bpmp_set_rp_state() to tegra264_pcie_power_off()
- use dev_err() instead of dev_info() for some error messages
- add clarifying comment as to why bandwidth requests aren't fatal
- address some compiler warnings on 32-bit physical address platforms
- drop needless comments
- explicitly deinitialize controller on suspend
- use devm_pm_runtime_active_enabled()
- rename "free" label to "free_ecam"
- use dev_err_probe() in more places
- reselect default pin state during resume, not probe
- return early on absence of wake GPIO
- simplify BW value calculation

Changes in v2:
- specify generations applicable for PCI_TEGRA driver to avoid confusion
- drop SPDX-FileCopyrightText tag
- rename link_state to link_up to clarify meaning
- replace memset() by an empty initializer
- sanity-check only enable BAR regions
- bring PCI link out of reset in case firmware didn't
- use common wait times instead of defining our own
- use core helpers to parse and print PCI link speed
- fix multi-line comment
- use dev_err_probe() more ubiquitously
- fix probe sequence and error cleanup
- use DEFINE_NOIRQ_DEV_PM_OPS() to avoid warnings for !PM_SUSPEND
- reuse more standard registers and remove unused register definitions
- use %pe and ERR_PTR() to print symbolic errors
- add signed-off-by from Manikanta as the original author
- add myself as author after significantly modifying the driver
---
 drivers/pci/controller/Kconfig         |   9 +-
 drivers/pci/controller/Makefile        |   1 +
 drivers/pci/controller/pcie-tegra264.c | 544 +++++++++++++++++++++++++++++++++
 3 files changed, 553 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 2247709ef6d6..dde2f59fef7a 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -255,7 +255,14 @@ config PCI_TEGRA
 	select IRQ_MSI_LIB
 	help
 	  Say Y here if you want support for the PCIe host controller found
-	  on NVIDIA Tegra SoCs.
+	  on NVIDIA Tegra SoCs (Tegra20 through Tegra186).
+
+config PCIE_TEGRA264
+	tristate "NVIDIA Tegra264 PCIe controller"
+	depends on ARCH_TEGRA || COMPILE_TEST
+	help
+	  Say Y here if you want support for the PCIe host controller found
+	  on NVIDIA Tegra264 SoCs.
 
 config PCIE_RCAR_HOST
 	bool "Renesas R-Car PCIe controller (host mode)"
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index ac8db283f0fe..d478743b5142 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_PCI_HYPERV_INTERFACE) += pci-hyperv-intf.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
+obj-$(CONFIG_PCIE_TEGRA264) += pcie-tegra264.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCIE_RCAR_HOST) += pcie-rcar.o pcie-rcar-host.o
 obj-$(CONFIG_PCIE_RCAR_EP) += pcie-rcar.o pcie-rcar-ep.o
diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
new file mode 100644
index 000000000000..0b8e0f724e25
--- /dev/null
+++ b/drivers/pci/controller/pcie-tegra264.c
@@ -0,0 +1,544 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Tegra264 SoC
+ *
+ * Copyright (c) 2022-2026, NVIDIA CORPORATION. All rights reserved.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/init.h>
+#include <linux/interconnect.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/pci-ecam.h>
+#include <linux/pci.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <soc/tegra/bpmp.h>
+#include <soc/tegra/bpmp-abi.h>
+#include <soc/tegra/fuse.h>
+
+#include "../pci.h"
+
+/* XAL registers */
+#define XAL_RC_ECAM_BASE_HI			0x00
+#define XAL_RC_ECAM_BASE_LO			0x04
+#define XAL_RC_ECAM_BUSMASK			0x08
+#define XAL_RC_IO_BASE_HI			0x0c
+#define XAL_RC_IO_BASE_LO			0x10
+#define XAL_RC_IO_LIMIT_HI			0x14
+#define XAL_RC_IO_LIMIT_LO			0x18
+#define XAL_RC_MEM_32BIT_BASE_HI		0x1c
+#define XAL_RC_MEM_32BIT_BASE_LO		0x20
+#define XAL_RC_MEM_32BIT_LIMIT_HI		0x24
+#define XAL_RC_MEM_32BIT_LIMIT_LO		0x28
+#define XAL_RC_MEM_64BIT_BASE_HI		0x2c
+#define XAL_RC_MEM_64BIT_BASE_LO		0x30
+#define XAL_RC_MEM_64BIT_LIMIT_HI		0x34
+#define XAL_RC_MEM_64BIT_LIMIT_LO		0x38
+#define XAL_RC_BAR_CNTL_STANDARD		0x40
+#define XAL_RC_BAR_CNTL_STANDARD_IOBAR_EN	BIT(0)
+#define XAL_RC_BAR_CNTL_STANDARD_32B_BAR_EN	BIT(1)
+#define XAL_RC_BAR_CNTL_STANDARD_64B_BAR_EN	BIT(2)
+
+/* XTL registers */
+#define XTL_RC_PCIE_CFG_LINK_STATUS		0x5a
+
+#define XTL_RC_MGMT_PERST_CONTROL		0x218
+#define XTL_RC_MGMT_PERST_CONTROL_PERST_O_N	BIT(0)
+
+#define XTL_RC_MGMT_CLOCK_CONTROL		0x47c
+#define XTL_RC_MGMT_CLOCK_CONTROL_PEX_CLKREQ_I_N_PIN_USE_CONV_TO_PRSNT	BIT(9)
+
+struct tegra264_pcie {
+	struct device *dev;
+
+	/* I/O memory */
+	void __iomem *xal;
+	void __iomem *xtl;
+	void __iomem *ecam;
+
+	/* bridge configuration */
+	struct pci_config_window *cfg;
+	struct pci_host_bridge *bridge;
+
+	/* wake IRQ */
+	struct gpio_desc *wake_gpio;
+	unsigned int wake_irq;
+
+	/* BPMP and bandwidth management */
+	struct icc_path *icc_path;
+	struct tegra_bpmp *bpmp;
+	u32 ctl_id;
+
+	bool link_up;
+};
+
+static int tegra264_pcie_parse_dt(struct tegra264_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	int err;
+
+	pcie->wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
+	if (IS_ERR(pcie->wake_gpio))
+		return PTR_ERR(pcie->wake_gpio);
+
+	if (!pcie->wake_gpio)
+		return 0;
+
+	device_init_wakeup(dev, true);
+
+	err = gpiod_to_irq(pcie->wake_gpio);
+	if (err < 0)
+		return dev_err_probe(dev, err, "failed to get wake IRQ\n");
+
+	pcie->wake_irq = (unsigned int)err;
+
+	return 0;
+}
+
+static void tegra264_pcie_power_off(struct tegra264_pcie *pcie)
+{
+	struct tegra_bpmp_message msg = {};
+	struct mrq_pcie_request req = {};
+	int err;
+
+	req.cmd = CMD_PCIE_RP_CONTROLLER_OFF;
+	req.rp_ctrlr_off.rp_controller = pcie->ctl_id;
+
+	msg.mrq = MRQ_PCIE;
+	msg.tx.data = &req;
+	msg.tx.size = sizeof(req);
+
+	err = tegra_bpmp_transfer(pcie->bpmp, &msg);
+	if (err)
+		dev_err(pcie->dev, "failed to turn off PCIe #%u: %pe\n",
+			pcie->ctl_id, ERR_PTR(err));
+
+	if (msg.rx.ret)
+		dev_err(pcie->dev, "failed to turn off PCIe #%u: %d\n",
+			pcie->ctl_id, msg.rx.ret);
+}
+
+static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
+{
+	u32 value, speed, width;
+	int err;
+
+	value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
+	speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
+	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
+
+	value = Mbps_to_icc(width * PCIE_SPEED2MBS_ENC(speed));
+
+	/*
+	 * We don't want to error out here because a boot-critical device
+	 * could be connected to this root port. Failure to set the bandwidth
+	 * request may have an adverse impact on performance, but it is not
+	 * generally fatal, so we opt to continue regardless so that users
+	 * get a chance to fix things.
+	 */
+	err = icc_set_bw(pcie->icc_path, value, value);
+	if (err < 0)
+		dev_err(pcie->dev,
+			"failed to request bandwidth (%u MBps): %pe\n",
+			value, ERR_PTR(err));
+}
+
+/*
+ * The various memory regions used by the controller (I/O, memory, ECAM) are
+ * set up during early boot and have hardware-level protections in place. If
+ * the DT ranges don't match what's been setup, the controller won't be able
+ * to write the address endpoints properly, so make sure to validate that DT
+ * and firmware programming agree on these ranges.
+ */
+static bool tegra264_pcie_check_ranges(struct platform_device *pdev)
+{
+	struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
+	struct device_node *np = pcie->dev->of_node;
+	struct of_pci_range_parser parser;
+	phys_addr_t phys, limit, hi, lo;
+	struct of_pci_range range;
+	struct resource *res;
+	bool status = true;
+	u32 value;
+	int err;
+
+	err = of_pci_range_parser_init(&parser, np);
+	if (err < 0)
+		return false;
+
+	for_each_of_pci_range(&parser, &range) {
+		unsigned int addr_hi, addr_lo, limit_hi, limit_lo, enable;
+		unsigned long type = range.flags & IORESOURCE_TYPE_BITS;
+		phys_addr_t start, end, mask;
+		const char *region = NULL;
+
+		end = range.cpu_addr + range.size - 1;
+		start = range.cpu_addr;
+
+		switch (type) {
+		case IORESOURCE_IO:
+			addr_hi = XAL_RC_IO_BASE_HI;
+			addr_lo = XAL_RC_IO_BASE_LO;
+			limit_hi = XAL_RC_IO_LIMIT_HI;
+			limit_lo = XAL_RC_IO_LIMIT_LO;
+			enable = XAL_RC_BAR_CNTL_STANDARD_IOBAR_EN;
+			mask = SZ_64K - 1;
+			region = "I/O";
+			break;
+
+		case IORESOURCE_MEM:
+			if (range.flags & IORESOURCE_PREFETCH) {
+				addr_hi = XAL_RC_MEM_64BIT_BASE_HI;
+				addr_lo = XAL_RC_MEM_64BIT_BASE_LO;
+				limit_hi = XAL_RC_MEM_64BIT_LIMIT_HI;
+				limit_lo = XAL_RC_MEM_64BIT_LIMIT_LO;
+				enable = XAL_RC_BAR_CNTL_STANDARD_64B_BAR_EN;
+				region = "prefetchable memory";
+			} else {
+				addr_hi = XAL_RC_MEM_32BIT_BASE_HI;
+				addr_lo = XAL_RC_MEM_32BIT_BASE_LO;
+				limit_hi = XAL_RC_MEM_32BIT_LIMIT_HI;
+				limit_lo = XAL_RC_MEM_32BIT_LIMIT_LO;
+				enable = XAL_RC_BAR_CNTL_STANDARD_32B_BAR_EN;
+				region = "memory";
+			}
+
+			mask = SZ_1M - 1;
+			break;
+		}
+
+		/* not interested in anything that's not I/O or memory */
+		if (!region)
+			continue;
+
+		/* don't check regions that haven't been enabled */
+		value = readl(pcie->xal + XAL_RC_BAR_CNTL_STANDARD);
+		if ((value & enable) == 0)
+			continue;
+
+		hi = readl(pcie->xal + addr_hi);
+		lo = readl(pcie->xal + addr_lo);
+		phys = ((hi << 16) << 16) | lo;
+
+		hi = readl(pcie->xal + limit_hi);
+		lo = readl(pcie->xal + limit_lo);
+		limit = ((hi << 16) << 16) | lo | mask;
+
+		if (phys != start || limit != end) {
+			dev_err(pcie->dev,
+				"%s region mismatch: %pap-%pap -> %pap-%pap\n",
+				region, &phys, &limit, &start, &end);
+			status = false;
+		}
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam");
+	if (!res)
+		return false;
+
+	hi = readl(pcie->xal + XAL_RC_ECAM_BASE_HI);
+	lo = readl(pcie->xal + XAL_RC_ECAM_BASE_LO);
+	phys = ((hi << 16) << 16) | lo;
+
+	value = readl(pcie->xal + XAL_RC_ECAM_BUSMASK);
+	limit = phys + ((value + 1) << 20) - 1;
+
+	if (phys != res->start || limit != res->end) {
+		dev_err(pcie->dev,
+			"ECAM region mismatch: %pap-%pap -> %pap-%pap\n",
+			&phys, &limit, &res->start, &res->end);
+		status = false;
+	}
+
+	return status;
+}
+
+static bool tegra264_pcie_link_up(struct tegra264_pcie *pcie,
+				  enum pci_bus_speed *speed)
+{
+	u16 value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
+
+	if (value & PCI_EXP_LNKSTA_DLLLA) {
+		if (speed)
+			*speed = pcie_link_speed[FIELD_GET(PCI_EXP_LNKSTA_CLS,
+							   value)];
+
+		return true;
+	}
+
+	return false;
+}
+
+static void tegra264_pcie_init(struct tegra264_pcie *pcie)
+{
+	enum pci_bus_speed speed;
+	unsigned int i;
+	u32 value;
+
+	/* bring the endpoint out of reset */
+	value = readl(pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+	value |= XTL_RC_MGMT_PERST_CONTROL_PERST_O_N;
+	writel(value, pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+
+	if (!tegra_is_silicon()) {
+		dev_info(pcie->dev,
+			 "skipping link state for PCIe #%u in simulation\n",
+			 pcie->ctl_id);
+		pcie->link_up = true;
+		return;
+	}
+
+	for (i = 0; i < PCIE_LINK_WAIT_MAX_RETRIES; i++) {
+		if (tegra264_pcie_link_up(pcie, NULL))
+			break;
+
+		usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
+	}
+
+	if (tegra264_pcie_link_up(pcie, &speed)) {
+		msleep(PCIE_RESET_CONFIG_WAIT_MS);
+		dev_info(pcie->dev, "PCIe #%u link is up (speed: %s)\n",
+			 pcie->ctl_id, pci_speed_string(speed));
+		tegra264_pcie_icc_set(pcie);
+		pcie->link_up = true;
+	} else {
+		dev_info(pcie->dev, "PCIe #%u link is down\n", pcie->ctl_id);
+
+		value = readl(pcie->xtl + XTL_RC_MGMT_CLOCK_CONTROL);
+
+		/*
+		 * Set link state only when link fails and no hot-plug feature
+		 * is present.
+		 */
+		if ((value & XTL_RC_MGMT_CLOCK_CONTROL_PEX_CLKREQ_I_N_PIN_USE_CONV_TO_PRSNT) == 0) {
+			dev_info(pcie->dev,
+				 "PCIe #%u link is down and not hotplug-capable, turning off\n",
+				 pcie->ctl_id);
+			tegra264_pcie_power_off(pcie);
+			pcie->link_up = false;
+		} else {
+			pcie->link_up = true;
+		}
+	}
+}
+
+static void tegra264_pcie_deinit(struct tegra264_pcie *pcie)
+{
+	u32 value;
+
+	/* take the endpoint into reset */
+	value = readl(pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+	value &= ~XTL_RC_MGMT_PERST_CONTROL_PERST_O_N;
+	writel(value, pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
+}
+
+static int tegra264_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pci_host_bridge *bridge;
+	struct tegra264_pcie *pcie;
+	struct resource_entry *bus;
+	struct resource *res;
+	int err;
+
+	bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct tegra264_pcie));
+	if (!bridge)
+		return dev_err_probe(dev, -ENOMEM,
+				     "failed to allocate host bridge\n");
+
+	pcie = pci_host_bridge_priv(bridge);
+	platform_set_drvdata(pdev, pcie);
+	pcie->bridge = bridge;
+	pcie->dev = dev;
+
+	err = tegra264_pcie_parse_dt(pcie);
+	if (err < 0)
+		return dev_err_probe(dev, err, "failed to parse device tree\n");
+
+	pcie->xal = devm_platform_ioremap_resource_byname(pdev, "xal");
+	if (IS_ERR(pcie->xal))
+		return dev_err_probe(dev, PTR_ERR(pcie->xal),
+				     "failed to map XAL memory\n");
+
+	pcie->xtl = devm_platform_ioremap_resource_byname(pdev, "xtl-pri");
+	if (IS_ERR(pcie->xtl))
+		return dev_err_probe(dev, PTR_ERR(pcie->xtl),
+				     "failed to map XTL-PRI memory\n");
+
+	bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS);
+	if (!bus)
+		return dev_err_probe(dev, -ENODEV,
+				     "failed to get bus resources\n");
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam");
+	if (!res)
+		return dev_err_probe(dev, -ENXIO,
+				     "failed to get ECAM resource\n");
+
+	pcie->icc_path = devm_of_icc_get(dev, "write");
+	if (IS_ERR(pcie->icc_path))
+		return dev_err_probe(dev, PTR_ERR(pcie->icc_path),
+				     "failed to get ICC\n");
+
+	/*
+	 * Parse BPMP property only for silicon, as interaction with BPMP is
+	 * not needed for other platforms.
+	 */
+	if (tegra_is_silicon()) {
+		pcie->bpmp = tegra_bpmp_get_with_id(dev, &pcie->ctl_id);
+		if (IS_ERR(pcie->bpmp))
+			return dev_err_probe(dev, PTR_ERR(pcie->bpmp),
+					     "failed to get BPMP\n");
+	}
+
+	err = devm_pm_runtime_active_enabled(dev);
+	if (err < 0)
+		return dev_err_probe(dev, "failed to enable runtime PM\n");
+
+	pm_runtime_get_sync(dev);
+
+	/* sanity check that programmed ranges match what's in DT */
+	if (!tegra264_pcie_check_ranges(pdev)) {
+		err = -EINVAL;
+		goto put_pm;
+	}
+
+	pcie->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
+	if (IS_ERR(pcie->cfg)) {
+		err = dev_err_probe(dev, PTR_ERR(pcie->cfg),
+				    "failed to create ECAM\n");
+		goto put_pm;
+	}
+
+	bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
+	bridge->sysdata = pcie->cfg;
+	pcie->ecam = pcie->cfg->win;
+
+	tegra264_pcie_init(pcie);
+
+	if (!pcie->link_up)
+		goto free_ecam;
+
+	err = pci_host_probe(bridge);
+	if (err < 0) {
+		dev_err_probe(dev, ERR_PTR(err), "failed to register host\n");
+		goto free_ecam;
+	}
+
+	return 0;
+
+free_ecam:
+	pci_ecam_free(pcie->cfg);
+put_pm:
+	pm_runtime_put_sync(dev);
+
+	if (tegra_is_silicon())
+		tegra_bpmp_put(pcie->bpmp);
+
+	return err;
+}
+
+static void tegra264_pcie_remove(struct platform_device *pdev)
+{
+	struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
+
+	/*
+	 * If we undo tegra264_pcie_init() then link goes down and need
+	 * controller reset to bring up the link again. Remove intention is
+	 * to clean up the root bridge and re-enumerate during bind.
+	 */
+	pci_lock_rescan_remove();
+	pci_stop_root_bus(pcie->bridge->bus);
+	pci_remove_root_bus(pcie->bridge->bus);
+	pci_unlock_rescan_remove();
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	if (tegra_is_silicon())
+		tegra_bpmp_put(pcie->bpmp);
+
+	pci_ecam_free(pcie->cfg);
+}
+
+static int tegra264_pcie_suspend_noirq(struct device *dev)
+{
+	struct tegra264_pcie *pcie = dev_get_drvdata(dev);
+	int err;
+
+	tegra_pcie_deinit(pcie);
+
+	if (pcie->wake_gpio && device_may_wakeup(dev)) {
+		err = enable_irq_wake(pcie->wake_irq);
+		if (err < 0)
+			dev_err(dev, "failed to enable wake IRQ: %pe\n",
+				ERR_PTR(err));
+	}
+
+	return 0;
+}
+
+static int tegra264_pcie_resume_noirq(struct device *dev)
+{
+	struct tegra264_pcie *pcie = dev_get_drvdata(dev);
+	int err;
+
+	err = pinctrl_pm_select_default_state(dev);
+	if (err < 0)
+		dev_err(dev, "failed to configure sideband pins: %pe\n",
+			ERR_PTR(err));
+
+	if (pcie->wake_gpio && device_may_wakeup(dev)) {
+		err = disable_irq_wake(pcie->wake_irq);
+		if (err < 0)
+			dev_err(dev, "failed to disable wake IRQ: %pe\n",
+				ERR_PTR(err));
+	}
+
+	if (pcie->link_up == false)
+		return 0;
+
+	tegra264_pcie_init(pcie);
+
+	return 0;
+}
+
+static DEFINE_NOIRQ_DEV_PM_OPS(tegra264_pcie_pm_ops,
+			       tegra264_pcie_suspend_noirq,
+			       tegra264_pcie_resume_noirq);
+
+static const struct of_device_id tegra264_pcie_of_match[] = {
+	{
+		.compatible = "nvidia,tegra264-pcie",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tegra264_pcie_of_match);
+
+static struct platform_driver tegra264_pcie_driver = {
+	.probe = tegra264_pcie_probe,
+	.remove = tegra264_pcie_remove,
+	.driver = {
+		.name = "tegra264-pcie",
+		.pm = &tegra264_pcie_pm_ops,
+		.of_match_table = tegra264_pcie_of_match,
+	},
+};
+module_platform_driver(tegra264_pcie_driver);
+
+MODULE_AUTHOR("Manikanta Maddireddy <mmaddireddy@nvidia.com>");
+MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
+MODULE_DESCRIPTION("NVIDIA Tegra264 PCIe host controller driver");
+MODULE_LICENSE("GPL");

-- 
2.52.0

[PATCH v5 4/4] arm64: tegra: Reorder reg and reg-names to match bindings

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The ECAM region cannot be the first entry in the "reg" property, because
in that case the unit-address wouldn't match the first entry. The order
of the nodes can also not be changed to match the ECAM entry because the
ECAM region is global and outside of any of the control busses.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- rebase onto v7.1-rc1

Changes in v4:
- revert ECAM "reg" entry order

Changes in v2:
- order ECAM "reg" entry before others
---
 arch/arm64/boot/dts/nvidia/tegra264.dtsi | 48 ++++++++++++++++----------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra264.dtsi b/arch/arm64/boot/dts/nvidia/tegra264.dtsi
index 06d8357bdf52..d4f2a4ab03c3 100644
--- a/arch/arm64/boot/dts/nvidia/tegra264.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra264.dtsi
@@ -3556,11 +3556,11 @@ cmdqv4: cmdqv@b200000 {
 
 		pci@c000000 {
 			compatible = "nvidia,tegra264-pcie";
-			reg = <0xd0 0xb0000000 0x0 0x10000000>,
-			      <0x00 0x0c000000 0x0 0x00004000>,
+			reg = <0x00 0x0c000000 0x0 0x00004000>,
 			      <0x00 0x0c004000 0x0 0x00001000>,
-			      <0x00 0x0c005000 0x0 0x00001000>;
-			reg-names = "ecam", "xal", "xtl", "xtl-pri";
+			      <0x00 0x0c005000 0x0 0x00001000>,
+			      <0xd0 0xb0000000 0x0 0x10000000>;
+			reg-names = "xal", "xtl", "xtl-pri", "ecam";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
@@ -3892,12 +3892,12 @@ gpio_uphy: gpio@8300000 {
 
 		pci@8400000 {
 			compatible = "nvidia,tegra264-pcie";
-			reg = <0xa8 0xb0000000 0x0 0x10000000>,
-			      <0x00 0x08400000 0x0 0x00004000>,
+			reg = <0x00 0x08400000 0x0 0x00004000>,
 			      <0x00 0x08404000 0x0 0x00001000>,
 			      <0x00 0x08405000 0x0 0x00001000>,
-			      <0x00 0x08410000 0x0 0x00010000>;
-			reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+			      <0x00 0x08410000 0x0 0x00010000>,
+			      <0xa8 0xb0000000 0x0 0x10000000>;
+			reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
@@ -3924,12 +3924,12 @@ pci@8400000 {
 
 		pci@8420000 {
 			compatible = "nvidia,tegra264-pcie";
-			reg = <0xb0 0xb0000000 0x0 0x10000000>,
-			      <0x00 0x08420000 0x0 0x00004000>,
+			reg = <0x00 0x08420000 0x0 0x00004000>,
 			      <0x00 0x08424000 0x0 0x00001000>,
 			      <0x00 0x08425000 0x0 0x00001000>,
-			      <0x00 0x08430000 0x0 0x00010000>;
-			reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+			      <0x00 0x08430000 0x0 0x00010000>,
+			      <0xb0 0xb0000000 0x0 0x10000000>;
+			reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
@@ -3956,12 +3956,12 @@ pci@8420000 {
 
 		pci@8440000 {
 			compatible = "nvidia,tegra264-pcie";
-			reg = <0xb8 0xb0000000 0x0 0x10000000>,
-			      <0x00 0x08440000 0x0 0x00004000>,
+			reg = <0x00 0x08440000 0x0 0x00004000>,
 			      <0x00 0x08444000 0x0 0x00001000>,
 			      <0x00 0x08445000 0x0 0x00001000>,
-			      <0x00 0x08450000 0x0 0x00010000>;
-			reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+			      <0x00 0x08450000 0x0 0x00010000>,
+			      <0xb8 0xb0000000 0x0 0x10000000>;
+			reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
@@ -3988,12 +3988,12 @@ pci@8440000 {
 
 		pci@8460000 {
 			compatible = "nvidia,tegra264-pcie";
-			reg = <0xc0 0xb0000000 0x0 0x10000000>,
-			      <0x00 0x08460000 0x0 0x00004000>,
+			reg = <0x00 0x08460000 0x0 0x00004000>,
 			      <0x00 0x08464000 0x0 0x00001000>,
 			      <0x00 0x08465000 0x0 0x00001000>,
-			      <0x00 0x08470000 0x0 0x00010000>;
-			reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+			      <0x00 0x08470000 0x0 0x00010000>,
+			      <0xc0 0xb0000000 0x0 0x10000000>;
+			reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";
@@ -4020,12 +4020,12 @@ pci@8460000 {
 
 		pci@8480000 {
 			compatible = "nvidia,tegra264-pcie";
-			reg = <0xc8 0xb0000000 0x0 0x10000000>,
-			      <0x00 0x08480000 0x0 0x00004000>,
+			reg = <0x00 0x08480000 0x0 0x00004000>,
 			      <0x00 0x08484000 0x0 0x00001000>,
 			      <0x00 0x08485000 0x0 0x00001000>,
-			      <0x00 0x08490000 0x0 0x00010000>;
-			reg-names = "ecam", "xal", "xtl", "xtl-pri", "xpl";
+			      <0x00 0x08490000 0x0 0x00010000>,
+			      <0xc8 0xb0000000 0x0 0x10000000>;
+			reg-names = "xal", "xtl", "xtl-pri", "xpl", "ecam";
 			#address-cells = <3>;
 			#size-cells = <2>;
 			device_type = "pci";

-- 
2.52.0

Re: [PATCH v5 1/4] dt-bindings: pci: Strictly distinguish C0 from C1-C5

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `oneOf` conditional branches for C0 and C1-C5 controllers fail to specify `minItems` and `maxItems` for `reg` and `reg-names`, allowing incorrect array lengths to pass schema validation.
--

commit e94ac7315673fefb739d35acd2b3406eaf80cdbc
Author: Thierry Reding <treding@nvidia.com>

dt-bindings: pci: Strictly distinguish C0 from C1-C5

This commit strictly distinguishes the C0 controller from the C1-C5
controllers in the Tegra264 PCIe binding. It updates the register lists
and names for each variant under an allOf oneOf block to match the
hardware correctly.

> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
> index dc4f8725c9f59..acb677d477fb3 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra264-pcie.yaml
[ ... ]
>  properties:
>    compatible:
>      const: nvidia,tegra264-pcie
>  
>    reg:
>      minItems: 4
> +    maxItems: 5
>  
>    reg-names:
>      minItems: 4
> +    maxItems: 5
[ ... ]
> @@ -70,6 +61,40 @@ required:
>  
>  allOf:
>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> +  - oneOf:
> +      - description: C0 controller (no UPHY)
> +        properties:
> +          reg:
> +            items:
> +              - description: application layer registers
> +              - description: transaction layer registers
> +              - description: privileged transaction layer registers
> +              - description: ECAM compatible configuration space
> +
> +          reg-names:
> +            items:
> +              - const: xal
> +              - const: xtl
> +              - const: xtl-pri
> +              - const: ecam

[Severity: Medium]
Will this schema fully enforce the register counts for each variant?

Since JSON schema tuple validation only evaluates elements present up to the
length of the instance array, it does not enforce bounds on the length unless
minItems and maxItems are explicitly provided in these conditional branches.

Because the top-level schema permits minItems: 4 and maxItems: 5, a C0 node
providing 5 registers might incorrectly pass validation against the C0 branch.
The first 4 items match the items list, the 5th item is unconstrained by the
C0 branch, and the array satisfies the top-level maxItems: 5.

Similarly, a C1-C5 node providing only 4 registers could pass the C1-C5
branch below. The 5th constraint would never be evaluated on a 4-item array,
and the array would satisfy the top-level minItems: 4.

Should explicit minItems: 4 and maxItems: 4 be added to the C0 properties
block, along with minItems: 5 and maxItems: 5 for the C1-C5 properties block,
to ensure the strict distinction mentioned in the commit message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-tegra264-pcie-v5-0-84a813b979d7@nvidia.com?part=1

Re: [PATCH v5 3/4] PCI: tegra: Add Tegra264 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [High] Missing `err` argument in `dev_err_probe()` call.
- [High] Passing `ERR_PTR(err)` to the integer `err` argument of `dev_err_probe()`.
- [High] Call to undefined function `tegra_pcie_deinit()` in the suspend callback.
- [High] The probe function returns success when the link is down but frees resources, causing a kernel crash on removal.
- [High] The driver calls a non-existent runtime PM API `devm_pm_runtime_active_enabled()`.
- [High] The PCIe bandwidth request calculation always evaluates to 0 MBps.
- [High] Calling sleeping functions inside a noirq PM callback.
- [High] Ignoring the return value of `pm_runtime_get_sync()` leads to unsafe hardware access and kernel crashes.
- [Medium] Leaking the BPMP reference if runtime PM initialization fails.
- [Medium] Unbalanced `pm_runtime_disable()` call during driver removal.
- [Medium] Wake IRQ is mapped and enabled for wakeup but never actually requested.
- [Low] Missing `pm_ptr()` wrapper for the PM operations structure pointer.
--

commit f706a8f756af69546e040928eae0b5849025ac5c
Author: Thierry Reding <treding@nvidia.com>
Subject: PCI: tegra: Add Tegra264 support

This patch adds a driver for the PCIe controller found on NVIDIA Tegra264 SoCs,
setting up address translation registers and creating a standard PCI host
using ECAM.

> diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> new file mode 100644
> index 0000000000000..0b8e0f724e251
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-tegra264.c
[ ... ]
> +static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
> +{
> +	u32 value, speed, width;
> +	int err;
> +
> +	value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> +	speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
> +	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
> +
> +	value = Mbps_to_icc(width * PCIE_SPEED2MBS_ENC(speed));

[Severity: High]
Does this calculation evaluate to 0 MBps? The speed variable is populated
with the raw hardware CLS value, but PCIE_SPEED2MBS_ENC() expects a mapped
enum pci_bus_speed constant. If the raw values do not match the expected
constants, the macro evaluates to 0, which would heavily throttle performance.

[ ... ]
> +static int tegra264_pcie_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	if (tegra_is_silicon()) {
> +		pcie->bpmp = tegra_bpmp_get_with_id(dev, &pcie->ctl_id);
> +		if (IS_ERR(pcie->bpmp))
> +			return dev_err_probe(dev, PTR_ERR(pcie->bpmp),
> +					     "failed to get BPMP\n");
> +	}
> +
> +	err = devm_pm_runtime_active_enabled(dev);

[Severity: High]
Does devm_pm_runtime_active_enabled() exist in the kernel tree? It appears
this might be a typo for devm_pm_runtime_enable() or a similar helper, which
would cause a compilation error due to an unresolved symbol.

[Severity: Medium]
If the devm_pm_runtime_active_enabled() call fails and the function returns
early, does this leak the BPMP reference? It looks like tegra_bpmp_put()
is not called on this error path.

> +	if (err < 0)
> +		return dev_err_probe(dev, "failed to enable runtime PM\n");

[Severity: High]
Does this dev_err_probe() call have the correct number of arguments? It seems
the integer error code argument is missing. This will cause the format string
to be cast to the integer err parameter, leading to an uninitialized format
string and a likely crash or compilation error.

> +
> +	pm_runtime_get_sync(dev);

[Severity: High]
Should the return value of pm_runtime_get_sync() be checked here? If powering
on the device fails, the driver proceeds to read MMIO registers during the
subsequent tegra264_pcie_check_ranges() call. Accessing unpowered hardware
registers can trigger a synchronous external abort and kernel panic.

[ ... ]
> +	tegra264_pcie_init(pcie);
> +
> +	if (!pcie->link_up)
> +		goto free_ecam;

[Severity: High]
If the link is not up, does this branch cause probe() to return success
while freeing the ECAM resources? The err variable is not set to a negative
error code before the goto. This would cause the driver to bind successfully,
but pcie->bridge->bus would remain uninitialized, leading to a NULL pointer
dereference during unbind in tegra264_pcie_remove().

> +
> +	err = pci_host_probe(bridge);
> +	if (err < 0) {
> +		dev_err_probe(dev, ERR_PTR(err), "failed to register host\n");

[Severity: High]
Is ERR_PTR(err) the correct argument to pass here? dev_err_probe() expects an
integer as its second parameter. Passing a pointer could cause a build
breakage or result in incorrect error logging.

[ ... ]
> +static void tegra264_pcie_remove(struct platform_device *pdev)
> +{
[ ... ]
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);

[Severity: Medium]
Since a devres helper is used in probe to enable runtime PM, it will
automatically queue a pm_runtime_disable() call on removal. Does calling it
explicitly here cause an unbalanced disable_depth counter? If the driver is
unbound and rebound, the counter might remain greater than 0, breaking
runtime PM for this device permanently.

[ ... ]
> +static int tegra264_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> +	int err;
> +
> +	tegra_pcie_deinit(pcie);

[Severity: High]
Is tegra_pcie_deinit() a typo? The driver defines the deinitialization
function as tegra264_pcie_deinit(pcie). Calling an undefined function will
cause a build failure.

> +
> +	if (pcie->wake_gpio && device_may_wakeup(dev)) {
> +		err = enable_irq_wake(pcie->wake_irq);

[Severity: Medium]
Is there a registered interrupt handler for this wake IRQ? The IRQ is mapped
and enabled for wakeup, but it does not seem to be requested anywhere via
request_irq() or dev_pm_set_wake_irq(). When the IRQ triggers, the kernel
will likely find no action handler, log a spurious interrupt error, and
permanently mask the IRQ, breaking subsequent wakeups.

[ ... ]
> +static int tegra264_pcie_resume_noirq(struct device *dev)
> +{
[ ... ]
> +	if (pcie->link_up == false)
> +		return 0;
> +
> +	tegra264_pcie_init(pcie);

[Severity: High]
Can tegra264_pcie_init() safely run in a noirq callback? The callback executes
with hardware interrupts disabled, but tegra264_pcie_init() invokes sleeping
functions like usleep_range() and msleep(). Calling sleeping functions in an
atomic or noirq context will trigger a system hang or kernel bug.

[ ... ]
> +static struct platform_driver tegra264_pcie_driver = {
> +	.probe = tegra264_pcie_probe,
> +	.remove = tegra264_pcie_remove,
> +	.driver = {
> +		.name = "tegra264-pcie",
> +		.pm = &tegra264_pcie_pm_ops,

[Severity: Low]
Should the pm operations pointer be wrapped with pm_ptr()? Directly assigning
it without the wrapper prevents the compiler from eliminating the dev_pm_ops
structure as dead code when PM support is disabled.

> +		.of_match_table = tegra264_pcie_of_match,
> +	},
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-tegra264-pcie-v5-0-84a813b979d7@nvidia.com?part=3

Re: [PATCH v5 2/4] PCI: Use standard wait times for PCIe link monitoring

[Lukas Wunner]

On Tue, May 26, 2026 at 10:53:11AM +0200, Thierry Reding wrote:
> Instead of defining the wait values for each driver, use common values
> defined in the core pci.h header file. Note that most drivers don't use
> the millisecond waits, but rather usleep_range(), so add these commonly
> used values to the header so that all drivers can use them.

Hm, why not just replace usleep_range() with msleep() and use the existing
macro instead of defining new ones?

> +++ b/drivers/pci/pci.h
> @@ -63,6 +63,8 @@ struct pcie_tlp_log;
>  /* Parameters for the waiting for link up routine */
>  #define PCIE_LINK_WAIT_MAX_RETRIES	10
>  #define PCIE_LINK_WAIT_SLEEP_MS		90
> +#define PCIE_LINK_WAIT_US_MIN		90000
> +#define PCIE_LINK_WAIT_US_MAX		100000

If you absolutely positively want to add a new macro, consider
90 * USEC_PER_MSEC instead of 90000.  Moreover, consider using
fsleep() instead of adding an additional MAX macro.

Thanks,

Lukas

Re: [PATCH v5 3/4] PCI: tegra: Add Tegra264 support

[Thierry Reding]

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

On Tue, May 26, 2026 at 10:53:12AM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add a driver for the PCIe controller found on NVIDIA Tegra264 SoCs. The
> driver is very small, with its main purpose being to set up the address
> translation registers and then creating a standard PCI host using ECAM.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - make PCIE_TEGRA264 symbol tristate
> - drop dependency on PCI_MSI
> - reorganize tegra264_pcie struct
> - use standard wake-gpios property
> - rename tegra264_pcie_bpmp_set_rp_state() to tegra264_pcie_power_off()
> - use dev_err() instead of dev_info() for some error messages
> - add clarifying comment as to why bandwidth requests aren't fatal
> - address some compiler warnings on 32-bit physical address platforms
> - drop needless comments
> - explicitly deinitialize controller on suspend
> - use devm_pm_runtime_active_enabled()
> - rename "free" label to "free_ecam"
> - use dev_err_probe() in more places
> - reselect default pin state during resume, not probe
> - return early on absence of wake GPIO
> - simplify BW value calculation
> 
> Changes in v2:
> - specify generations applicable for PCI_TEGRA driver to avoid confusion
> - drop SPDX-FileCopyrightText tag
> - rename link_state to link_up to clarify meaning
> - replace memset() by an empty initializer
> - sanity-check only enable BAR regions
> - bring PCI link out of reset in case firmware didn't
> - use common wait times instead of defining our own
> - use core helpers to parse and print PCI link speed
> - fix multi-line comment
> - use dev_err_probe() more ubiquitously
> - fix probe sequence and error cleanup
> - use DEFINE_NOIRQ_DEV_PM_OPS() to avoid warnings for !PM_SUSPEND
> - reuse more standard registers and remove unused register definitions
> - use %pe and ERR_PTR() to print symbolic errors
> - add signed-off-by from Manikanta as the original author
> - add myself as author after significantly modifying the driver
> ---
>  drivers/pci/controller/Kconfig         |   9 +-
>  drivers/pci/controller/Makefile        |   1 +
>  drivers/pci/controller/pcie-tegra264.c | 544 +++++++++++++++++++++++++++++++++
>  3 files changed, 553 insertions(+), 1 deletion(-)

Ignore this one. I messed up and fixups for this ended up in an
unrelated patch. I'll send an updated version shortly.

Sorry for the noise,
Thierry

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

Re: [PATCH v5 2/4] PCI: Use standard wait times for PCIe link monitoring

[Thierry Reding]

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

On Tue, May 26, 2026 at 01:10:39PM +0200, Lukas Wunner wrote:
> On Tue, May 26, 2026 at 10:53:11AM +0200, Thierry Reding wrote:
> > Instead of defining the wait values for each driver, use common values
> > defined in the core pci.h header file. Note that most drivers don't use
> > the millisecond waits, but rather usleep_range(), so add these commonly
> > used values to the header so that all drivers can use them.
> 
> Hm, why not just replace usleep_range() with msleep() and use the existing
> macro instead of defining new ones?

I'm trying to somewhat unify the implementations across drivers without
changing behaviour. I don't have a way of testing any of these drivers,
so keeping the existing implementation and just switching out the symbol
seemed like a good compromise.

> > +++ b/drivers/pci/pci.h
> > @@ -63,6 +63,8 @@ struct pcie_tlp_log;
> >  /* Parameters for the waiting for link up routine */
> >  #define PCIE_LINK_WAIT_MAX_RETRIES	10
> >  #define PCIE_LINK_WAIT_SLEEP_MS		90
> > +#define PCIE_LINK_WAIT_US_MIN		90000
> > +#define PCIE_LINK_WAIT_US_MAX		100000
> 
> If you absolutely positively want to add a new macro, consider
> 90 * USEC_PER_MSEC instead of 90000.

That seems rather useless to me. We have a _US infix in those symbolic
names, so it's obvious what the unit is, so nobody should be confused as
to the purpose of these. I suppose if you want to make sure it
correlates to the SLEEP_MS variant we could maybe do:

  #define PCIE_LINK_WAIT_US_MIN (PCIE_LINK_WAIT_SLEEP_MS * USEC_PER_MSEC)

Then again, I count 2 drivers (in linux-next) that use the existing
PCIE_LINK_WAIT_SLEEP_MS, one of which multiplies by MILLI to get at the
US version (so it could easily be converted to the US_MIN version). Only
pcie-designware.c uses msleep() with PCIE_LINK_WAIT_SLEEP_MS, so it is
clearly the outlier.

Maybe I should just go all the way and drop the SLEEP_MS symbol and
replace usage by the US versions?

> Moreover, consider using fsleep() instead of adding an additional MAX
> macro.

This would then be changing behaviour again for all the drivers.

Thierry

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

Re: [PATCH v5 2/4] PCI: Use standard wait times for PCIe link monitoring

[Lukas Wunner]

On Wed, May 27, 2026 at 10:28:51AM +0200, Thierry Reding wrote:
> On Tue, May 26, 2026 at 01:10:39PM +0200, Lukas Wunner wrote:
> > On Tue, May 26, 2026 at 10:53:11AM +0200, Thierry Reding wrote:
> > > Instead of defining the wait values for each driver, use common values
> > > defined in the core pci.h header file. Note that most drivers don't use
> > > the millisecond waits, but rather usleep_range(), so add these commonly
> > > used values to the header so that all drivers can use them.
> > 
> > Hm, why not just replace usleep_range() with msleep() and use the existing
> > macro instead of defining new ones?
> 
> I'm trying to somewhat unify the implementations across drivers without
> changing behaviour. I don't have a way of testing any of these drivers,
> so keeping the existing implementation and just switching out the symbol
> seemed like a good compromise.

It looks like the code was copy-pasted from one driver to the next
and I applaud every effort to deduplicate, unify and simplify things.

I don't quite see how using msleep() instead of usleep_range()
should result in breakage.  The drivers are just polling for
link up for an amount of time and as long as you're not decreasing
that amount, everything ought to be fine.  Moving to msleep()
should never decrease the amount.

> Then again, I count 2 drivers (in linux-next) that use the existing
> PCIE_LINK_WAIT_SLEEP_MS, one of which multiplies by MILLI to get at the
> US version (so it could easily be converted to the US_MIN version). Only
> pcie-designware.c uses msleep() with PCIE_LINK_WAIT_SLEEP_MS, so it is
> clearly the outlier.

Konrad Dybcio went ahead and replaced usleep_range() with msleep()
in that driver, kudos to him.  Cf. b262518262f5 ("PCI: dwc: Use msleep()
in dw_pcie_wait_for_link()").  If you could just carry over that change
to the other drivers that would be great.

Thanks,

Lukas