On Fri, Jul 24, 2026 at 01:58:15PM +0530, Manikanta Maddireddy wrote: > > > On 16/07/26 10:37 pm, Thierry Reding wrote: > > From: Thierry Reding > > > > 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 > > Signed-off-by: Thierry Reding > > --- > > Changes in v8: > > - remove controller deinitialization, firmware does this already > > - separately track hotplug support and link up state for clarity > > - select pinctrl sleep state on suspend for symmetry with resume > > - fail probe if the link is down and not hotplug-capable > > - remove WAKE# IRQ support > > - add err_ prefix to gotos > > > > Changes in v7: > > - select PCI_ECAM to satisfy the build dependency (Jonathan Hunter) > > - remove pre-silicon support patch to avoid extra build dependency > > > > Changes in v6: > > - remove unneeded pm_runtime_disable() call (Sashiko) > > - do not use noirq suspend/resume callbacks (Sashiko) > > - wrap PM ops in pm_ptr() macro (Sashiko) > > - use standard wait times with msleep() (Lukas Wunner) > > - properly check errors for wake IRQs > > - fix build failures /o\ > > > > 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 | 10 +- > > drivers/pci/controller/Makefile | 1 + > > drivers/pci/controller/pcie-tegra264.c | 485 +++++++++++++++++++++++++++++++++ > > 3 files changed, 495 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > > index 2247709ef6d6..3045c8aecc7e 100644 > > --- a/drivers/pci/controller/Kconfig > > +++ b/drivers/pci/controller/Kconfig > > @@ -255,7 +255,15 @@ 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 > > + select PCI_ECAM > > + 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..7071f4b3b97b > > --- /dev/null > > +++ b/drivers/pci/controller/pcie-tegra264.c > > @@ -0,0 +1,485 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * PCIe host controller driver for Tegra264 SoC > > + * > > + * Copyright (c) 2022-2026, NVIDIA CORPORATION. All rights reserved. > > + */ > > + > > +#include > > +#include > > GPIO header can be removed since WAKE gpio feature is removed. > > > +#include > > +#include > > +#include > > Interrupt header file can be removed. Indeed. [...] > > +static int tegra264_pcie_probe(struct platform_device *pdev) [...] > > + err = pm_runtime_get_sync(dev); > > + if (err < 0) { > > + dev_err_probe(dev, err, "failed to power on device\n"); > > + goto err_put_bpmp; > > + } > > pm_runtime_get_sync() increments the runtime-PM usage counter even when its > resume operation returns an error. The failure path does not call > pm_runtime_put_noidle() and bypasses err_put_pm. Prefer > pm_runtime_resume_and_get(), whose failure semantics avoid the usage-counter > imbalance, or explicitly balance the failed pm_runtime_get_sync(). Good point. I'll replace this by pm_runtime_resume_and_get(). [...] > > +static int tegra264_pcie_suspend(struct device *dev) > > +{ > > + /* > > + * No need to do anything device-specific here, BPMP handles the > > + * details of taking the link into L2. > > + */ > > + > > + return pinctrl_pm_select_sleep_state(dev); > > +} > > + > > +static int tegra264_pcie_resume(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)); > > + return err; > > + } > > + > > + tegra264_pcie_init(pcie); > > + > > + return 0; > > +} > > + > > +static DEFINE_SIMPLE_DEV_PM_OPS(tegra264_pcie_pm_ops, > > + tegra264_pcie_suspend, > > + tegra264_pcie_resume); > > PCI subsystem registered pm_noirq callbacks. So, controller should also > register to pm_noirq callbacks. Unfortunately that causes illegal uses of msleep() in no-IRQ context. Sashiko complains about it and is right. I don't observe any runtime issues with switching to the _noirq variants, but it doesn't seem right to stick to it. Is there any reason why this needs to be _noirq, or any benefits if it was? Thierry